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

qt5: fix build with Darwin sandbox enabled #234122

Merged
merged 3 commits into from
Jun 15, 2023

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented May 26, 2023

Description of changes

Qt requires access to the system ICU data due to its linking against the system CoreFoundation and invoking CF APIs that tries to access it. This manifests as a crash during build when it fails to access the data.

I’ve bundled a couple of fixes into this one since setting propagatedSandboxProfile on qt5.qtbase doesn’t appear to work.

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.05 Release Notes (or backporting 22.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
  • Fits CONTRIBUTING.md.

@reckenrode reckenrode requested a review from ttuegel as a code owner May 26, 2023 02:38
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label May 26, 2023
@ofborg ofborg bot requested review from periklis, bkchr, hrdinka and qknight May 26, 2023 03:45
@ofborg ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 501-1000 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 26, 2023
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

I'm failing to reproduce the failure on the staging commit this is based on. I don't have sandboxing enabled by default so I'm using this for testing:

nix-build --no-out-link --pure -A qtpass --option sandbox true --check

The --check is because there's a substitute available.
I suspect it's the sandboxing that's not working, rather than it not being a problem for the build. If I touch paths in my home directory from postPatch I still don't get a failure. I've tried with qt5.qtbase as well but the only error I get is --check failing because it's not deterministic apparently.

Is there a better way to test whether the sandboxing is active? And to reproduce the failure?

pkgs/applications/misc/qtpass/default.nix Outdated Show resolved Hide resolved
@reckenrode
Copy link
Contributor Author

I created #237458 for the propgatedSandboxProfile issue.

Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

So I'm failing to reproduce the issue entirely.
With the profiles the build works iff sandboxing is set to relaxed. Without them the build fails but due to a segmentation fault not obviously connected to accessing /usr/share/icu.

Console.app output right after the build, cleared beforehand.

Build log:

> nix-build --no-out-link --pure -
A qtpass --check                                                                checking outputs of '/nix/store/6lb3qdm1ig582bz5ln48ymp87n6lh176-qtpass-1.3.2.dr
v'...                                                                           
qmakePrePhase                                                                   qtPreHook                                                                       
unpacking sources                                                               unpacking source archive /nix/store/x1s95pdksjc8bd83wgwp2xm57yq32bbb-source     source root is source                                                           
patching sources                                                                
applying patch /nix/store/dpf7yjc547n8kb21diaiv7gm0l31gv05-qtpass-Dont-hardcode-
pass-otp-usr-lib-path.patch                                                     
patching file src/configdialog.cpp                                              
configuring                                                                     
QMAKEPATH=/nix/store/cx6n3v808k8wl7drdhvh34wnn5zplwqg-qtbase-5.15.9-dev:/nix/sto
re/jla2nk9l9h88spqr97pnak94rm51ivvj-qttools-5.15.9-dev:/nix/store/hnfwiykwaqaymv
y45b6749djhjy9hzfq-qtdeclarative-5.15.9-dev:/nix/store/p090cbbin6n7c0j27i7rkqkkv
r3ddgrp-qtsvg-5.15.9-dev                                                        
qmake PREFIX=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT
_OUT=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT_DEV=/ni
x/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT_BIN=/nix/store/
p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT_DOC=/nix/store/p32kavjg
mpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/share/doc/qt-5.15.9 NIX_OUTPUT_QML=/nix/st
ore/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/lib/qt-5.15.9/qml NIX_OUTPUT_P
LUGIN=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/lib/qt-5.15.9/plu
gins CONFIG+=release CONFIG+=nostrip QMAKE_LUPDATE=/nix/store/jla2nk9l9h88spqr97
pnak94rm51ivvj-qttools-5.15.9-dev/bin/lupdate QMAKE_LRELEASE=/nix/store/jla2nk9l
9h88spqr97pnak94rm51ivvj-qttools-5.15.9-dev/bin/lrelease                        
Info: creating stash file /private/tmp/nix-build-qtpass-1.3.2.drv-0/source/.qmak
e.stash                                                                         
sh: line 1: 24298 Segmentation fault: 11  /nix/store/jla2nk9l9h88spqr97pnak94rm5
1ivvj-qttools-5.15.9-dev/bin/lupdate -locations relative -no-ui-lines /private/t
mp/nix-build-qtpass-1.3.2.drv-0/source/qtpass.pro
sh: line 1: 24300 Segmentation fault: 11  /nix/store/jla2nk9l9h88spqr97pnak94rm5
1ivvj-qttools-5.15.9-dev/bin/lrelease /private/tmp/nix-build-qtpass-1.3.2.drv-0/
source/qtpass.pro
qmake: enabled parallel building                                                
qmake: enabled parallel installing
building                                                                        
build flags: -j1 SHELL=/nix/store/rp7p04lg2yqbca8r14bby68z01wbqj14-bash-5.2-p15/
bin/bash                                                                        
cd src/ && ( test -e Makefile || /nix/store/cx6n3v808k8wl7drdhvh34wnn5zplwqg-qtb
ase-5.15.9-dev/bin/qmake -o Makefile /private/tmp/nix-build-qtpass-1.3.2.drv-0/s
ource/src/src.pro PREFIX=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.
2 NIX_OUTPUT_OUT=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OU
TPUT_DEV=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT_BIN
=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2 NIX_OUTPUT_DOC=/nix/st
ore/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/share/doc/qt-5.15.9 NIX_OUTPUT
_QML=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/lib/qt-5.15.9/qml 
NIX_OUTPUT_PLUGIN=/nix/store/p32kavjgmpni826hjah7hv7aa0z2grl3-qtpass-1.3.2/lib/q
t-5.15.9/plugins CONFIG+=release CONFIG+=nostrip QMAKE_LUPDATE=/nix/store/jla2nk
9l9h88spqr97pnak94rm51ivvj-qttools-5.15.9-dev/bin/lupdate QMAKE_LRELEASE=/nix/st
ore/jla2nk9l9h88spqr97pnak94rm51ivvj-qttools-5.15.9-dev/bin/lrelease ) && make -
f Makefile                                                                      
make[1]: Entering directory '/private/tmp/nix-build-qtpass-1.3.2.drv-0/source/sr
c'                                                                              
/nix/store/jla2nk9l9h88spqr97pnak94rm51ivvj-qttools-5.15.9-dev/bin/lrelease ../l
ocalization/localization_nl_NL.ts -qm ../localization/localization_nl_NL.qm     
make[1]: *** [Makefile:574: ../localization/localization_nl_NL.qm] Segmentation 
fault: 11                                                                       
make[1]: Leaving directory '/private/tmp/nix-build-qtpass-1.3.2.drv-0/source/src
'                                       
make: *** [Makefile:52: sub-src-make_first] Error 2

This is on 255911fcb9a8 (HEAD) Merge staging-next into staging.

Another thing I ran into is that Nix apparently doesn't allow profiles when sandboxing is set to true. Seems like a questionable design decision but it also seems like it prevents us from fixing the Qt build in that case?

> nix-build --no-out-link --pure -A qtpass --check
error: derivation '/nix/store/jap1jxm4psgpfa1jmph9lsf4fyrhk4ki-qtpass-1.3.2.drv' specifies a sandbox profile, but this is only allowed when 'sandbox' is 'relaxed'

@@ -84,6 +84,12 @@ stdenv.mkDerivation (finalAttrs: {

propagatedNativeBuildInputs = [ lndir ];

# libQt5Core links calls CoreFoundation APIs that call into the system ICU. Binaries linked
# against it will crash during build unless they can access `/usr/share/icu/icudtXXl.dat`.
sandboxProfile = lib.optionalString stdenv.isDarwin ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use propagatedSandboxProfile despite the fact that it doesn't work? Since it still signals intent?

@reckenrode
Copy link
Contributor Author

reckenrode commented Jun 13, 2023

Let me know if I should clarify the commit message. The segmentation fault is what happens when it can’t access the ICU data. That’s odd the sandbox denial is not showing up in your logs.

As for not building when sandbox = true, there doesn’t seem to be any fix for that. Qt links against the system CoreFoundation, which may need to access system ICU data. Unfortunately, there’s no way around that.

Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

Realized the comments aren't super clear on where propagatedSandboxProfile doesn't work.

pkgs/applications/misc/qtpass/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/qtkeychain/default.nix Outdated Show resolved Hide resolved
@reckenrode reckenrode force-pushed the qt-darwin-sandbox branch 2 times, most recently from b57b056 to 3c0d333 Compare June 14, 2023 00:52
@reckenrode
Copy link
Contributor Author

@toonn I added your changes and updated the commit message to explain how the problem manifests.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 14, 2023
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

One more thing : )

pkgs/applications/misc/qtpass/default.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

LGTM

Qt requires access to the system ICU data due to its linking against
the system CoreFoundation and invoking CF APIs that tries to access it.
This manifests as a crash during build when it fails to access the data.
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 14, 2023
Co-authored-by: toonn <toonn@toonn.io>
Co-authored-by: toonn <toonn@toonn.io>
@reckenrode
Copy link
Contributor Author

Latest push should fix the evaluation error.

@toonn toonn merged commit c1fa08f into NixOS:staging Jun 15, 2023
@reckenrode reckenrode mentioned this pull request Jul 16, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: qt/kde 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants