qt5: fix cross · Pull Request #264965 · NixOS/nixpkgs

ghost · 2023-11-02T07:39:57Z

Reengineered to avoid rebuilding anything. Should have ~0 rebuilds.

To be useful, you also need this two-commit PR from staging (which does cause rebuilds; that's why it's a separate PR):

qt5.qtbase: ignore hook propagated from buildPlatform #264964

Description of changes

This commit fixes builds of pkgsCross.*.qt5.qtbase by:

Adding the buildPlatform compiler to depsBuildBuild in qtbase.nix and qtModule.nix. The qtbase build machinery expects to find it in the $PATH in unprefixed form.
Setting the PKG_CONFIG_SYSROOT_DIR and PKG_CONFIG_LIBDIR environment variables when compiling a cross-targeted qmake. This is required; if these environment variables are unset, qmake won't even try to use pkg-config.
Adding the -device and -device-option flags necessary for cross compilation to configureFlags.
Adding the (one-entry at the moment) Rosetta Stone for QT-5 as a let-defined qtPlatform function which takes a nixpkgs platform and returns a QT-5 mkspecs-string.

Includes

Things done

Artturin · 2023-11-02T08:09:44Z

pkgs/development/python-modules/pyqtwebengine/default.nix

Only the /include is added , not any sub paths IIRC

Many of the includes in https://github.com/search?q=%2F%23Include.%2BQtWebChannel%2F&type=code have a / so I don't know if they're supposed to be added to it.

pkgs/applications/networking/browsers/qutebrowser/default.nix

pkgs/development/python-modules/pyqtwebengine/default.nix

pkgs/development/libraries/qt-5/modules/qtbase.nix

pkgs/development/libraries/qt-5/5.15/default.nix

ghost · 2023-11-03T05:08:03Z

@ofborg build pkgsCross.aarch64-multiplatform.qt5.qutebrowser

Our qutebrowser package has been vandalized by various commits, such as c9cc3a2 and ad0bbaf, which made erroneous assertions such as "since qutebrowser 3.0.0 the derivation is only building for qt6." This commit repairs the vandalism.

…compiling

…ldInputs if cross compiling

ghost · 2023-11-05T00:11:59Z

@ofborg eval

pkgs/applications/networking/browsers/qutebrowser/default.nix

ghost · 2023-11-05T00:54:57Z

This works; along with #264964:

pkgsCross.aarch64-multiplatform.qt5.qutebrowser on x86_64-linux — Success

It causes no rebuilds:

There are a lot of conditionals; most can be removed, but doing so causes rebuilds, so the conditional-removals must go to staging.

I'm incredibly tired of rebasing this.

I'm merging it.

Maybe somebody will rage-revert it, maybe not.

In either case I'll stay on the branch (local if necessary) that keeps it in-tree, because it's kind of insane to use a distribution with a browser-only forge that repeatedly breaks your web browser. If I can stop spending all my time rebasing this I might actually be able to finish getting qt6 to cross-compile after 23.11:

dotlambda · 2023-11-05T01:07:27Z

Why did you neither implement my suggestion nor wait for other reviews?

winterqt · 2023-11-05T02:38:20Z

Maybe somebody will rage-revert it, maybe not.

This is possibly the most self-aware yet utterly absurd comment I've seen here to date.

A gentle reminder that we are a team, and you are expected to work with others to meet our shared goals; completely blindsiding the active reviewers goes against that entirely.

pkgs/development/libraries/qt-5/modules/qtdeclarative.nix

pkgs/development/python-modules/pyqtwebengine/default.nix

Artturin · 2023-11-05T03:02:26Z

pkgs/development/libraries/qt-5/qtModule.nix

