pyqt6: 6.5.2 -> 6.6.0

[nrdxp]

Description of changes

update to match current qt release version.

Also included is a fix for ffado package to remove the GUI mixer by default, since that causes webengine to depend on pyqt unnecessarily.

There is no way way to avoid a costly rebuild of qtwebengine for this change, but this fix will resolve having to do so each time we bump pyqt. addressed in #269467

Because a bump of sip was also required, there are quite a few package rebuilds.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[lilyinstarlight]

Should python3Packages.pyqt6-charts also be bumped from 6.5.0 to 6.6.0 here?

[nrdxp]

Should python3Packages.pyqt6-charts also be bumped from 6.5.0 to 6.6.0 here?

didn't catch that one, yes thank you

[nrdxp]

any objections to go ahead an merge this? My machine is not beefy enough to run a full nixpkgs-review here, but I have built the pyqt6 package successfully at least.

[LunNova]

Changes look good, can't do a full nixpkgs-review due to rebuild count.

nom build .#ffado .#python3Packages.pyqt6 .#qt6Packages.qtwebengine succeeds

[2xsaiko]

How about only disabling the ffado mixer in the package where it would cause a dependency cycle instead of by default? That would mean it would still work if the flag is enabled globally via an overlay (and would avoid having to do that or manually enabling it in the first place for people who want the mixer), and preserve it being there.

[K900]

Is there even an actual dependency cycle here? Or are you just trying to reduce the amount of rebuilds caused by pyqt updates?

[K900]

-1 until we figure out the ffado situation

[nrdxp]

How about only disabling the ffado mixer in the package where it would cause a dependency cycle instead of by default? That would mean it would still work if the flag is enabled globally via an overlay (and would avoid having to do that or manually enabling it in the first place for people who want the mixer), and preserve it being there.

Is that the right default though? Any package that uses ffado as a build dep probably doesn't want the mixer, and doesn't want to bring in a huge graphical stack as a dependency for no good reason. Whereas as user who does can simple toggle it in their config fairly easily.

Is there even an actual dependency cycle here? Or are you just trying to reduce the amount of rebuilds caused by pyqt updates?

Define "actual dependency cycle". If you mean that qtwebengine depends on pyqt5 unneccesarily before this change, and that any change to pyqt unnecessarily causes a costly rebuid of webengine for no good reason, then the answer is yes.

[K900]

So there is no actual dependency cycle, as in there's no situation where A depends on B, and B depends on A, and the result is impossible to express directly in Nix. Whether something is "unnecessary" or not has no bearing on whether it causes a dependency cycle.

[nrdxp]

Okay, well perhaps I used incorrect wording, but in any case, it seems an appropriate change and should reduce the closure size of any package that uses ffado as a library.

[K900]

Build closure specifically, as the mixer is fully separated into a bin output which is not actually referenced by most consumers.

[nrdxp]

What exactly are you trying to get at? I already checked that my intention was effective with nix why-depends if that's what you mean. Before this change webengine depends on pyqt5, after it does not.

[K900]

At runtime or at build time? My intention here is to figure out what exactly is being changed here and why, as the original wording of "dependency cycle" implied a very specific thing that is clearly not taking place here.

[nrdxp]

At runtime or at build time? My intention here is to figure out what exactly is being changed here and why, as the original wording of "dependency cycle" implied a very specific thing that is clearly not taking place here.

It is is both the run and build time closure of webengine that is affected. The library is not legitimately needed by the buildtime at all, but ends up in the runtime closure regardless.

[K900]

Then the actual issue we have is that the dependency is leaking out from ffado's bin output, and that's what I'd prefer to see addressed. I'll take a look tomorrow morning.

[nrdxp]

Then the actual issue we have is that the dependency is leaking out from ffado's bin output, and that's what I'd prefer to see addressed. I'll take a look tomorrow morning.

Yes, and that is the issue being addressed here. At least by default. The mixer is a GUI program which uses pyqt5. So if we leave it on by default, then any package that uses ffado will have pyqt5 in their runtime closure. Since pyqt5 is need by the ffado mixer at runtime, the only way not to leak is is to disable it entirely. Also updated OP with clearer language (hopefully)

[lilyinstarlight]

It is is both the run and build time closure of webengine that is affected. The library is not legitimately needed by the buildtime at all, but ends up in the runtime closure regardless.

Is that only the case after the rest of this PR is applied (i.e. only the non-ffado changes)? I can only confirm that it is a build-time reference on master but not a runtime reference

[I] lily@bina ~/s/nixpkgs (master)> nix why-depends --derivation nixpkgs#qt6.qtwebengine nixpkgs#python3Packages.pyqt5
/nix/store/c234l1a5fnbjnj82zc057g0y4kcbnn76-qtwebengine-6.6.0.drv
└───/nix/store/p9pxxma59bs82qzpy2i3a893g54jd66n-pipewire-0.3.80.drv
    └───/nix/store/ywzw9f4z9bnrvknshgw3z8qaa4sz56i1-ffado-2.4.7.drv
        └───/nix/store/1wb1aqidy2ldzb2h3qa8gaaf1cfdig4i-python3.11-PyQt5-5.15.9.drv
