Skip to content
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

Merged
merged 4 commits into from Nov 22, 2019
Merged

Fix Plasma NixOS tests #73876

merged 4 commits into from Nov 22, 2019

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 21, 2019

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

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 21, 2019

Had to do more drastic changes than just update since Qt 4 is no longer supported by Phonon since #71745

@jtojnar jtojnar changed the title libsForQt5.phonon-backend-gstreamer: 4.9.0 → 4.10.0 Fix Plasma NixOS tests Nov 22, 2019
@FRidh
Copy link
Member

FRidh commented Nov 22, 2019

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.
@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019

Rebased.

@worldofpeace
Copy link
Contributor

@jtojnar Can you change the commit message phonon: mark as broken to phonon: mark as broken for qt4? At a glance I thought phonon was completely broken.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019

Qt5 libraries use libsForQt5 prefix. Anyway, I will be away from computer for the rest of the day so feel free to reword it yourself.

Copy link
Contributor

@worldofpeace worldofpeace left a 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?

@worldofpeace
Copy link
Contributor

worldofpeace commented Nov 22, 2019

Qt5 libraries use libsForQt5 prefix. Anyway, I will be away from computer for the rest of the day so feel free to reword it yourself.

I see. Will push my changes then.

@worldofpeace
Copy link
Contributor

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 withQt4 but instead asserting it to be false, with an assertMsg like Qt4 support has been removed. Please remove withQt4?

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019 via email

@worldofpeace
Copy link
Contributor

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.

I actually only considered it because it wasn't the only entry in aliases.nix that does this.
Though also, considering there's going to be a massive removal of attributes for qt4, perhaps it's like a forced upgrade to qt5? In the end, that's what a user of the alias would need this for.

But I do get the point of an alias not pointing to a fundamentally changed package, though it is still phonon.

phonon = throw "Please use libsForQt5.phonon, as Qt4 support in this package has been removed.";

jtojnar and others added 3 commits November 22, 2019 20:24
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>
@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019

Pushed, thanks.

Copy link
Contributor

@worldofpeace worldofpeace left a 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.

@worldofpeace worldofpeace merged commit 9995881 into NixOS:master Nov 22, 2019
@worldofpeace worldofpeace deleted the phonon-backends branch November 22, 2019 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants