-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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: add darwin support #202015
qt6: add darwin support #202015
Conversation
4e1bfa4
to
a7bae16
Compare
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.
diff LGTM
Actually this could have just targeted master, the amount of rebuild is relatively small. |
nixpkgs-review on aarch64-darwin: 2 packages failed to build: 6 packages built: |
@wegank would you mind at least fixing the build of qxlsx to ensure that qtbase actually works? |
qt6Packages.qxlsx can build if adding |
a7bae16
to
5ecb508
Compare
Rebased to master and fixed both |
Result of 8 packages built:
|
5ecb508
to
91db85f
Compare
91db85f
to
33bc099
Compare
Only failure |
Result of 3 packages marked as broken and skipped:
86 packages built:
|
Looks good! Tried out Calibre too which depends on QtWebEngine which is a fairly good manual test. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
@@ -90,7 +98,9 @@ let | |||
qt3d = callPackage ./modules/qt3d.nix { }; | |||
qt5compat = callPackage ./modules/qt5compat.nix { }; | |||
qtcharts = callPackage ./modules/qtcharts.nix { }; | |||
qtconnectivity = callPackage ./modules/qtconnectivity.nix { }; | |||
qtconnectivity = callPackage ./modules/qtconnectivity.nix { |
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.
You must use the callPackage wrapper or integrate it into qt's callPackage wrapper.
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.
It actually uses the same callPackage wrapper as in qtbase.
--replace '$$PROTOBUF/lib/libprotobuf-lite.a' '${protobuf}/lib/libprotobuf-lite.dylib' | ||
''; | ||
|
||
qmakeFlags = lib.optionals stdenv.isDarwin [ | ||
"CONFIG+=c++17" | ||
"QMAKE_MACOSX_DEPLOYMENT_TARGET=10.15" |
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.
Should we get this from the sdk version?
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.
I don't think so, it's just to avoid an error (in the comment) mentioning macOS 10.15. Also, this package seems to rely on Xcode Command Line Tools. Should we mark this as broken for now?
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.
If it doesn't work sure. We can always revisit it. Also this keeps the scope of this PR manageable.
a036c58
to
a44cd98
Compare
a44cd98
to
44b66a3
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
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.
Diff LGTM.
Description of changes
#188938
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes