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

qt-5: add buildPackages.stdenv.cc to nativeBuildInputs #220310

Closed
wants to merge 5 commits into from
Closed

qt-5: add buildPackages.stdenv.cc to nativeBuildInputs #220310

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 9, 2023

Description of changes

Because qt-5 uses lib.newScope rather than lib.newScopeWithSplicing, the splicing mechanism for cross compilation does not appear to work correctly.

Rather than risk breakage on native compiles, let's just choose the buildPackages version manually for perl and the host cc (which is needed for qmake). This commit does that by adding them to nativeBuildInputs in qtModule.nix and modules/qtbase.nix.

@ghost ghost requested a review from ttuegel as a code owner March 9, 2023 09:43
@ghost ghost mentioned this pull request Mar 9, 2023
@ofborg ofborg bot added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Mar 9, 2023
@Artturin
Copy link
Member

Artturin commented Mar 9, 2023

this isn't related to newScope, instead the issue is that the inherits in all-packages aren't inherited from __splicedPackages

change

qt5 = recurseIntoAttrs (makeOverridable
  (import ../development/libraries/qt-5/5.15) {
    inherit newScope;
    inherit lib fetchurl fetchpatch fetchgit fetchFromGitHub makeSetupHook makeWrapper;
    inherit bison cups dconf harfbuzz libGL perl gtk3 python3;
    inherit (gst_all_1) gstreamer gst-plugins-base;
    inherit darwin;
    inherit buildPackages;
    stdenv = if stdenv.isDarwin then darwin.apple_sdk_11_0.stdenv else stdenv;
  });

to

  qt5 = recurseIntoAttrs (makeOverridable
    (import ../development/libraries/qt-5/5.15) {
      inherit (__splicedPackages)
        newScope lib fetchurl fetchpatch fetchgit fetchFromGitHub makeSetupHook makeWrapper
        bison cups dconf harfbuzz libGL perl gtk3 python3
        darwin buildPackages;
      inherit (__splicedPackages.gst_all_1) gstreamer gst-plugins-base;
      stdenv = if stdenv.isDarwin then darwin.apple_sdk_11_0.stdenv else stdenv;
    });

@Artturin
Copy link
Member

Artturin commented Mar 9, 2023

#220374 inherit __splicedPackages + makeScopeWithSplicing

@ghost
Copy link
Author

ghost commented Mar 9, 2023

Rebased on top of #220374

This allowed to drop the buildPackages. prefix from perl. The addition of buildPackages.stdenv.cc is still necessary however.

@ghost ghost marked this pull request as ready for review March 9, 2023 21:24
@ghost ghost changed the title qt-5: nativeBuildInputs for cross compilation qt-5: add buildPackages.stdenv.cc to nativeBuildInputs Mar 9, 2023
@Artturin
Copy link
Member

Artturin commented Mar 9, 2023

adding buildPackages.stdenv.cc to nativeBuildInputs looks wrong, investigating..

@Artturin
Copy link
Member

Artturin commented Mar 9, 2023

qmake/Makefile (which is generated) has the wrong CC and CXX variables (non prefixed binaries)

also for ref here's the qt5base build file for build root
https://github.com/buildroot/buildroot/blob/master/package/qt5/qt5base/qt5base.mk
i recommend cloning that repository
here's a couple more cross focused repos
https://github.com/openembedded/openembedded-core
https://git.yoctoproject.org/poky

@ghost ghost marked this pull request as draft March 10, 2023 05:44
@ghost
Copy link
Author

ghost commented Mar 10, 2023

qmake/Makefile (which is generated) has the wrong CC and CXX variables (non prefixed binaries)

Is that wrong? qmake runs on the buildPlatform (counterintuitively).

In theory in order to fit nixpkgs way of thinking, qmake ought to be in its own derivation separate from qtbase. But that would be a pretty huge undertaking for a library that is no longer actively developed upstream...

