WIP: wrapAppsHook: unify wrapper generation Gapps/Python/Qt/... #102949

FRidh · 2020-11-05T17:48:24Z

Motivation for this change

We have several hooks that generate wrappers. When multiple of these are used in a build, this results in nested wrappers. Typically, we disable the wrapper generation of all but one of the hooks then, but this is a manual step.

With this change, a new hook is created that is propagated by the other hooks (GApps/Python/Qt). These hooks export to an environment variable of their own, but also to a common variable which the new hook will then use.

This is a prerequisite to go to declarative wrappers as well.

Things done

TODO

This is not functioning yet.

Agree about the name of the hook and the environment variable that is used
Qt used to not wrap scripts. This hook no longer distinguishes between scripts and binaries. OK?
Python. Not implemented yet. Needs WIP: Python: wrap and patch using requiredPythonModules instead of propagatedBuildInputs #102613
Tests
GApps. Needs testing. GApps hook has tests.
Update docs
release notes

Long ago we used pth files for finding Python dependencies. Around the time we changed to the wheel builder that feature was no longer used.

wrapPythonProgramsIn both wraps and patches. Long-term, we should separate this further.

This is where all Python-related hooks can be found.

instead of requiring the user to always invoke `wrapPythonPrograms` manually.

… variable In Nixpkgs we have multiple hooks that generate wrappers using `makeWrapper`. This more than occasionally results in nested wrappers, because multiple hooks invoked `makeWrapper`. This commit introduces a new hook, `wrapAppsHook`, that solves the nested wrapper issue and deduplicates some logic. This hook invokes `makeWrapper` to build wrappers and loads its arguments from `appsWrapperArgs`. Other hooks that create wrappers should export to this environment variable and propagate this hook.

First need to switch from propagatedBuildInputs to requiredPythonModules.

FRidh · 2020-11-05T17:54:07Z

pkgs/build-support/setup-hooks/wrap-apps-hook/wrap-apps-hook.sh

@@ -0,0 +1,79 @@
+# shellcheck shell=bash
+appsWrapperArgs=()


Do we want to use this variable, or something else? Originally I had makeWrapperArgs, but that will conflict with the current (rare) usage of makeWrapperArgs of Python packages.

FRidh · 2020-11-05T17:55:02Z

pkgs/build-support/setup-hooks/wrap-apps-hook/wrap-apps-hook.sh

+            # TODO: further checks
+            # Following is done e.g. for Qt
+            #
+            # patchelf --print-interpreter "$file" >/dev/null 2>&1 || continue


This is the Qt check I mentioned. In my opinion we should wrap scripts as well. If there are scripts or simply executables to exclude, we should consider supporting an include and exclude list.

Yup, we should wrap those too – we had issues where PyQt programs were not properly wrapped.

FRidh · 2020-11-05T17:55:59Z

pkgs/development/interpreters/python/hooks/python-patch-executables-hook.sh

@@ -0,0 +1,11 @@
+# # Patch Python executables


wrapPython does both patching and wrapping. Ideally that is split into two.

FRidh · 2020-11-05T17:56:46Z

pkgs/development/interpreters/python/wrap.sh

+    # We need to support both the case when makeWrapperArgs
+    # is an array and a IFS-separated string.
+    # TODO: remove the string branch when __structuredAttrs are used.
+    if [[ "${makeWrapperArgs+defined}" == "defined" && "$(declare -p makeWrapperArgs)" =~ ^'declare -a makeWrapperArgs=' ]]; then


makeWrapperArgs would disappear, because we will have the appsWrapperArgs.

There is some code here for backwards-compatibiltiy as well that will be removed.

FRidh · 2020-11-05T17:59:09Z

pkgs/build-support/setup-hooks/wrap-apps-hook/wrap-apps-hook.sh

+    [ -z "$wrapAppsHookHasRun" ] || return 0
+    wrapAppsHookHasRun=1
+
+    local targetDirs =( "$prefix/bin" "$prefix/sbin" "$prefix/libexec"  )


Each of the hooks had a different set of targetDirs. I chose to consider all of them.

FRidh · 2020-11-05T18:46:37Z

pkgs/development/interpreters/python/hooks/default.nix

@@ -122,6 +122,12 @@ in rec {
      };
    } ./python-remove-tests-dir-hook.sh) {};

