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.qtbase: ignore hook propagated from buildPlatform #264964

Closed
wants to merge 4 commits into from
Closed

qt5.qtbase: ignore hook propagated from buildPlatform #264964

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 2, 2023

Includes

Description of changes

The main headache with cross-compiling QT is that our QT derivations go bananas with propagatedInputs. Unfortunately this means that both buildPackages.qtbase and pkgs.qtbase get propagated into the dependencies of every cross-compiled build, which triggers the hook check:

if [[ -n "${__nix_qtbase-}" ]]; then
# Throw an error if a different version of Qt was already set up.
if [[ "$__nix_qtbase" != "@dev@" ]]; then
echo >&2 "Error: detected mismatched Qt dependencies:"
echo >&2 " @dev@"
echo >&2 " $__nix_qtbase"
exit 1
fi
else # Only set up Qt once.

Previously in #227900 I simply disabled the check outright for cross-compiled builds. This is a more refined solution, where we skip the hook if has a different stdenv.outPath than the derivation being built.

This works because the outpath of stdenv is written into the environment of every derivation. There's nothing else we can use that's guaranteed to be in every derivation that qtbase-setup-hook.sh is propagated into.

We should stop using propagatedBuildInputs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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.

@ghost ghost marked this pull request as ready for review November 3, 2023 02:54
@ghost ghost requested a review from ttuegel as a code owner November 3, 2023 02:54
@ghost ghost requested a review from Artturin November 3, 2023 02:54
@ghost ghost marked this pull request as draft November 4, 2023 01:07
@ghost ghost marked this pull request as ready for review November 4, 2023 05:46
@ofborg ofborg bot requested review from bkchr, qknight and periklis November 4, 2023 06:05
@ghost
Copy link
Author

ghost commented Nov 4, 2023

error: building of '/nix/store/73fp0qc16cfbn2ddhfxg9b564zix7242-llvm-16.0.6.drv!dev' from .drv file timed out after 3600 seconds

@ghost
Copy link
Author

ghost commented Nov 4, 2023

Rebased, maybe this time LLVM won't time out.

@ghost ghost marked this pull request as draft November 12, 2023 00:09
Artturin and others added 4 commits November 11, 2023 16:12
broken by "qt5.wrapQtAppsHook: move libs to depsTargetTargetPropagated"
qtbase propagates a lot of stuff

frescobaldi> msgcat -o - frescobaldi/cs.po userguide/cs.po | msgfmt -o ../frescobaldi_app/i18n/cs.mo -
frescobaldi> sh: line 1: msgcat: command not found
frescobaldi> sh: line 1: msgfmt: command not found
…denvs

The main headache with cross-compiling QT is that our QT derivations
go bananas with propagatedInputs.  Unfortunately this means that
both buildPackages.qtbase and hostPackages.qtbase get propagated
into the dependencies of every cross-compiled builds, which triggers
the hook check.

Previously in #227900 I simply
disabled the check outright for cross-compiled builds.  This is a
more refined solution, where we skip the hook if it propagates
across a platform change.
@ghost ghost marked this pull request as ready for review November 12, 2023 00:12
@@ -353,7 +351,8 @@ let

wrapQtAppsHook = callPackage ({ makeBinaryWrapper, qtbase, qtwayland }: makeSetupHook {
name = "wrap-qt5-apps-hook";
propagatedBuildInputs = [ qtbase.dev makeBinaryWrapper ]
propagatedBuildInputs = [ makeBinaryWrapper ];
depsTargetTargetPropagated = [ qtbase.dev ]
Copy link
Member

Choose a reason for hiding this comment

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

what about the build tools?

@@ -391,6 +391,7 @@ stdenv.mkDerivation (finalAttrs: ({
dontStrip = debugSymbols;

setupHook = ../hooks/qtbase-setup-hook.sh;
env.qtbase_stdenv = stdenv.outPath;
Copy link
Member

@Artturin Artturin Nov 25, 2023

Choose a reason for hiding this comment

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

This overrides env, has to be moved to the env block

qtbase-aarch64-unknown-linux-gnu> qxcbcursor.cpp: In constructor 'QXcbCursor::QXcbCursor(QXcbConnection*, QXcbScreen*)':
qtbase-aarch64-unknown-linux-gnu> qxcbcursor.cpp:320:43: error: 'NIXPKGS_LIBXCURSOR' was not declared in this scope
qtbase-aarch64-unknown-linux-gnu>   320 |         QLibrary xcursorLib(QLatin1String(NIXPKGS_LIBXCURSOR), 1);
qtbase-aarch64-unknown-linux-gnu>       |                                           ^~~~~~~~~~~~~~~~~~

@ghost ghost closed this Jan 23, 2024
@ghost ghost deleted the qt5-skip-hook-propagated-across-platform-boundary branch January 23, 2024 06:51
@Artturin Artturin mentioned this pull request May 26, 2024
13 tasks
This pull request was closed.
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.

1 participant