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.qtbase: fix macdeployqt would not find qmlimportscanner #248149

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

arximboldi
Copy link
Contributor

@arximboldi arximboldi commented Aug 9, 2023

Description of changes

The qmlimportscanner tool is provided by qtdeclarative. Because of the modularized installation in Nix, it can not be found via the usual mechanisms. Also, hard-coding it like we do for Qt5 would also not work, as it would require making qtbase depend on qtdeclarative.

Here we add an option to provide its location via the environment. While this means macdeployqt does not work out of the box, it provides a workaround for users.

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/)
  • 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.

The qmlimportscanner tool is provided by qtdeclarative. Because of the
modularized installation in Nix, it can not be found via the usual
mechanisms.  Also, hard-coding it like we do for Qt5 would also not
work, as it would require making qtbase depend on qtdeclarative.

Here we add an option to provide its location via the environment.
While this means macdeployqt does not work out of the box, it provides
a workaround for users.

Also, we make sure that qmlimportscanner gets passed the right QML
import paths as described in the environment.
@arximboldi
Copy link
Contributor Author

I just pushed some tweaks.

I have verified in another project that with all these tweaks, using macdeploytool produces valid MacOS app bundles that are usable in foreign installations (without Nix).

@arximboldi
Copy link
Contributor Author

This closes #63093

Copy link
Member

@NickCao NickCao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I'm happy with the changes, as it is isolated within macdeployqt and should not breaking existing usecases.

Copy link
Member

@NickCao NickCao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, cc @K900 to see if you have any other suggestions.

@arximboldi
Copy link
Contributor Author

Any news on this? I'd love to get this in as I have a PR on another project pending on these changes. But of course take as much time as you need, the work you're all doing on taking care of Nixpkgs is wonderful! 🙏

@NickCao
Copy link
Member

NickCao commented Aug 15, 2023

Any news on this? I'd love to get this in as I have a PR on another project pending on these changes. But of course take as much time as you need, the work you're all doing on taking care of Nixpkgs is wonderful! pray

@K900 might be too busy to take a look, let's merge this and see.

@NickCao NickCao merged commit ac0f2fd into NixOS:master Aug 15, 2023
25 checks passed
@arximboldi
Copy link
Contributor Author

Thank you so much @NickCao !

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

2 participants