+  pythonWrapExecutablesHook = callPackage ({ wrapPython }:


This hook would replace wrapPython, so users typically do not need to invoke wrapPythonPrograms(In) anymore. cc @jonringer

ttuegel · 2020-12-05T04:17:47Z

pkgs/development/libraries/qt-5/hooks/wrap-qt-apps-hook.sh

-        done
-    done
+    # Extend the appsWrapperArgs for wrapAppsHook
+    appsWrapperArgs+=qtWrapperArgs


I'm sure you know Bash better than I do 😅 but why isn't this,

appsWrapperArgs+=( "${qtWrapperArgs[@]}" )

stale · 2021-06-03T19:34:34Z

I marked this as stale due to inactivity. → More info

nixos-discourse · 2022-08-23T09:26:15Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/low-effort-and-high-impact-tasks/21095/41

rnhmjoj · 2023-06-12T12:25:00Z

So, it's 2023, it's been 6 years since #25351, we're still shipping broken packages but we now have structured attributes (opt-in via __structuredAttrs = true) in stdenv. What's the next step?

If this is not going to be done for the couple of release, I would ask you to reconsider merging #83321, which works well and is a significantly simpler solution. I understand the appeal of a grand unified wrapping scheme to solve all this issues, but if it's going to take ten years, I rather implement the stop gap solution now: it's not going to make the matter any worse.

rnhmjoj · 2024-04-21T22:46:35Z

ping

FRidh added 8 commits November 2, 2020 17:35

Python wrap.sh: remove unused createBuildInputsPth

9a2d7fb

Long ago we used pth files for finding Python dependencies. Around the time we changed to the wheel builder that feature was no longer used.

wrapPython: split wrapPythonProgramsIn into multiple functions

e2017f4

wrapPythonProgramsIn both wraps and patches. Long-term, we should separate this further.

wrapPython: call it from python/hooks

1fd7fc0

This is where all Python-related hooks can be found.

pythonWrapExecutablesHook: wrap in a fixup phase

7d2b2d8

instead of requiring the user to always invoke `wrapPythonPrograms` manually.

wrapGAppsHook: migrate to wrapAppsHook

e3d42d5

wrapQtAppsHook: migrate to wrapAppsHook

e574fa7

TODO Python

63395d4

First need to switch from propagatedBuildInputs to requiredPythonModules.

FRidh added this to the 21.03 milestone Nov 5, 2020

FRidh requested review from ttuegel, worldofpeace and jtojnar November 5, 2020 17:49

FRidh commented Nov 5, 2020

View reviewed changes

ofborg bot added 6.topic: python 6.topic: qt/kde labels Nov 5, 2020

FRidh commented Nov 5, 2020

View reviewed changes

This was referenced Nov 6, 2020

python: propagate makeWrapper arguments #83321

Closed

buildPythonPackage: add api version for automation tools #103401

Closed

ttuegel reviewed Dec 5, 2020

View reviewed changes

rnhmjoj mentioned this pull request Feb 24, 2021

pyplot not working in ipython #80147

Open

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 3, 2021

rnhmjoj mentioned this pull request Feb 10, 2022

Matplotlib Qt backend does not work on Wayland #158806

Closed

Artturin modified the milestones: 21.05, 23.05 Dec 31, 2022

stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Dec 31, 2022

RaitoBezarius modified the milestones: 23.05, 23.11 May 31, 2023

rnhmjoj mentioned this pull request Jul 6, 2023

matplotlib is missing GSettings #25351

Open

wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: wrapAppsHook: unify wrapper generation Gapps/Python/Qt/... #102949

WIP: wrapAppsHook: unify wrapper generation Gapps/Python/Qt/... #102949

FRidh commented Nov 5, 2020

FRidh Nov 5, 2020

FRidh Nov 5, 2020

jtojnar Nov 5, 2020

FRidh Nov 5, 2020

FRidh Nov 5, 2020

FRidh Nov 5, 2020

FRidh Nov 5, 2020

ttuegel Dec 5, 2020

stale bot commented Jun 3, 2021

nixos-discourse commented Aug 23, 2022

rnhmjoj commented Jun 12, 2023 •

edited

Loading

rnhmjoj commented Apr 21, 2024

WIP: wrapAppsHook: unify wrapper generation Gapps/Python/Qt/... #102949

Are you sure you want to change the base?

WIP: wrapAppsHook: unify wrapper generation Gapps/Python/Qt/... #102949

Conversation

FRidh commented Nov 5, 2020

Motivation for this change

Things done

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jun 3, 2021

nixos-discourse commented Aug 23, 2022

rnhmjoj commented Jun 12, 2023 • edited Loading

rnhmjoj commented Apr 21, 2024

rnhmjoj commented Jun 12, 2023 •

edited

Loading