qt59.qtbase: fix darwin build so qtdeclarative can build #29812

j-hao · 2017-09-26T15:14:19Z

Motivation for this change

All changes are for Darwin: move sed command to patch files; disable mesa; fix OpenGL and AGL header files path. qtdeclarative and qtwebkit can be built

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

ttuegel

I cannot test on macOS, so I should not approve this request, but neither does anything look wrong.

LnL7 · 2017-09-26T20:24:15Z

pkgs/development/libraries/qt-5/5.9/qtbase/default.nix

@@ -80,7 +80,7 @@ stdenv.mkDerivation {

  patches =
    copyPathsToStore (lib.readPathsFromFile ./. ./series)
-    ++ stdenv.lib.optional stdenv.isDarwin ./darwin-cf.patch;
+    ++ stdenv.lib.optional stdenv.isDarwin (lib.readPathsFromFile ./. ./darwin-series);


This doesn't seem to work correctly. I don't really get why it's done like that for the other patches either, what's the advantage over just a list of paths?

applying patch /nix/store/bz3c8wjkkippnb60hcqydklkpx6jngrm-git-export/pkgs/development/libraries/qt-5/5.9/qtbase/mkspecs-common-mac.patch /nix/store/wh1ck71iabaw743xnf1j3aj9y8wh9jv3-stdenv-darwin/setup: line 740: /nix/store/bz3c8wjkkippnb60hcqydklkpx6jngrm-git-export/pkgs/development/libraries/qt-5/5.9/qtbase/mkspecs-common-mac.patch: No such file or directory builder for ‘/nix/store/wbpddvjbjm993ffmcl5h2v7z3bfkhbm4-qtbase-5.9.1.drv’ failed with exit code 1

I don't really get why it's done like that for the other patches either, what's the advantage over just a list of paths?

I can point quilt at that list of patches and work with them on top of the unpacked sources (almost) as if it was Git.

Oh, I just follow the current way of qt* modules. They are using a list of patches even for just one patch. Like https://github.com/NixOS/nixpkgs/tree/master/pkgs/development/libraries/qt-5/5.9/qtdeclarative

@LnL7 are you sure you pulled in all the changes? Looks like the patch file is not in your test env

@j-hao You need to use copyPathsToStore as on line 82. This is probably broken for @LnL7 because he is using sandboxes, so his builder cannot access non-store paths.

Let me fix this

Ah yes, that must be it. The hydra builds would have the same problem.

LnL7 · 2017-09-26T20:34:53Z

pkgs/development/libraries/qt-5/5.9/qtbase/default.nix

@@ -45,7 +45,7 @@ stdenv.mkDerivation {
      libjpeg libpng libtiff
    ]

-    ++ lib.optional mesaSupported mesa
+    ++ lib.optional (mesaSupported && !stdenv.isDarwin) mesa


This already defaults to false on darwin.

On Darwin, the mesaSupported is passed in as true instead of using default value (false). That's the reason why qtdeclarative cannot build. If you look at output of qtbase configuration, the current result shows OpenVG is yes due to mesaSupported value as true.

periklis · 2017-09-27T07:43:03Z

pkgs/development/libraries/qt-5/5.9/qtbase/default.nix

+          substituteInPlace ./mkspecs/common/mac.conf \
+              --replace "/System/Library/Frameworks/OpenGL.framework/" "${darwin.apple_sdk.frameworks.OpenGL}/Library/Frameworks/OpenGL.framework/"
+          substituteInPlace ./mkspecs/common/mac.conf \
+              --replace "/System/Library/Frameworks/AGL.framework/" "${darwin.apple_sdk.frameworks.AGL}/Library/Frameworks/AGL.framework/"


Why is this not part of the patch file?

Can we know the actual ${darwin.apple_sdk.frameworks.AGL} path within patch file?

don't think so, but I think replacing paths like this preferable since it doesn't break as easily as a patch

Oh thanks for clarifying this. I missed the ${darwin...} substitutions.

btw. these two lines will probably enable qt5.qtwebkit, too.

Yes, I can build qt5.qtwebkit locally

j-hao · 2017-09-29T23:06:40Z

Updated new version. Let me know if anything need to change

LnL7 · 2017-10-01T13:44:13Z

All of the remaining failures are caused by a chmod now, including a dependency of qtdeclarative.

Moving /nix/store/pkw2bp04m7c9xywjwva365glihcwrxqg-qtsvg-5.9.1/lib/qt-5.9/plugins to /nix/store/0zzl2389avjd6nwz8q7cw88xmirhzja4-qtsvg-5.9.1-bin/lib/qt-5.9/plugins
Removing empty /nix/store/pkw2bp04m7c9xywjwva365glihcwrxqg-qtsvg-5.9.1/lib/qt-5.9/ and (possibly) its parents
chmod: changing permissions of '/nix/store/pkw2bp04m7c9xywjwva365glihcwrxqg-qtsvg-5.9.1/lib/cmake/Qt5/Qt5Config.cmake': Operation not permitted
builder for ‘/nix/store/6aq90l300875x6jaimcl9avsz31p1qxz-qtsvg-5.9.1.drv’ failed with exit code 1

j-hao · 2017-10-04T02:58:23Z

@LnL7 I'm having trouble to reproduce the error you saw. Can you let me know how to reproduce it?

LnL7 · 2017-10-04T17:09:55Z

@j-hao Hmm, maybe the minimal sandbox is only used when build user separation is enabled. Are you using the nix-daemon?

It's probably trying to make a setuid binary, that's not allowed in a build sandbox.

j-hao · 2017-10-11T08:54:34Z

@LnL7 the problem occurred if I use nix-daemon. I have switched to nix-daemon and tested the build. It can now build qt5.full for me. Please check

LnL7 · 2017-10-11T18:56:16Z

@j-hao Nice, everything builds now except for qtwebengine.

FRidh added 6.topic: darwin Running or building packages on Darwin 6.topic: qt/kde labels Sep 26, 2017

FRidh requested review from LnL7 and ttuegel September 26, 2017 16:05

ttuegel reviewed Sep 26, 2017

View reviewed changes

LnL7 reviewed Sep 26, 2017

View reviewed changes

LnL7 added the 9.needs: port to stable A PR needs a backport to the stable release. label Sep 26, 2017

periklis reviewed Sep 27, 2017

View reviewed changes

j-hao force-pushed the master branch from 4e7d376 to 50c75c2 Compare September 27, 2017 22:54

qt59.qtbase: fix darwin build so qt full can build

9dd335e

j-hao force-pushed the master branch from 50c75c2 to 9dd335e Compare October 11, 2017 08:52

LnL7 approved these changes Oct 11, 2017

View reviewed changes

LnL7 merged commit 0d8c6f4 into NixOS:master Oct 11, 2017

LnL7 added the 8.has: port to stable A PR already has a backport to the stable release. label Oct 11, 2017

samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qt59.qtbase: fix darwin build so qtdeclarative can build #29812

qt59.qtbase: fix darwin build so qtdeclarative can build #29812

j-hao commented Sep 26, 2017

ttuegel left a comment

LnL7 Sep 26, 2017

ttuegel Sep 26, 2017

j-hao Sep 27, 2017

ttuegel Sep 27, 2017

j-hao Sep 27, 2017

LnL7 Sep 27, 2017

LnL7 Sep 26, 2017

j-hao Sep 27, 2017

periklis Sep 27, 2017

j-hao Sep 27, 2017

LnL7 Sep 27, 2017

periklis Sep 28, 2017

periklis Sep 28, 2017

j-hao Sep 29, 2017

j-hao commented Sep 29, 2017

LnL7 commented Oct 1, 2017

j-hao commented Oct 4, 2017

LnL7 commented Oct 4, 2017 •

edited

j-hao commented Oct 11, 2017

LnL7 commented Oct 11, 2017

qt59.qtbase: fix darwin build so qtdeclarative can build #29812

qt59.qtbase: fix darwin build so qtdeclarative can build #29812

Conversation

j-hao commented Sep 26, 2017

Motivation for this change

Things done

ttuegel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-hao commented Sep 29, 2017

LnL7 commented Oct 1, 2017

j-hao commented Oct 4, 2017

LnL7 commented Oct 4, 2017 • edited

j-hao commented Oct 11, 2017

LnL7 commented Oct 11, 2017

LnL7 commented Oct 4, 2017 •

edited