-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
Add mingw32 support qtbase #287077
Add mingw32 support qtbase #287077
Conversation
edit: need to rerun this once the commit it depends on reaches master |
Given the rebuild count, should I be targeting the staging branch? |
No, below 500 is fine for master. |
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
@ofborg build pkgs.pkgsCross.mingwW64.qt6.qtbase |
ca88b68
to
d8f8ba2
Compare
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
@ofborg build pkgs.pkgsCross.mingwW64.qt6.qtbase |
@@ -42,7 +42,7 @@ let | |||
qtModule = callPackage ./qtModule.nix { }; | |||
|
|||
qtbase = callPackage ./modules/qtbase.nix { | |||
withGtk3 = true; | |||
withGtk3 = (!stdenv.hostPlatform.isMinGW); |
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.
Unneeded parenthesis.
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.
fixed
Just synced and rebased with main. Running |
d8f8ba2
to
a964579
Compare
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
a964579
to
84c6e1e
Compare
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
@ofborg build pkgs.pkgsCross.mingwW64.qt6.qtbase |
You mean specifically marking the combination of host darwin target mingw as broken? I didn't even know mingw worked on darwin 😅. Marking that broken seems fine. I can't test anything on mac, will give this a review run on x64 and make sure a few apps still work there. |
Just to make sure there is no confusion, what I mean is, if your build platfrom is darwin and your host platform is mingw then the build will fail. |
@ofborg build citra-canary |
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.
Packages I use frequently seem fine, not seeing regressions for x64 linux.
I'm not great with Nix cross compilation so I haven't thoroughly reviewed the change for that. Nothing's standing out as wrong.
@@ -42,7 +42,7 @@ let | |||
qtModule = callPackage ./qtModule.nix { }; | |||
|
|||
qtbase = callPackage ./modules/qtbase.nix { | |||
withGtk3 = true; | |||
withGtk3 = !stdenv.hostPlatform.isMinGW; |
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.
Isn't gtk3 available on mingw?
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.
Nope
nix build '.#pkgs.pkgsCross.mingw32.gtk3'
error:
… while calling the 'derivationStrict' builtin
at /builtin/derivation.nix:9:12: (source not available)
… while evaluating derivation 'gtk+3-i686-w64-mingw32-3.24.39'
whose name attribute is located at /nix/store/kp890n9higzcq3n6bzqimvnl2qkx5i1v-source/pkgs/stdenv/generic/make-derivation.nix:353:7
… while evaluating attribute 'buildInputs' of derivation 'gtk+3-i686-w64-mingw32-3.24.39'
at /nix/store/kp890n9higzcq3n6bzqimvnl2qkx5i1v-source/pkgs/stdenv/generic/make-derivation.nix:400:7:
399| depsHostHost = elemAt (elemAt dependencies 1) 0;
400| buildInputs = elemAt (elemAt dependencies 1) 1;
| ^
401| depsTargetTarget = elemAt (elemAt dependencies 2) 0;
error: Package ‘libxkbcommon-1.5.0’ in /nix/store/kp890n9higzcq3n6bzqimvnl2qkx5i1v-source/pkgs/development/libraries/libxkbcommon/default.nix:53 is not available on the requested hostPlatform:
hostPlatform.config = "i686-w64-mingw32"
package.meta.platforms = [
"i686-cygwin"
"x86_64-cygwin"
"x86_64-darwin"
"i686-darwin"
"aarch64-darwin"
"armv7a-darwin"
"i686-freebsd13"
"x86_64-freebsd13"
"x86_64-solaris"
"aarch64-linux"
"armv5tel-linux"
"armv6l-linux"
"armv7a-linux"
"armv7l-linux"
"i686-linux"
"loongarch64-linux"
"m68k-linux"
"microblaze-linux"
"microblazeel-linux"
"mips-linux"
"mips64-linux"
"mips64el-linux"
"mipsel-linux"
"powerpc64-linux"
"powerpc64le-linux"
"riscv32-linux"
"riscv64-linux"
"s390-linux"
"s390x-linux"
"x86_64-linux"
"aarch64-netbsd"
"armv6l-netbsd"
"armv7a-netbsd"
"armv7l-netbsd"
"i686-netbsd"
"m68k-netbsd"
"mipsel-netbsd"
"powerpc-netbsd"
"riscv32-netbsd"
"riscv64-netbsd"
"x86_64-netbsd"
"i686-openbsd"
"x86_64-openbsd"
"x86_64-redox"
]
package.meta.badPlatforms = [ ]
, refusing to evaluate.
a) To temporarily allow packages that are unsupported for this system, you can use an environment variable
for a single invocation of the nix tools.
$ export NIXPKGS_ALLOW_UNSUPPORTED_SYSTEM=1
Note: When using `nix shell`, `nix build`, `nix develop`, etc with a flake,
then pass `--impure` in order to allow use of environment variables.
b) For `nixos-rebuild` you can set
{ nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.
c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
{ allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.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.
Then, using !lib.meta.availableOn stdenv.hostPlatform gtk3
is more appropriate.
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 think there is a bug in the in the gtk3 package
nix-repl> lib.meta.availableOn pkgsCross.mingw32.stdenv.hostPlatform gtk3
true
nix-repl> :b pkgsCross.mingw32.gtk3
error:
… while calling the 'derivationStrict' builtin
at /builtin/derivation.nix:9:12: (source not available)
… while evaluating derivation 'gtk+3-i686-w64-mingw32-3.24.39'
whose name attribute is located at /home/sean/nixpkgs-contributing/pkgs/stdenv/generic/make-derivation.nix:353:7
… while evaluating attribute 'buildInputs' of derivation 'gtk+3-i686-w64-mingw32-3.24.39'
at /home/sean/nixpkgs-contributing/pkgs/stdenv/generic/make-derivation.nix:400:7:
399| depsHostHost = elemAt (elemAt dependencies 1) 0;
400| buildInputs = elemAt (elemAt dependencies 1) 1;
| ^
401| depsTargetTarget = elemAt (elemAt dependencies 2) 0;
error: Package ‘libxkbcommon-1.5.0’ in /home/sean/nixpkgs-contributing/pkgs/development/libraries/libxkbcommon/default.nix:60 is not available on the requested hostPlatform:
hostPlatform.config = "i686-w64-mingw32"
package.meta.platforms = [
"i686-cygwin"
"x86_64-cygwin"
"x86_64-darwin"
"i686-darwin"
"aarch64-darwin"
"armv7a-darwin"
"i686-freebsd13"
"x86_64-freebsd13"
"x86_64-solaris"
"aarch64-linux"
"armv5tel-linux"
"armv6l-linux"
"armv7a-linux"
"armv7l-linux"
"i686-linux"
"loongarch64-linux"
"m68k-linux"
"microblaze-linux"
"microblazeel-linux"
"mips-linux"
"mips64-linux"
"mips64el-linux"
"mipsel-linux"
"powerpc64-linux"
"powerpc64le-linux"
"riscv32-linux"
"riscv64-linux"
"s390-linux"
"s390x-linux"
"x86_64-linux"
"aarch64-netbsd"
"armv6l-netbsd"
"armv7a-netbsd"
"armv7l-netbsd"
"i686-netbsd"
"m68k-netbsd"
"mipsel-netbsd"
"powerpc-netbsd"
"riscv32-netbsd"
"riscv64-netbsd"
"x86_64-netbsd"
"i686-openbsd"
"x86_64-openbsd"
"x86_64-redox"
]
package.meta.badPlatforms = [ ]
, refusing to evaluate.
a) To temporarily allow packages that are unsupported for this system, you can use an environment variable
for a single invocation of the nix tools.
$ export NIXPKGS_ALLOW_UNSUPPORTED_SYSTEM=1
Note: When using `nix shell`, `nix build`, `nix develop`, etc with a flake,
then pass `--impure` in order to allow use of environment variables.
b) For `nixos-rebuild` you can set
{ nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.
c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
{ allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.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.
No change since gtk3 appears to indicate it has mingw support when in fact it doesn't.
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.
We really should fix this on the gtk3 side.
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.
My hope is that we can leave the mingw conditional for now to get initial mingw support for qtbase. When gtk3 actually has mingw support, we can change that line. I suspect that adding mingw support for gtk3 may not be trivial.
qtwebsockets | ||
qtwebview | ||
] ++ lib.optionals (!stdenv.isDarwin) [ qtwayland libglvnd ])) { }; | ||
full = callPackage |
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.
Please drop the formatting changes.
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.
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.
Fine then.
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 is a bug in nixpkgs-fmt that the after the callPackage a line break is added. Also it makes the entire PR looking bigger than it is.
"-DQT_HOST_PATH=${pkgsBuildBuild.qt6.qtbase}" | ||
"-DQt6HostInfo_DIR=${pkgsBuildBuild.qt6.qtbase}/lib/cmake/Qt6HostInfo" |
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 these paths are explicitly set, maybe there's no need to put it in depsBuildBuild? (Also causes inf recursion.)
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 are correct. Logic updated to only include this when stdenv.hostPlatform != stdenv.buildPlatform. Fixed.
And blanket fix for |
84c6e1e
to
987e38d
Compare
There was a regression introduced in #290266 Failure
Success
|
Thank you for raising this, @seanybaggins. |
cfb46f9
to
2c154d3
Compare
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
@ofborg build pkgs.pkgsCross.mingwW64.qt6.qtbase |
@NickCao this is ready to be reviewed and hopefully merged in :) |
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.
All the hardcoded stdenv.hostPlatform.isMinGW
predicated should be replaced with either isCrossBuild
or lib.meta.availableOn stdenv.hostPlatform <package>
. Otherwise LGTM.
, qttranslations ? null | ||
, isCrossBuild ? stdenv.hostPlatform != stdenv.buildPlatform |
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 be stdenv.buildPlatform.canExecute stdenv.hostPlatform
and be kept in the let .. in clause below (as there's no point in exposing this as an argument).
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.
Fixed
@@ -42,7 +42,7 @@ let | |||
qtModule = callPackage ./qtModule.nix { }; | |||
|
|||
qtbase = callPackage ./modules/qtbase.nix { | |||
withGtk3 = true; | |||
withGtk3 = !stdenv.hostPlatform.isMinGW; |
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.
We really should fix this on the gtk3 side.
libb2 | ||
md4c | ||
] ++ lib.optionals (!stdenv.hostPlatform.isMinGW) [ | ||
double-conversion |
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.
double-conversion
is now supported on windows since #290089
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.
fixed
at-spi2-core | ||
] ++ lib.optionals (!stdenv.hostPlatform.isDarwin) [ | ||
] ++ lib.optionals (!stdenv.hostPlatform.isDarwin && !stdenv.hostPlatform.isMinGW) [ |
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.
] ++ lib.optionals (!stdenv.hostPlatform.isDarwin && !stdenv.hostPlatform.isMinGW) [ | |
] ++ lib.optionals (lib.meta.availableOn stdenv.hostPlatform libinput) [ |
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.
fixed
++ lib.optional (cups != null && !stdenv.hostPlatform.isMinGW) cups | ||
++ lib.optional (libmysqlclient != null && !stdenv.hostPlatform.isMinGW) libmysqlclient | ||
++ lib.optional (postgresql != null && !stdenv.hostPlatform.isMinGW) postgresql; |
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.
++ lib.optional (cups != null && !stdenv.hostPlatform.isMinGW) cups | |
++ lib.optional (libmysqlclient != null && !stdenv.hostPlatform.isMinGW) libmysqlclient | |
++ lib.optional (postgresql != null && !stdenv.hostPlatform.isMinGW) postgresql; | |
++ lib.optional (cups != null && lib.meta.availableOn stdenv.hostPlatform cups) cups | |
++ lib.optional (libmysqlclient != null && lib.meta.availableOn stdenv.hostPlatform libmysqlclient) libmysqlclient | |
++ lib.optional (postgresql != null && lib.meta.availableOn stdenv.hostPlatform postgresql) postgresql; |
] ++ lib.optionals stdenv.hostPlatform.isMinGW [ | ||
vulkan-headers | ||
vulkan-loader | ||
]; |
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.
Why only for mingw? Shall we also add vulkan support to linux builds?
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.
This isn't a mingw only thing. See
nixpkgs/pkgs/development/libraries/qt-6/modules/qtbase.nix
Lines 142 to 143 in b5c9562
vulkan-headers | |
vulkan-loader |
|
||
buildInputs = [ | ||
buildInputs = lib.optionals (!stdenv.hostPlatform.isMinGW) [ |
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.
buildInputs = lib.optionals (!stdenv.hostPlatform.isMinGW) [ | |
buildInputs = lib.optionals (lib.meta.availableOn stdenv.hostPlatform at-spi2-core) [ |
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.
fixed
@@ -202,7 +211,7 @@ stdenv.mkDerivation rec { | |||
inherit patches; | |||
|
|||
# https://bugreports.qt.io/browse/QTBUG-97568 | |||
postPatch = '' | |||
postPatch = lib.optionalString (!stdenv.hostPlatform.isMinGW) '' |
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.
postPatch = lib.optionalString (!stdenv.hostPlatform.isMinGW) '' | |
postPatch = '' |
coreutils
are executed on the build platform not the host platform.
@@ -202,7 +211,7 @@ stdenv.mkDerivation rec { | |||
inherit patches; | |||
|
|||
# https://bugreports.qt.io/browse/QTBUG-97568 | |||
postPatch = '' | |||
postPatch = lib.optionalString (!stdenv.hostPlatform.isMinGW) '' | |||
substituteInPlace src/corelib/CMakeLists.txt --replace-fail "/bin/ls" "${coreutils}/bin/ls" |
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.
substituteInPlace src/corelib/CMakeLists.txt --replace-fail "/bin/ls" "${coreutils}/bin/ls" | |
substituteInPlace src/corelib/CMakeLists.txt --replace-fail "/bin/ls" "${buildPackages.coreutils}/bin/ls" |
"-DQT_HOST_PATH=${pkgsBuildBuild.qt6.qtbase}" | ||
"-DQt6HostInfo_DIR=${pkgsBuildBuild.qt6.qtbase}/lib/cmake/Qt6HostInfo" | ||
] | ||
++ lib.optional (qttranslations != null && !stdenv.hostPlatform.isMinGW) "-DINSTALL_TRANSLATIONSDIR=${qttranslations}/translations"; |
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.
++ lib.optional (qttranslations != null && !stdenv.hostPlatform.isMinGW) "-DINSTALL_TRANSLATIONSDIR=${qttranslations}/translations"; | |
++ lib.optional (qttranslations != null && !isCrossBuild) "-DINSTALL_TRANSLATIONSDIR=${qttranslations}/translations"; |
FYI: #290761 has been merged. @seanybaggins have the changes from that PR found their way into this PR and did they address the issues you were observing? |
Your PR worked. Now able to build again :) |
Thanks for confirming, @seanybaggins, much appreciated and much success with this PR! |
2c154d3
to
fd9c0ae
Compare
fd9c0ae
to
174ca6a
Compare
@NickCao ready for another round. |
@ofborg build pkgs.pkgsCross.mingw32.qt6.qtbase |
@ofborg build pkgs.pkgsCross.mingwW64.qtbase |
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, some nitpicks can be handled later. I intend to merge this after the plasma 6 update to reduce the risk of introducing breakages (and rebuilds for the testers).
When is the plasma 6 update? Are you able to link to something? |
Must be this: #286522 (merged two hours ago). |
Now running nixpkgs-review. |
Didn't finish nixpkgs-review, but looks clean. |
Description of changes
Waiting for #285661 to land in master.
See also https://nixpk.gs/pr-tracker.html?pr=285661
Relevant for #272538 and #274274
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.