+    (args.nativeBuildInputs or []) ++ [
+      perl qmake
+    ] ++ lib.optionals (stdenv.buildPlatform != stdenv.hostPlatform) [
+      pkgsHostTarget.qt5.qtbase.dev


why a buildInput in nativeBuildInputs, the programs there shouldn't be executable?

Normally no, except it's QT, and QT has both qmake-which-runs-on-the-buildPlatform as well as libQtBase-which-is-built-for-the-hostPlatform mashed into the same package.

pkgs/development/python-modules/pyqt/5.x.nix

Artturin · 2023-11-05T04:13:55Z

pkgs/development/libraries/qt-5/5.15/default.nix

-        propagatedBuildInputs = [ qtbase.dev ];
+        ${if stdenv.buildPlatform == stdenv.hostPlatform
+          then "propagatedBuildInputs"
+          else "depsTargetTargetPropagated"} = [ qtbase.dev ];


Seems wrong, the binaries in qtbase won't be in PATH

Remember, qtbase is both a build tool (qmake) and a library (libQt5Base). The line above deals with the library.

Are you sure it deals with only the library? https://github.com/nixos/nixpkgs/blob/753bf0e2933c4d66c0eb8481b60157419bae3629/pkgs/development/libraries/qt-5/hooks/qmake-hook.sh#L37

I> Normally no, except it's QT, and QT has both qmake-which-runs-on-the-buildPlatform as well as libQtBase-which-is-built-for-the-hostPlatform mashed into the same package.

Gotta add comments, really confusing

Artturin · 2023-11-05T04:25:10Z

github removed all my nixpkgs notifications and unsubscribed me from all the nixpkgs PRs and issues (4k+) 😠 so ping directly in other PRs which i haven't been in since ~Friday, November 3rd, 2023 at 10:37 PM

https://discourse.nixos.org/t/github-unsubscribed-me-from-all-nixpkgs-prs-and-issues/35026

rnhmjoj · 2023-11-05T16:52:16Z

You sneaked controversial changes (the qutebrowser stuff) in an unrelated PR and self-merged them without waiting for anyone to review. I think this behavior is borderline malicious and I seriously suggest that your commit rights be revoked.

winterqt · 2023-11-05T17:21:17Z

You sneaked controversial changes (the qutebrowser stuff) in an unrelated PR

At least the merged change didn't make it so that any usage of qutebrowser would be downgraded to an insecure version, like a previous version of that PR apparently did. 😕

ghost · 2023-11-06T00:21:54Z

You sneaked controversial changes (the qutebrowser stuff) in

I think you are confused here.

It was in fact you who sneaked controversial changes to qutebrowser in #250171 which deleted qt5.qutebrowser (the only one that builds under many configurations) into a version-bump PR titled "qutebrowser: 2.5.4 -> 3.0.0".

nrdxp · 2023-11-06T00:50:54Z

It was in fact you who sneaked controversial changes to qutebrowser

Who, besides you, agrees that the upgrade to qt6 was controversial? Certainly not upstream. In fact, it is often considered controversial to break from upstream packaging norms in any serious manner, which means you have the burden of proof here, not us.

In addition, I think most users were anxious for the qt6 version, seeing as the qt5 webengine was already severely outdated by the time qt6 support was made stable in qutebrowser. I would bet that there would be a much larger popullation of confused users if qt6 wasn't made the default as soon as it was stable.

Besides that, it is just plain subversive to sneak in changes like this when maintainers don't agree with you, especially if you don't take a share of the maintenance burden by adding yourself as a maintainer.

ghost · 2023-11-06T01:21:01Z

Who, besides you, agrees that the upgrade to qt6 was controversial?

Please don't put words in my mouth.

Take a moment to read the PR that I linked to. Before that PR qutebrowser supported both qt5 and qt6. I have no objection to the addition of support for qt6.

dotlambda · 2023-11-07T15:40:04Z

pkgs/development/libraries/qt-5/5.15/default.nix

+      if stdenv.buildPlatform == stdenv.hostPlatform
+      then bootstrapScope.qttranslations
+      else null;
+    qutebrowser = final.callPackage ../../../../applications/networking/browsers/qutebrowser { };


Applications don't belong here so I suggest #251671.

NixOS#264965 (comment)

github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: qt/kde Object-oriented framework for GUI creation labels Nov 2, 2023

ghost mentioned this pull request Nov 2, 2023

qt5: fix splicing #264903

Merged

13 tasks

Artturin reviewed Nov 2, 2023

View reviewed changes

ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Nov 2, 2023

github-actions bot added the 6.topic: lib The Nixpkgs function library label Nov 2, 2023

ofborg bot requested review from OPNA2608, abbradar, bkchr, matthewbauer, periklis, qknight, svanderburg and ttuegel November 2, 2023 13:19

Artturin reviewed Nov 3, 2023

View reviewed changes

pkgs/applications/networking/browsers/qutebrowser/default.nix Outdated Show resolved Hide resolved

Artturin reviewed Nov 3, 2023

View reviewed changes

pkgs/development/python-modules/pyqtwebengine/default.nix Outdated Show resolved Hide resolved

Artturin reviewed Nov 3, 2023

View reviewed changes

pkgs/development/libraries/qt-5/modules/qtbase.nix Outdated Show resolved Hide resolved

Artturin reviewed Nov 3, 2023

View reviewed changes

pkgs/development/libraries/qt-5/5.15/default.nix Outdated Show resolved Hide resolved

ofborg bot added the 8.has: package (new) This PR adds a new package label Nov 3, 2023

Adam Joseph added 6 commits November 3, 2023 22:28

qutebrowser: repair vandalism

0e3b820

Our qutebrowser package has been vandalized by various commits, such as c9cc3a2 and ad0bbaf, which made erroneous assertions such as "since qutebrowser 3.0.0 the derivation is only building for qt6." This commit repairs the vandalism.

lib.systems.inspect: add patternLogicalAnd

1048c90

qt5.qtdeclarative: add postFixup if cross compiling

4ef828d

qt5.qtModule: reformat arguments

760d596

qt5.qtModule: add buildPackages.stdenv.cc to depsBuildBuild if cross …

e42b965

…compiling

qt5.qtModule: add explicit pkgsHostTarget.qt5.qtbase.dev to nativeBui…

129b22f

…ldInputs if cross compiling

Adam Joseph added 2 commits November 4, 2023 15:58

qutebrowser: take python3 from buildPackages

274da99

test.cross.sanity: add qt5.qutebrowser, firefox

753bf0e

dotlambda reviewed Nov 5, 2023

View reviewed changes

pkgs/applications/networking/browsers/qutebrowser/default.nix Show resolved Hide resolved

ofborg bot requested a review from dotlambda November 5, 2023 00:37

ghost merged commit 510397a into NixOS:master Nov 5, 2023

ghost deleted the qt5-fix-cross branch November 5, 2023 00:55

Artturin reviewed Nov 5, 2023

View reviewed changes

pkgs/development/libraries/qt-5/modules/qtdeclarative.nix Show resolved Hide resolved

Artturin reviewed Nov 5, 2023

View reviewed changes

pkgs/development/python-modules/pyqtwebengine/default.nix Show resolved Hide resolved

Artturin reviewed Nov 5, 2023

View reviewed changes

pkgs/development/python-modules/pyqt/5.x.nix Show resolved Hide resolved

Artturin reviewed Nov 5, 2023

View reviewed changes

This was referenced Nov 5, 2023

pyqtwebengine: use python.pythonOnBuildForHost #265570

Merged

qt5: remove most cross conditionals #265573

Closed

qtdeclarative: remove unnecessary postFixup #265583

Merged

dotlambda mentioned this pull request Nov 5, 2023

qt5.qutebrowser: reinit at 3.0.2 #265364

Closed

13 tasks

dotlambda mentioned this pull request Nov 6, 2023

qutebrowser: cleanup #265757

Merged

13 tasks

dotlambda reviewed Nov 7, 2023

View reviewed changes

nyabinary pushed a commit to nyabinary/nixpkgs that referenced this pull request Nov 14, 2023

qtdeclarative: remove unnecessary postFixup

665b2be

NixOS#264965 (comment)

Artturin mentioned this pull request Nov 22, 2023

vala: look for files in targetOffset #267550

Merged

13 tasks

Artturin mentioned this pull request Jan 29, 2024

List of @amjoseph-nixpkgs Pull requests before account removal #284796

Open

This pull request was closed.

Uh oh!

Conversation

ghost commented Nov 2, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Artturin Nov 2, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Nov 3, 2023

Uh oh!

ghost commented Nov 5, 2023

Uh oh!

Uh oh!

ghost commented Nov 5, 2023

Uh oh!

dotlambda commented Nov 5, 2023

Uh oh!

winterqt commented Nov 5, 2023

Uh oh!

Uh oh!

Uh oh!

Artturin Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Nov 5, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Artturin Nov 5, 2023

Choose a reason for hiding this comment

Uh oh!

ghost Nov 5, 2023

Choose a reason for hiding this comment

Uh oh!

Artturin Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Artturin Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Artturin commented Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rnhmjoj commented Nov 5, 2023

Uh oh!

winterqt commented Nov 5, 2023

Uh oh!

ghost commented Nov 6, 2023

Uh oh!

nrdxp commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Nov 6, 2023

Uh oh!

dotlambda Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

ghost commented Nov 2, 2023 •

edited by ghost

Loading

Artturin Nov 5, 2023 •

edited

Loading

Artturin Nov 5, 2023 •

edited

Loading

Artturin Nov 5, 2023 •

edited

Loading

Artturin commented Nov 5, 2023 •

edited

Loading

nrdxp commented Nov 6, 2023 •

edited

Loading