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: cross compile with a clean separation of build and host binaries #267311

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

eryngion
Copy link

@eryngion eryngion commented Nov 13, 2023

Description of changes

By default when cross compiling QT builds all binaries that are marked in their .pro files with option(host_build) for the build platform instead of the host platform, and those binaries end up in the dev outputs (maybe not only there) of packages intended for the host platform. While there is some expectation of this situation in Nix [1], it prevents us from easily swapping natively and cross built dependencies, i.e. "lazy cross-compiling" and the host platform doesn't get the full set of binaries.

Here I'm attempting to have only host-targeted binaries in the cross-compiled packages and somehow live with this new situation. Even if this approach fails, I have here a couple of fixes that can helpful elsewhere.

I'll direct this PR to staging when we will be done discussing it.

[1] patchShebangs expects that binaries in dev outputs will be run on the build platform, and propagatedNativeBuildInputs, being implemented as an in-store dependency, essentially adds the same hard dependency on the platform where the package was built.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux (cross)
    • 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.

@@ -56,7 +52,7 @@ qmakePathHook() {
QMAKEPATH="${QMAKEPATH}${QMAKEPATH:+:}$1"
fi
}
envBuildHostHooks+=(qmakePathHook)
envHostTargetHooks+=(qmakePathHook)
Copy link
Author

Choose a reason for hiding this comment

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

Maybe qmakePathHook should be run for both types of packages? Not sure yet.

@eryngion
Copy link
Author

@amjoseph-nixpkgs @Artturin

@FliegendeWurst FliegendeWurst added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Nov 13, 2023
eryngion and others added 20 commits May 28, 2024 15:04
Not sure what was the problem here, but as of next commit
qtbase successfully builds with `environment.noXlibs = true`
and qttranslations.

This reverts commit 2e14d8b.
Add to qtbase support for building a small bootstrap package.

Move qmake and other tools from qtbase.dev to qtbase.qmake
to avoid pulling in a second version of qtbase.dev in cross builds.

Use hidden bootstrap packages for qtbase and qttools instead of
overrideScope-based staging process to solve some spurious builds
and infinite recursions.

This commit by itself will allow to cross compile only qttranslations
and qtbase.
Also teach the setup hook, qmake and cmake scripts to search various
build tools in PATH and use them for cross compilation.

This will allow to cross compile packages that require only qtbase and
qmake.
… a native-built one

For this we need to dynamically detect cross compilation in qtbase-setup-hook.sh
and in mkspecs, and stop unconditionally using binary lndir in propagatedNativeBuildInputs.

For now qtbase substitution will work only in one (most used) direction, and in many cases
will additionally require to manually add a buildHost version of qtbase.qmake to
nativeBuildInputs of the package that depends on qtbase.
Without it qtwebchannel will fail to find some library includes,
qtquickcontrols will refuse to build without properly failing the
build:
```
Some of the required modules (qtHaveModule(quick)) are not available.
Skipped.
```
And there are probably more problems, not always obvious.
It needs only qtdeclarative to build, not qtquickcontrols.
If you're a nativeBuildInput somewhere, you probably shouldn't propagate
qtbase.
…s with qttools as a buildInput

And don't propagate qtbase to avoid pulling in a second qtbase.dev in
cross builds.
The sources in tarbals are identical, and hyphen from pkgs properly supports cross compilation.
It will use the bundled woff2 lib if we don't provide a system one.
And "DWARF optimization and duplicate removal tool" is just nice to
have, I guess.
Not sure why it was done.

This reverts commit 49e8d9d.
I expect here that qtwebengine will propagate qtwebchannel and qtdeclarative and that
qtdeclarative will propagate itself as a nativeBuildInput. But maybe we
shouldn't rely on those propagations?
Rely on callPackage whenever possible. And pass a spliced version of
stdenv to fix cross compilaton for qtwebengine.
It never worked and it's just an attempt to duplicate the logic
from qmake (i.e. QMAKEPATH -> QMAKEMODULES translation) that qmake
does all by itself.
@Artturin Artturin force-pushed the qt5-cross-with-a-clean-native-split branch from a5ebde0 to 27353f1 Compare May 28, 2024 12:32
@Artturin
Copy link
Member

Rebased.

@ofborg ofborg bot requested a review from cpages May 28, 2024 13:28
@eryngion
Copy link
Author

eryngion commented Jun 4, 2024

detected

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.

6 participants