Edit: or we could think of qmake as being a compiler, and give it a targetPlatform (so qmake's (build==host)!=target)

@ghost ghost marked this pull request as ready for review March 10, 2023 06:31
@ghost
Copy link
Author

ghost commented Mar 10, 2023

i recommend cloning that repository here's a couple more cross focused repos

Void is another good one.

Unfortunately I'm trying to do this without totally rewriting our qt5 expressions. I think the recipes from other distros would be more useful in a rewrite-from-scratch (which I am not volunteering for).

Qmake expects to be able to find the host's C compiler in the $PATH
when cross compiling.  Let's add it.

Co-authored-by: superherointj <5861043+superherointj@users.noreply.github.com>
@Artturin
Copy link
Member

Artturin commented Mar 10, 2023

qmake/Makefile (which is generated) has the wrong CC and CXX variables (non prefixed binaries)

Is that wrong? qmake runs on the buildPlatform (counterintuitively).

In theory in order to fit nixpkgs way of thinking, qmake ought to be in its own derivation separate from qtbase. But that would be a pretty huge undertaking for a library that is no longer actively developed upstream...

Edit: or we could think of qmake as being a compiler, and give it a targetPlatform (so qmake's (build==host)!=target)

pkgsCross.aarch64-multiplatform.buildPackages.qt5.qtbase (nativeBuildInputs) already builds, while this is making it possible to build pkgsCross.aarch64-multiplatform.qt5.qtbase(buildInputs) from which binaries won't be run on buildPlatform so using the non-prefixd binaries to build it seems like a bug

adding buildPackages.stdenv.cc to qtModule is especially wrong

even if a build build compiler was needed then buildPackages.stdenv.cc should go in to depsBuildBuild

pkgs/development/libraries/qt-5/5.15/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/qt-5/5.15/default.nix Outdated Show resolved Hide resolved
@@ -77,7 +79,11 @@ stdenv.mkDerivation (finalAttrs: {
++ lib.optional (postgresql != null) postgresql;

nativeBuildInputs = [ bison flex gperf lndir perl pkg-config which ]
++ lib.optionals stdenv.isDarwin [ xcbuild ];
++ lib.optionals stdenv.isDarwin [ xcbuild ]
# `qtbase` expects to find `cc` (with no prefix) in the
Copy link
Member

Choose a reason for hiding this comment

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

Can we patch that?

Why do we need this in the wrapper and package?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, there's a lot of ugliness in nixpkgs' qt5 expressions. But I am trying really hard not to undertake a rewrite-from-scratch of them here.

pkgs/development/libraries/qt-5/qtModule.nix Outdated Show resolved Hide resolved
Adam Joseph and others added 4 commits March 21, 2023 02:37
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
As recommended here:
#220310 (comment)

The non-idiomatic `if..then..else null` is to prevent a mass-rebuild
sending this to staging.  I can submit a separate PR to staging to
change it to the `lib.optional` idiom if this bothers people.
@ghost
Copy link
Author

ghost commented Mar 21, 2023

pkgsCross.aarch64-multiplatform.buildPackages.qt5.qtbase (nativeBuildInputs) already builds, while this is making it possible to build pkgsCross.aarch64-multiplatform.qt5.qtbase(buildInputs) from which binaries won't be run on buildPlatform so using the non-prefixd binaries to build it seems like a bug

I'm having trouble understanding this, except for the last bit about prefixes, which I disagree with: the fact that native compilers are unprefixed is a bug in nixpkgs (which is painful to fix) not a feature.

adding buildPackages.stdenv.cc to qtModule is especially wrong

even if a build build compiler was needed then buildPackages.stdenv.cc should go in to depsBuildBuild

Since qtbase has no meaningful targetPlatform that's a bit of a nitpick, but okay: ceb4d99

@ghost ghost closed this Apr 24, 2023
@ghost ghost deleted the pr/qt5/cross/nativeBuildInputs branch April 24, 2023 02:39
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: qt/kde 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants