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

qt6.wrapQtAppsHook: remove dependencies: qtbase qtwayland #174946

Merged

Conversation

milahu
Copy link
Contributor

@milahu milahu commented May 27, 2022

Description of changes

fix #174828

make dependencies explicit to reduce closure size for cli apps

qt cli apps: buildInputs = [ qtbase ];

qt gui apps: buildInputs = [ qtbase qtwayland ];

qt wrappers like pyqt or pyside: ?

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@milahu milahu changed the title Qt6 wrap qt apps hook remove qtwayland qt6.wrapQtAppsHook: remove dependency qtwayland May 27, 2022
@NickCao
Copy link
Member

NickCao commented May 27, 2022

A second thought on this lead me to believe that qtbase thould be removed from deps too. The hook itself does not require qtbase as a dep.

@NickCao
Copy link
Member

NickCao commented May 27, 2022

A clean separation between the hook which runs on the buildPlatform, and qtbase on the hostPlatform would also make cross compilations eaiser, without having to deal with splicing and friends.

@milahu
Copy link
Contributor Author

milahu commented May 27, 2022

yes

also tdesktop would have failed because dontWrapQtApps = true;

@milahu milahu force-pushed the qt6-wrapQtAppsHook-remove-qtwayland branch from b870ac2 to 784a633 Compare May 27, 2022 15:53
@milahu milahu changed the title qt6.wrapQtAppsHook: remove dependency qtwayland qt6.wrapQtAppsHook: remove dependencies: qtbase qtwayland May 27, 2022
@SuperSandro2000 SuperSandro2000 merged commit 765a6ab into NixOS:master May 30, 2022
@nrdxp
Copy link
Contributor

nrdxp commented Jul 4, 2022

So this doesn't have the advertised affect, and so perhaps this should be reverted, unless/until we find the problem.

x-ref: #177530 (comment)

If we do revert, we should make sure qtwayland is only added on linux.

@SuperSandro2000
Copy link
Member

So this doesn't have the advertised affect, and so perhaps this should be reverted, unless/until we find the problem.

It has at least one effect: It reduces closure size and I would like to keep that.

If qtwayland needs to be added back to the hook, we should think about an extra hook or make qtwayland optional.

@milahu milahu deleted the qt6-wrapQtAppsHook-remove-qtwayland branch January 25, 2023 10:35
Janik-Haag added a commit to Janik-Haag/nixpkgs that referenced this pull request Mar 16, 2023
yu-re-ka pushed a commit that referenced this pull request Mar 16, 2023
GUI applications are supposed to add qtwayland to buildInputs, see #174946 for details

Co-authored-by: Janik H <janik@aq0.de>
@euank euank mentioned this pull request Mar 26, 2023
11 tasks
@NickCao NickCao mentioned this pull request Jun 6, 2023
12 tasks
@FlorianFranzen
Copy link
Contributor

Can we please remove the 'xcb' backends as well, as it bloats all qt derivations for me on wayland.

As a matter of fact why do we not remove all platform plugin. Per default that seems to be eglfs, linuxfb, minimal, minimalegl, offscreen, vkkhrdisplay, vnc and xcb, so there is still a lot of bloat to remove.

It might force users to rebuild all qt applications if they want to use qt at all, but it makes the derivation very small, which seems the main goal here. It is their fault to choose to run a bloaty graphical user interface to begin with.

Mic92 pushed a commit that referenced this pull request Dec 9, 2023
github-actions bot pushed a commit that referenced this pull request Dec 9, 2023
See #174946.

(cherry picked from commit f62f553)
Mic92 pushed a commit that referenced this pull request Dec 9, 2023
See #174946.

(cherry picked from commit f62f553)
@Aleksanaa Aleksanaa mentioned this pull request Feb 8, 2024
13 tasks
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.

qt6Packages.callPackage should not add qt6.qtdeclarative to dependencies
5 participants