[I] lily@bina ~/s/nixpkgs (master)> nix why-depends nixpkgs#qt6.qtwebengine nixpkgs#python3Packages.pyqt5
'flake:nixpkgs#qt6.qtwebengine' does not depend on 'flake:nixpkgs#python3Packages.pyqt5'

[K900]

With this PR applied, there is no longer a build-time reference either, but this then raises the question of whether the extra complexity is worth it when PyQt updates will most likely have to go through staging anyway for the foreseeable future.

[nrdxp]

Is that only the case after the rest of this PR is applied?

It was at the time this PR was authored, perhaps something has changed in master since then.

[lilyinstarlight]

It was at the time this PR was authored, perhaps something has changed in master since then.

Closure size does not seem to have changed by a lot in a while: https://hydra.nixos.org/job/nixpkgs/trunk/pipewire.x86_64-linux#tabs-charts

But either way I guess the ffado change is unnecessary here now regardless then?

[nrdxp]

With this PR applied, there is no longer a build-time reference either, but this then raises the question of whether the extra complexity is worth it when PyQt updates will most likely have to go through staging anyway for the foreseeable future.

Previous pyqt updates didn't go through staging. It is only due to the sip bump that many python packages are being rebuilt here, but that is not always required.

[nrdxp]

It was at the time this PR was authored, perhaps something has changed in master since then.

Closure size does not seem to have changed by a lot in a while: https://hydra.nixos.org/job/nixpkgs/trunk/pipewire.x86_64-linux#tabs-charts

But either way I guess the ffado change is unnecessary here now regardless then?

It is still present on master.

> nix-store -q --requisites /nix/store/vdpw7qqpsjzmipzzpl61vs73ar1fnw1a-qtwebengine-6.6.0.drv | grep PyQt
  2001:/nix/store/226a6whr7bdkiwb59rbgz51ffgfd4617-PyQt5-5.15.9.tar.gz.drv
  2172:/nix/store/6djai5n62hbmily18az5ljawb1ns7aim-PyQt-builder-1.15.2.tar.gz.drv
  2182:/nix/store/7z053jggg6p7bmp8p6b6jlvrimpfmai3-PyQt5_sip-12.11.0.tar.gz.drv
  2184:/nix/store/ai41nxyc9m1l44x94378c2dw6hvq2d33-python3.11-PyQt5-5.15.9.drv

nix why-depends wanted to build qtwebengine, so I used legacy commands to get an answer faster.

[K900]

Those aren't comparable. Your command is equivalent to nix why-depends --derivation, which looks at the build time closure, not at the runtime closure.

[nrdxp]

Those aren't comparable. Your command is equivalent to nix why-depends --derivation, which looks at the build time closure, not at the runtime closure.

Okay, regardless the point of the change was to avoid a rebuild of webengine when pyqt is bumped. Even if it is only the build time closure, that is still enough to trigger a rebuild for no reason. qtwebengine is one of the heaviest builds we have, so avoiding rebuilding it for no good reason sounds like a good idea to me.

[K900]

In that case I suggest we scope this PR to just the version bump, and discuss the ffado situation separately after 23.11 branch-off.

[nrdxp]

In that case I suggest we scope this PR to just the version bump, and discuss the ffado situation separately after 23.11 branch-off.

Sounds like a fair compromise. I can prepare a separate PR for that later then.

[2xsaiko]

Is that the right default though? Any package that uses ffado as a build dep probably doesn't want the mixer, and doesn't want to bring in a huge graphical stack as a dependency for no good reason. Whereas as user who does can simple toggle it in their config fairly easily.

It's not that simple if you want to run it with nix run or the other CLI tools. Plus, toggling the mixer off by default means it doesn't get built/tested by Hydra, so breakages might be found only after a package update.

[nrdxp]

Is that the right default though? Any package that uses ffado as a build dep probably doesn't want the mixer, and doesn't want to bring in a huge graphical stack as a dependency for no good reason. Whereas as user who does can simple toggle it in their config fairly easily.

It's not that simple if you want to run it with nix run or the other CLI tools. Plus, toggling the mixer off by default means it doesn't get built/tested by Hydra, so breakages might be found only after a package update.

Makes sense. Probably the best solution is what @K900 seemed to be getting at earlier in separating the ffado bins into their own output.

[lilyinstarlight]

With the ffado change removed and just having the version bump, this diff looks good to me now

If ofborg builds pass and @K900 is good with this now, then I think this is merge-ready. I added a backport label given this should be a backwards compatible change, but let me know/remove it if you feel it is not for some reason

Thank you @nrdxp!

[github-actions]

Successfully created backport PR for staging-23.11:

• [Backport staging-23.11] pyqt6: 6.5.2 -> 6.6.0 #269121