New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Plasma NixOS tests #73876
Fix Plasma NixOS tests #73876
Conversation
d07e991
to
1c04e1a
Compare
1c04e1a
to
8e20453
Compare
c8d62de
to
7e1fd81
Compare
Had to do more drastic changes than just update since Qt 4 is no longer supported by Phonon since #71745 |
This needs rebasing. Note many parts of KDE are failing since the staging-next merge. #73756 |
Phonon no longer supports Qt4 so this is useless.
7e1fd81
to
c9ab490
Compare
Rebased. |
@jtojnar Can you change the commit message |
Qt5 libraries use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://phabricator.kde.org/D22688 says qt4 was removed there entirely.
Can we remove all withQt4 options, and the ? null
for the function arguments?
I see. Will push my changes then. |
c9ab490
to
9b236c2
Compare
Here were my changes There was already a todo to remove it completely, so I added aliases and removed the qt4 versions and options. @jtojnar I think you kept the arguments to fail gracefully, it kinda sucks we don't have a nice thing like aliases like this. What do you think about keeping |
After giving it some more thought, consumers would be using the
libraries directly as supplied through `(libsForQt5.)callPackage` so I
would not bother keeping the arguments. It should be pretty easy to
figure out the cause for someone who is deliberately overriding it
directly.
What we should not do is point the aliases to the Qt5 versions – they
are fundamentally different packages. I think changing the aliases to
throw should work here.
…On Fri, 22 Nov, 2019 at 07:11, worldofpeace ***@***.***> wrote:
Here were my changes
https://github.com/NixOS/nixpkgs/compare/c9ab4901720f470c6bd9cf75563bd186370c3caa..9b236c2d567461076a83abb05629f7f6d03eb886
There was already a todo to remove it completely, so I added aliases
and removed the qt4 versions and options. @jtojnar I think you kept
the arguments to fail gracefully, it kinda sucks we don't have a nice
thing like aliases like this. What do you think about keeping withQt4
but instead asserting it to be false, with an assertMsg like Qt4
support has been removed. Please remove withQt4?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I actually only considered it because it wasn't the only entry in aliases.nix that does this. But I do get the point of an alias not pointing to a fundamentally changed package, though it is still phonon.
|
Required to build with Phonon 4.11 (NixOS#71745). Not having this blocks the channels. Requires qttools for Qt5LinguistTools. Qt4 support removed since Phonon no longer supports it either. Co-authored-by: worldofpeace <worldofpeace@protonmail.ch>
Required to build with Phonon 4.11 (NixOS#71745). Requires qttools for Qt5LinguistTools. Qt4 support removed since Phonon no longer supports it either. Co-authored-by: worldofpeace <worldofpeace@protonmail.ch>
Qt4 is no longer supported. https://phabricator.kde.org/D22688 Co-authored-by: worldofpeace <worldofpeace@protonmail.ch>
9b236c2
to
62a09e7
Compare
Pushed, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, going to merge right away because our channel is blocked.
Required to build with Phonon 4.11 (#71745). Not having this blocks the channels.
Phonon no longer supports Qt 4 so I marked it as broken there.
cc @ttuegel