From ef814003124673a97509ee14613ec05dde4913a6 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 10 May 2020 10:54:04 +0300 Subject: [PATCH 01/37] [RFC 55]: Declarative Wrappers --- rfcs/0055-declarative-wrappers.md | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 rfcs/0055-declarative-wrappers.md diff --git a/rfcs/0055-declarative-wrappers.md b/rfcs/0055-declarative-wrappers.md new file mode 100644 index 000000000..c7e27dbd6 --- /dev/null +++ b/rfcs/0055-declarative-wrappers.md @@ -0,0 +1,58 @@ +--- +feature: Declarative Wrappers +start-date: (fill me in with today's date, YYYY-MM-DD) +author: Doron Behar +co-authors: (find a buddy later to help out with the RFC) +shepherd-team: (names, to be nominated and accepted by RFC steering committee) +shepherd-leader: (name to be appointed by RFC steering committee) +related-issues: https://github.com/NixOS/nixpkgs/pull/85103 +--- + +# Declarative Wrappers +[summary]: #summary + +Nixpkgs has many + +# Motivation +[motivation]: #motivation + +Why are we doing this? What use cases does it support? What is the expected +outcome? + +# Detailed design +[design]: #detailed-design + +This is the core, normative part of the RFC. Explain the design in enough +detail for somebody familiar with the ecosystem to understand, and implement. +This should get into specifics and corner-cases. Yet, this section should also +be terse, avoiding redundancy even at the cost of clarity. + +# Examples and Interactions +[examples-and-interactions]: #examples-and-interactions + +This section illustrates the detailed design. This section should clarify all +confusion the reader has from the previous sections. It is especially important +to counterbalance the desired terseness of the detailed design; if you feel +your detailed design is rudely short, consider making this section longer +instead. + +# Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +# Alternatives +[alternatives]: #alternatives + +What other designs have been considered? What is the impact of not doing this? + +# Unresolved questions +[unresolved]: #unresolved-questions + +What parts of the design are still TBD or unknowns? + +# Future work +[future]: #future-work + +What future work, if any, would be implied or impacted by this feature +without being directly part of the work? From b780229a542bccfc0e2be6eb4ff60e993a71c820 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sat, 15 Aug 2020 13:55:04 +0300 Subject: [PATCH 02/37] Declarative wrappers --- rfcs/0075-declarative-wrappers.md | 114 ++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 rfcs/0075-declarative-wrappers.md diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md new file mode 100644 index 000000000..d3aa0b2c0 --- /dev/null +++ b/rfcs/0075-declarative-wrappers.md @@ -0,0 +1,114 @@ +--- +feature: Declarative Wrappers +start-date: (fill me in with today's date, YYYY-MM-DD) +author: Doron Behar +co-authors: (find a buddy later to help out with the RFC) +shepherd-team: (names, to be nominated and accepted by RFC steering committee) +shepherd-leader: (name to be appointed by RFC steering committee) +related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/pull/85103). +--- + +# Summary +[summary]: #summary + +Manage the environment of wrappers declaratively and reduce the usage and +deprecate shell hooks such as `wrapGAppsHook` and `wrapQtAppsHook` in favor of +a new method to add an environment to an executable. + +# Motivation +[motivation]: #motivation + +- Make wrappers a separate derivation to make mere changes to the environment + not trigger a heavy rebuild. +- Make it easier to debug why env vars are added to an executable, by using Nix + as the language to evaluate what env vars are needed, instead of not + documented good enough and not easily debug-able shell hooks. + +We have numerous issues regarding wrappers and our wrapper shell hooks. Here's +a list of them: + + + +- https://github.com/NixOS/nixpkgs/issues/32790 :: Only related - they talk about patching some packages to use GI_GIR_PATH +- https://github.com/NixOS/nixpkgs/pull/83321 +- https://discourse.nixos.org/t/declarative-wrappers/1775/1 +- https://github.com/NixOS/nixpkgs/pull/53816 :: [RFC] Python library wrappers +- https://github.com/NixOS/nixpkgs/pull/70691 +- https://github.com/NixOS/nixpkgs/pull/71089 +- https://discourse.nixos.org/t/wrapqtappshook-out-of-tree/5619/6 +- https://github.com/NixOS/nixpkgs/pull/83705 +- https://github.com/NixOS/nixpkgs/issues/87667 +- https://github.com/NixOS/nixpkgs/pull/89145 :: issue with luarocks generated luasockets vs when built with `buildLuaPackage` +- https://github.com/NixOS/nixpkgs/issues/87883 :: gimp: No module named gtk / plugins do not work +- https://github.com/NixOS/nixpkgs/pull/61213 :: scons: Fix a wrapping issue which overrides your PATH +- https://github.com/NixOS/nixpkgs/pull/61553 :: cc-wrapper: add hook +- https://github.com/NixOS/nixpkgs/pull/32552 :: makeWrapper: Use case not elif-chaining, and other cleanups +- https://github.com/NixOS/nixpkgs/issues/78792 :: general idea regarding how to orchestrate wrap hooks +- https://github.com/NixOS/nixpkgs/issues/83667 :: propagated-build-inputs in cross compilations +- https://github.com/NixOS/nixpkgs/issues/85306 :: Just another unwrapped derivation living in the wild issue +- https://github.com/NixOS/nixpkgs/pull/86166 :: an unclear fix to an issue with wrappings of stdenv tools +- https://github.com/NixOS/nixpkgs/issues/53111 :: small issue regarding gnome apps using `gapplication launch` which may be solved nicely if wrapped +- https://github.com/NixOS/nixpkgs/issues/86369 :: qt plugin path +- https://github.com/NixOS/nixpkgs/issues/86054 :: qt translations not found +- https://github.com/NixOS/nixpkgs/issues/86048 :: hplip unwrapped +- https://github.com/NixOS/nixpkgs/issues/84308 :: gtk-doc cross references +- https://github.com/NixOS/nixpkgs/issues/84249 :: git-remote-hg not wrapped correctly +(unreported) kdoctools exports an XDG_DATA_DIRS file for kdeconnect +(unreported) kconfigwidgets.dev is refernced by kdeconnect because of QT_PLUGIN_PATH +(unreported) bin/ of gobject-introspection.dev is added to (e.g) beets +- https://github.com/NixOS/nixpkgs/issues/87033 :: Something related to GDK_PIXBUF +- https://github.com/NixOS/nixpkgs/issues/49132 :: GDK_PIXBUF compatibilities +- https://github.com/NixOS/nixpkgs/issues/54278 :: GDK_PIXBUF compatibilities +- https://github.com/NixOS/nixpkgs/issues/39493 :: no ability to wrapProgram every executable that depends on gdk-pixbuf +- https://github.com/NixOS/nixpkgs/issues/60260 :: General complain about wrappers. +- https://github.com/NixOS/nixpkgs/issues/87667 :: VLC and gtk path + +# Detailed design [design]: #detailed-design + +Consider every env var set by our shell hooks and our builders such as +`buildPythonPackage` and friends. Every such var is usually set because certain +packages used in the wrapped package's inputs, imply that this env var will be +needed during runtime. Many such vars' values are known to concatenated with +`:`. + +Now, What if we'd write down in the `passthru`s of these packages, that "in order to +run something that requires this package, you need `THIS_ENV_VAR` to include +`$out/this/value`? Imagine this information was available to us in a consistent +manner. We could then write a Nix function, that will calculate the necessary +arguments to the shell functions `makeWrapper` or `wrapProgram`, somewhat +similarly to how our fixup hooks already do this. + +This Nix function, let us call it `wrapGeneric`, should iterate recursively all +`buildInputs` and `propagatedBuildInputs` of a given derivation, and decide +what environment this derivation will need to run. + +# Examples and Interactions +[examples-and-interactions]: #examples-and-interactions + +This section illustrates the detailed design. This section should clarify all +confusion the reader has from the previous sections. It is especially important +to counterbalance the desired terseness of the detailed design; if you feel +your detailed design is rudely short, consider making this section longer +instead. + +# Drawbacks +[drawbacks]: #drawbacks + +If we think our shell hooks can scale, and that they are easily manageable, and +that we are OK with + +# Alternatives +[alternatives]: #alternatives + +What other designs have been considered? What is the impact of not doing this? + +# Unresolved questions +[unresolved]: #unresolved-questions + +What parts of the design are still TBD or unknowns? + +# Future work +[future]: #future-work + +What future work, if any, would be implied or impacted by this feature +without being directly part of the work? From d09a05882b2db00e2268976021f2f9d5c25c7e54 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 11:51:09 +0300 Subject: [PATCH 03/37] Finish motivation --- rfcs/0075-declarative-wrappers.md | 197 ++++++++++++++++++++++++------ 1 file changed, 158 insertions(+), 39 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index d3aa0b2c0..e371b049e 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -11,9 +11,8 @@ related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/ # Summary [summary]: #summary -Manage the environment of wrappers declaratively and reduce the usage and -deprecate shell hooks such as `wrapGAppsHook` and `wrapQtAppsHook` in favor of -a new method to add an environment to an executable. +Manage the environment of wrappers declaratively and deprecate shell based +methods for calculating runtime environment of packages. # Motivation [motivation]: #motivation @@ -25,43 +24,163 @@ a new method to add an environment to an executable. documented good enough and not easily debug-able shell hooks. We have numerous issues regarding wrappers and our wrapper shell hooks. Here's -a list of them: - - - -- https://github.com/NixOS/nixpkgs/issues/32790 :: Only related - they talk about patching some packages to use GI_GIR_PATH -- https://github.com/NixOS/nixpkgs/pull/83321 -- https://discourse.nixos.org/t/declarative-wrappers/1775/1 -- https://github.com/NixOS/nixpkgs/pull/53816 :: [RFC] Python library wrappers -- https://github.com/NixOS/nixpkgs/pull/70691 -- https://github.com/NixOS/nixpkgs/pull/71089 -- https://discourse.nixos.org/t/wrapqtappshook-out-of-tree/5619/6 -- https://github.com/NixOS/nixpkgs/pull/83705 +a list of them, sorted by categories. + +## Closure related + +- https://github.com/NixOS/nixpkgs/issues/95027 + +@jtojnar & @yegortimoshenko How +hard would it be to test all of our wrapped `gobject-introspection` using +packages that the equivalent, `GI_GIR_PATH` environment should work? If our +wrappers were declarative, and they were a separate derivation, at least we +wouldn't have to rebuild tons of packages to do so - we'd have to rebuild only +the wrappers. Plus, since all of the environment is available to us via +`builtins.toJSON`, it should be possible to write a script that will compare +the environments to make the transition easier to review. + +## Missing environment + +- https://github.com/NixOS/nixpkgs/pull/83321 and +- https://github.com/NixOS/nixpkgs/pull/53816 + +@rnhmjoj & @timokau How unfortunate it is that Python's `buildEnv` doesn't know +to do anything besides setting `NIX_PYTHONPATH` - it knows nothing about other +env vars, which is totally legitimate for dependencies of the environment to +rely upon runtime. Declarative wrappers don't care about the meaning of env +vars - all of them are treated equally, considering all of the inputs of a +derivation equally. + +- https://github.com/NixOS/nixpkgs/pull/75851 - https://github.com/NixOS/nixpkgs/issues/87667 -- https://github.com/NixOS/nixpkgs/pull/89145 :: issue with luarocks generated luasockets vs when built with `buildLuaPackage` -- https://github.com/NixOS/nixpkgs/issues/87883 :: gimp: No module named gtk / plugins do not work -- https://github.com/NixOS/nixpkgs/pull/61213 :: scons: Fix a wrapping issue which overrides your PATH -- https://github.com/NixOS/nixpkgs/pull/61553 :: cc-wrapper: add hook -- https://github.com/NixOS/nixpkgs/pull/32552 :: makeWrapper: Use case not elif-chaining, and other cleanups -- https://github.com/NixOS/nixpkgs/issues/78792 :: general idea regarding how to orchestrate wrap hooks -- https://github.com/NixOS/nixpkgs/issues/83667 :: propagated-build-inputs in cross compilations -- https://github.com/NixOS/nixpkgs/issues/85306 :: Just another unwrapped derivation living in the wild issue -- https://github.com/NixOS/nixpkgs/pull/86166 :: an unclear fix to an issue with wrappings of stdenv tools -- https://github.com/NixOS/nixpkgs/issues/53111 :: small issue regarding gnome apps using `gapplication launch` which may be solved nicely if wrapped -- https://github.com/NixOS/nixpkgs/issues/86369 :: qt plugin path -- https://github.com/NixOS/nixpkgs/issues/86054 :: qt translations not found -- https://github.com/NixOS/nixpkgs/issues/86048 :: hplip unwrapped -- https://github.com/NixOS/nixpkgs/issues/84308 :: gtk-doc cross references -- https://github.com/NixOS/nixpkgs/issues/84249 :: git-remote-hg not wrapped correctly -(unreported) kdoctools exports an XDG_DATA_DIRS file for kdeconnect -(unreported) kconfigwidgets.dev is refernced by kdeconnect because of QT_PLUGIN_PATH -(unreported) bin/ of gobject-introspection.dev is added to (e.g) beets -- https://github.com/NixOS/nixpkgs/issues/87033 :: Something related to GDK_PIXBUF -- https://github.com/NixOS/nixpkgs/issues/49132 :: GDK_PIXBUF compatibilities -- https://github.com/NixOS/nixpkgs/issues/54278 :: GDK_PIXBUF compatibilities -- https://github.com/NixOS/nixpkgs/issues/39493 :: no ability to wrapProgram every executable that depends on gdk-pixbuf -- https://github.com/NixOS/nixpkgs/issues/60260 :: General complain about wrappers. -- https://github.com/NixOS/nixpkgs/issues/87667 :: VLC and gtk path + +Fixable with our current wrapping tools (I guess?) but it's unfortunate that we +have to trigger a rebuild of VLC and potentially increase it's closure size, +just because of a missing env var for some users. If only our wrapping +requirements were accessible via Nix attrsets, we could have instructed our +modules to consider this information when building the wrappers of the packages +in `environment.systemPackages`. + +- https://github.com/NixOS/nixpkgs/issues/87883 (Fixed) + +@jtojnar wouldn't it be wonderful if the wrapper of gimp would have known +exactly what `NIX_PYTHONPATH` to use when wrapping gimp, just because `pygtk` +was in it's inputs? Declarative wrappers would also allow us to merge the +wrappings of such derivation to reduce double wrappings, as currently done at +[`wrapper.nix`](https://github.com/NixOS/nixpkgs/blob/b7be00ad5ed0cdbba73fa7fd7fadcb842831f137/pkgs/applications/graphics/gimp/wrapper.nix#L16-L28) +and +[`default.nix`](https://github.com/NixOS/nixpkgs/blob/b7be00ad5ed0cdbba73fa7fd7fadcb842831f137/pkgs/applications/graphics/gimp/default.nix#L142-L145). + +- https://github.com/NixOS/nixpkgs/issues/85306 +- https://github.com/NixOS/nixpkgs/issues/84249 + +`git-remote-hg` and `qttools` are not wrapped properly. + +- https://github.com/NixOS/nixpkgs/issues/86048 + +I guess we don't wrap HPLIP because not everybody want to use these binaries +and hence want these GUI deps in their closure (if they were wrapped with a +setup hook)? Declarative wrappers would allow some users to use the wrapped +binaries and others not need it, via an override or a NixOS config flag, +without triggering a rebuild of HPLIP itself. + +## Orchestrating wrapping hooks + +- https://github.com/NixOS/nixpkgs/issues/78792 + +@worldofpeace you are correct. All of these setup-hooks are a mess, but at +least we have documented, yet not totally implemented this section of the +manual +https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues-double-wrapped + +Declarative wrappers will deprecate the usage of our shell based hooks and will +wrap all executables automatically according to their needs. + +- https://github.com/NixOS/nixpkgs/issues/86369 + +@ttuegel I get the sense [you support this +idea](https://github.com/NixOS/nixpkgs/issues/86369#issuecomment-626732191). +But for anyone else interested, the issue is a bit complex, so once you'll read +the design of this RFC, and see examples of what the POC implementation of +declarative wrappers [is capable +of](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666), I hope +you'll see how declarative wrappers can solve this issue. + + +## Other Issues only _possibly_ fixable by declarative wrappers + +- https://github.com/NixOS/nixpkgs/pull/61213 + +I'm not sure what's the issue there. But, I'm sure that a Nix based builder of +a Python environment should make it easier to control and alter if needed, what +environment is used even by builders, not only user facing Python environments. + +- https://github.com/NixOS/nixpkgs/issues/83667 + +@FRidh I see no reason for Python deps of Python packages to need to be in +`propagatedBuildInputs` and not regular `buildInputs`. I think this was done so +in the past so it'd be easy to know how to wrap them? Declarative wrappers +won't require runtime-env-requiring deps to be only in `propagatedBuildInputs` +or `buildInputs` - it should pick such deps from both lists. Hence, I think it +should be possible to make Python's static builds consistent with other +ecosystems. + +- https://github.com/NixOS/nixpkgs/issues/86054 + +@ttuegel TBH I can't tell if declarative wrappers might help, but I'm linking +this issue here as well because @worldofpeace wrote it might related to +wrappers? Feel free to suggest removing this in the RFC review. + +- https://github.com/NixOS/nixpkgs/issues/49132 +- https://github.com/NixOS/nixpkgs/issues/54278 +- https://github.com/NixOS/nixpkgs/issues/39493 + +`GDK_PIXBUF` compatibilities? I haven't investigated them to the details, so feel +free @jtojnar to review me and tell me that declarative wrappers won't help. + +## Unreported issues (AFAIK) + +`kdeconnect` has `kdoctools` in it's closure because it's wrapper has +`kdoctools` due to it picked by `wrapQtAppsHook`: + +``` +$ nix why-depends -f. kdeconnect kdoctools +/nix/store/sh42k6cz4j48br4cxi2qn173rys4japp-kdeconnect-1.3.5 +╚═══bin/kdeconnect-cli: …xport XDG_DATA_DIRS='/nix/store/m16681i5dhhkhszi9w42ir037jvbnab9-kdoctools-5.71.0/share'${XDG_DA… + => /nix/store/m16681i5dhhkhszi9w42ir037jvbnab9-kdoctools-5.71.0 +``` + +A similar issue is with `kconfigwidgets.dev` and `kdeconnect`: + +``` +$ nix why-depends -f. kdeconnect kdeframeworks.kconfigwidgets.dev + /nix/store/sh42k6cz4j48br4cxi2qn173rys4japp-kdeconnect-1.3.5 + ╚═══bin/kdeconnect-cli: …port QT_PLUGIN_PATH='/nix/store/qssjj6ki7jiskw2kfygvfiy8fxrclwrl-kconfigwidgets-5.71.0-dev/lib/q… + => /nix/store/qssjj6ki7jiskw2kfygvfiy8fxrclwrl-kconfigwidgets-5.71.0-dev +``` + +Also similar (but possibly fixable by moving `gobject-introspection` to a +different inputs list? + +``` +$ nix why-depends -f. beets gobject-introspection.dev +/nix/store/93lfrhm8vp17m8ziqi8vp6v4cff67wkb-beets-1.4.9 +╚═══bin/beet: …-expat-2.2.8-dev/bin:/nix/store/y3ym76wrak3300vsjyf3klr52cnzmxwd-gobject-introspection-1.64.1-de… + => /nix/store/y3ym76wrak3300vsjyf3klr52cnzmxwd-gobject-introspection-1.64.1-dev +``` + +## Other issues + +- https://github.com/NixOS/nixpkgs/issues/60260 + +General, justified complain about wrappers. + +- https://github.com/NixOS/nixpkgs/issues/95027 + +Since our wrappers are shell scripts, `gdb` can't run them. What if we had +written a C based wrapper, that perhaps would read what environment it needs to +set from a JSON file, and it will call the unwrapped original executable? I +need feedback regarding whether `gdb` will play nice with this. # Detailed design [design]: #detailed-design From 37c7a8c3e02453da5710be4714607019e6cf3a8a Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 12:51:43 +0300 Subject: [PATCH 04/37] Remove old RFC file --- rfcs/0055-declarative-wrappers.md | 58 ------------------------------- 1 file changed, 58 deletions(-) delete mode 100644 rfcs/0055-declarative-wrappers.md diff --git a/rfcs/0055-declarative-wrappers.md b/rfcs/0055-declarative-wrappers.md deleted file mode 100644 index c7e27dbd6..000000000 --- a/rfcs/0055-declarative-wrappers.md +++ /dev/null @@ -1,58 +0,0 @@ ---- -feature: Declarative Wrappers -start-date: (fill me in with today's date, YYYY-MM-DD) -author: Doron Behar -co-authors: (find a buddy later to help out with the RFC) -shepherd-team: (names, to be nominated and accepted by RFC steering committee) -shepherd-leader: (name to be appointed by RFC steering committee) -related-issues: https://github.com/NixOS/nixpkgs/pull/85103 ---- - -# Declarative Wrappers -[summary]: #summary - -Nixpkgs has many - -# Motivation -[motivation]: #motivation - -Why are we doing this? What use cases does it support? What is the expected -outcome? - -# Detailed design -[design]: #detailed-design - -This is the core, normative part of the RFC. Explain the design in enough -detail for somebody familiar with the ecosystem to understand, and implement. -This should get into specifics and corner-cases. Yet, this section should also -be terse, avoiding redundancy even at the cost of clarity. - -# Examples and Interactions -[examples-and-interactions]: #examples-and-interactions - -This section illustrates the detailed design. This section should clarify all -confusion the reader has from the previous sections. It is especially important -to counterbalance the desired terseness of the detailed design; if you feel -your detailed design is rudely short, consider making this section longer -instead. - -# Drawbacks -[drawbacks]: #drawbacks - -Why should we *not* do this? - -# Alternatives -[alternatives]: #alternatives - -What other designs have been considered? What is the impact of not doing this? - -# Unresolved questions -[unresolved]: #unresolved-questions - -What parts of the design are still TBD or unknowns? - -# Future work -[future]: #future-work - -What future work, if any, would be implied or impacted by this feature -without being directly part of the work? From 2a0a6306eb37231ba5411b3cf79630846a1c55c6 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 12:52:56 +0300 Subject: [PATCH 05/37] Add more relevant issues regarding compiling wrappers --- rfcs/0075-declarative-wrappers.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index e371b049e..379ab3b2a 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -176,6 +176,9 @@ $ nix why-depends -f. beets gobject-introspection.dev General, justified complain about wrappers. - https://github.com/NixOS/nixpkgs/issues/95027 +- https://github.com/NixOS/nixpkgs/issues/23018 +- https://github.com/NixOS/nixpkgs/issues/11133 +- https://github.com/NixOS/nixpkgs/pull/95569 Since our wrappers are shell scripts, `gdb` can't run them. What if we had written a C based wrapper, that perhaps would read what environment it needs to From c9d70559e03f49b6672fb07e7e6fed7a0009b539 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:26:48 +0300 Subject: [PATCH 06/37] Write most of the design and more --- rfcs/0075-declarative-wrappers.md | 201 +++++++++++++++++++++++------- 1 file changed, 155 insertions(+), 46 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 379ab3b2a..512511c35 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -28,7 +28,7 @@ a list of them, sorted by categories. ## Closure related -- https://github.com/NixOS/nixpkgs/issues/95027 +- [issue 95027](https://github.com/NixOS/nixpkgs/issues/95027) @jtojnar & @yegortimoshenko How hard would it be to test all of our wrapped `gobject-introspection` using @@ -41,8 +41,8 @@ the environments to make the transition easier to review. ## Missing environment -- https://github.com/NixOS/nixpkgs/pull/83321 and -- https://github.com/NixOS/nixpkgs/pull/53816 +- [pull 83321](https://github.com/NixOS/nixpkgs/pull/83321) +- [pull 53816](https://github.com/NixOS/nixpkgs/pull/53816) @rnhmjoj & @timokau How unfortunate it is that Python's `buildEnv` doesn't know to do anything besides setting `NIX_PYTHONPATH` - it knows nothing about other @@ -51,8 +51,8 @@ rely upon runtime. Declarative wrappers don't care about the meaning of env vars - all of them are treated equally, considering all of the inputs of a derivation equally. -- https://github.com/NixOS/nixpkgs/pull/75851 -- https://github.com/NixOS/nixpkgs/issues/87667 +- [pull 75851](https://github.com/NixOS/nixpkgs/pull/75851) +- [issue 87667](https://github.com/NixOS/nixpkgs/issues/87667) Fixable with our current wrapping tools (I guess?) but it's unfortunate that we have to trigger a rebuild of VLC and potentially increase it's closure size, @@ -61,7 +61,7 @@ requirements were accessible via Nix attrsets, we could have instructed our modules to consider this information when building the wrappers of the packages in `environment.systemPackages`. -- https://github.com/NixOS/nixpkgs/issues/87883 (Fixed) +- [issue 87883](https://github.com/NixOS/nixpkgs/issues/87883) (Fixed) @jtojnar wouldn't it be wonderful if the wrapper of gimp would have known exactly what `NIX_PYTHONPATH` to use when wrapping gimp, just because `pygtk` @@ -71,12 +71,12 @@ wrappings of such derivation to reduce double wrappings, as currently done at and [`default.nix`](https://github.com/NixOS/nixpkgs/blob/b7be00ad5ed0cdbba73fa7fd7fadcb842831f137/pkgs/applications/graphics/gimp/default.nix#L142-L145). -- https://github.com/NixOS/nixpkgs/issues/85306 -- https://github.com/NixOS/nixpkgs/issues/84249 +- [issue 85306](https://github.com/NixOS/nixpkgs/issues/85306) +- [issue 84249](https://github.com/NixOS/nixpkgs/issues/84249) `git-remote-hg` and `qttools` are not wrapped properly. -- https://github.com/NixOS/nixpkgs/issues/86048 +- [issue 86048](https://github.com/NixOS/nixpkgs/issues/86048) I guess we don't wrap HPLIP because not everybody want to use these binaries and hence want these GUI deps in their closure (if they were wrapped with a @@ -86,7 +86,7 @@ without triggering a rebuild of HPLIP itself. ## Orchestrating wrapping hooks -- https://github.com/NixOS/nixpkgs/issues/78792 +- [issue 78792](https://github.com/NixOS/nixpkgs/issues/78792) @worldofpeace you are correct. All of these setup-hooks are a mess, but at least we have documented, yet not totally implemented this section of the @@ -96,7 +96,7 @@ https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues-double-wrapped Declarative wrappers will deprecate the usage of our shell based hooks and will wrap all executables automatically according to their needs. -- https://github.com/NixOS/nixpkgs/issues/86369 +- [issue 86369](https://github.com/NixOS/nixpkgs/issues/86369) @ttuegel I get the sense [you support this idea](https://github.com/NixOS/nixpkgs/issues/86369#issuecomment-626732191). @@ -109,13 +109,13 @@ you'll see how declarative wrappers can solve this issue. ## Other Issues only _possibly_ fixable by declarative wrappers -- https://github.com/NixOS/nixpkgs/pull/61213 +- [pull 61213](https://github.com/NixOS/nixpkgs/pull/61213) I'm not sure what's the issue there. But, I'm sure that a Nix based builder of a Python environment should make it easier to control and alter if needed, what environment is used even by builders, not only user facing Python environments. -- https://github.com/NixOS/nixpkgs/issues/83667 +- [issue 83667](https://github.com/NixOS/nixpkgs/issues/83667) @FRidh I see no reason for Python deps of Python packages to need to be in `propagatedBuildInputs` and not regular `buildInputs`. I think this was done so @@ -125,15 +125,15 @@ or `buildInputs` - it should pick such deps from both lists. Hence, I think it should be possible to make Python's static builds consistent with other ecosystems. -- https://github.com/NixOS/nixpkgs/issues/86054 +- [issue 86054](https://github.com/NixOS/nixpkgs/issues/86054) @ttuegel TBH I can't tell if declarative wrappers might help, but I'm linking this issue here as well because @worldofpeace wrote it might related to wrappers? Feel free to suggest removing this in the RFC review. -- https://github.com/NixOS/nixpkgs/issues/49132 -- https://github.com/NixOS/nixpkgs/issues/54278 -- https://github.com/NixOS/nixpkgs/issues/39493 +- [issue 49132](https://github.com/NixOS/nixpkgs/issues/49132) +- [issue 54278](https://github.com/NixOS/nixpkgs/issues/54278) +- [issue 39493](https://github.com/NixOS/nixpkgs/issues/39493) `GDK_PIXBUF` compatibilities? I haven't investigated them to the details, so feel free @jtojnar to review me and tell me that declarative wrappers won't help. @@ -171,66 +171,175 @@ $ nix why-depends -f. beets gobject-introspection.dev ## Other issues -- https://github.com/NixOS/nixpkgs/issues/60260 +- [issue 60260](https://github.com/NixOS/nixpkgs/issues/60260) General, justified complain about wrappers. -- https://github.com/NixOS/nixpkgs/issues/95027 -- https://github.com/NixOS/nixpkgs/issues/23018 -- https://github.com/NixOS/nixpkgs/issues/11133 -- https://github.com/NixOS/nixpkgs/pull/95569 +- [issue 95027](https://github.com/NixOS/nixpkgs/issues/95027) +- [issue 23018](https://github.com/NixOS/nixpkgs/issues/23018) +- [issue 11133](https://github.com/NixOS/nixpkgs/issues/11133) +- [pull 95569](https://github.com/NixOS/nixpkgs/pull/95569) Since our wrappers are shell scripts, `gdb` can't run them. What if we had written a C based wrapper, that perhaps would read what environment it needs to set from a JSON file, and it will call the unwrapped original executable? I need feedback regarding whether `gdb` will play nice with this. -# Detailed design [design]: #detailed-design +This issue may not directly relate to declarative wrappers, and it is already +addressed in @FRidh's [pull 95569](https://github.com/NixOS/nixpkgs/pull/95569), but perhaps +both ideas could be integrated into an alternative, simpler creation method of +binary wrappers. See +[comment](https://github.com/NixOS/nixpkgs/pull/95569#issuecomment-674508806) -Consider every env var set by our shell hooks and our builders such as -`buildPythonPackage` and friends. Every such var is usually set because certain -packages used in the wrapped package's inputs, imply that this env var will be -needed during runtime. Many such vars' values are known to concatenated with -`:`. +# Detailed design +[design]: #detailed-design -Now, What if we'd write down in the `passthru`s of these packages, that "in order to -run something that requires this package, you need `THIS_ENV_VAR` to include -`$out/this/value`? Imagine this information was available to us in a consistent -manner. We could then write a Nix function, that will calculate the necessary -arguments to the shell functions `makeWrapper` or `wrapProgram`, somewhat -similarly to how our fixup hooks already do this. +The current design is roughly implemented at +[pull 85103](https://github.com/NixOS/nixpkgs/pull/85103) . + +The idea is to have a Nix function, called `wrapGeneric` with an interface +similar to [`wrapMpv`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/video/mpv/wrapper.nix#L9-L23) and [`wrapNeovim`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/editors/neovim/wrapper.nix#L11-L24) which will accept a single derivation or +an array of them and it'll wrap all of their executables with the proper +environment, based on their inputs. + +`wrapGeneric` should iterate recursively all `buildInputs` and +`propagatedBuildInputs` of the input derivations, and construct an attrset with +which it'll calculate the necessary environment of the executables. Then either +via `wrapProgram` or a better method, it'll create the wrappers. + +A contributor using `wrapGeneric` shouldn't _care_ what type of wrapping needs +to be performed on his derivation's executables - whether these are Qt related +wrappings or a Gtk / gobject related. `wrapGeneric` should know all there is to +know about environment variables every library / input may need during runtime, +and with this information at hand, construct the necessary wrapper. + +In order for `wrapGenric` to know all of this information about our packaged +libraries - the information about runtime env, we need to write in the +`passthru`s of these libraries, what env vars they need. This Nix function, let us call it `wrapGeneric`, should iterate recursively all `buildInputs` and `propagatedBuildInputs` of a given derivation, and decide -what environment this derivation will need to run. +what environment this derivation will need to run. Such information was added +in the [POC pull's +@6283f15](https://github.com/NixOS/nixpkgs/pull/85103/commits/6283f15bb9b65af64571a78b039115807dcc2958). + +Additional features / improvements are [already +available](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666) +in the POC pull. For example: + +- It should be impossible for multi-value env vars to have duplicates, as + that's guaranteed by Nix' behavior when constructing arrays. +- Asking the wrapper creator to use more links and less colon-separated values + in env vars - should help avoid what [pull + 84689](https://github.com/NixOS/nixpkgs/pull/84689) fixed. # Examples and Interactions [examples-and-interactions]: #examples-and-interactions -This section illustrates the detailed design. This section should clarify all -confusion the reader has from the previous sections. It is especially important -to counterbalance the desired terseness of the detailed design; if you feel -your detailed design is rudely short, consider making this section longer -instead. +All examples are copied from and based on the [POC +pull](https://github.com/NixOS/nixpkgs/pull/85103). + +The new method of creating a python environment: + +```nix + my-python-env = wrapGeneric python3 { + linkByEnv = { + # you can use "link" here and that will link only the necessary files + # to the runtime environment. + PYTHONPATH = "linkPkg"; + }; + extraPkgs = with python3.pkgs; [ + matplotlib + ]; + wrapOut = { + # tells wrapGeneric to add to the wrappers this value for PYTHONPATH. + # Naturally, this should play along with the values given in + # linkByEnv.PYTHONPATH. + PYTHONPATH = "$out/${python3.sitePackages}"; + }; + }; +``` + +Consider a package is wrapped without directly making accessible the unwrapped +derivation. Meaning, say `all-packages.nix` has: + +```nix + my-awesome-pkg = wrapGeneric (callPackage ../applications/my-awesome-pkg { }) { }; +``` + +Assuming the user knows my-awesome-pkg is wrapped with wrapGeneric, they would +need to use an overlay like this, to override the unwrapped derivation: + +```nix +self: super: + +{ + my-awesome-pkg = super.wrapGeneric ( + super.my-awesome-pkg.unwrapped.overrideAttrs(oldAttrs: { + preFixup = '' + overriding preFixup from an overlay!! + ''; + }) + ) {}; +} +``` + +And to override the wrapper derivation, it should be possible using: + +```nix +self: super: + +{ + my-awesome-pkg = super.wrapGeneric super.my-awesome-pkg.unwrapped { + extraPkgs = [ + super.qt5.certain-qt-plugin + ]; + }; +} +``` # Drawbacks [drawbacks]: #drawbacks -If we think our shell hooks can scale, and that they are easily manageable, and -that we are OK with +The current design is heavily based on Nix, and knowing how to write and debug +Nix expressions is a skill not everyone are akin to learn. Also, overriding a +wrapped derivation is somewhat more awkward, due to this. Perhaps this +interface could be improved, and for sure proper documentation written should +help. # Alternatives [alternatives]: #alternatives -What other designs have been considered? What is the impact of not doing this? +Perhaps our shell hooks _can_ be fixed / improved, and we could help make it +easier to debug them via `NIX_DEBUG`. Then it might help us track down e.g why +environment variables are added twice etc. Still though, this wouldn't solve +half of the other issues presented here. Most importantly, the shell hooks rely +upon being in the inputs during build of the original derivation, hence mere +changes to an environment may trigger rebuilds that take a lot of time and +resources from avarage users. See [this +comment](https://github.com/NixOS/nixpkgs/pull/88136#issuecomment-632674653). # Unresolved questions [unresolved]: #unresolved-questions -What parts of the design are still TBD or unknowns? +The POC implementation does 1 thing which I'm most sure could be done better, +and that's iterating **recursively** all `buildInputs` and +`propagatedBuildInputs` of the input derivations. This is currently implemented +via a recursive (Nix) function, that's is prone to stack overflow due reach a +state of infinite recursion. But this risk is currently mitigated using an +array of packages we know don't need any env vars at runtime, and for sure are +very much at the bottom of the list of very common inputs. This is implemented [here](https://github.com/NixOS/nixpkgs/pull/85103/files#diff-44c2102a355f50131eb8f69fb7e7c18bR75-R131). + +There are other methods of doing this which might be better, but TBH I haven't +yet investigated all of them. For reference and hopefully for an advice, this +need was requested by others and discussed at: + +- [nix issue](https://github.com/NixOS/nix/issues/1245). +- [Interesting idea by @aszlig](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781) I haven't tested. +- [@nmattia's post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). +- [Discourse thread](https://discourse.nixos.org/t/any-way-to-get-a-derivations-inputdrvs-from-within-nix/7212/3). # Future work [future]: #future-work -What future work, if any, would be implied or impacted by this feature -without being directly part of the work? +Not that I can think of. From abb7609f1a1a8bb4228ed73ee6139ea49262f4a7 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:29:02 +0300 Subject: [PATCH 07/37] Small improvements to summary vs motivation --- rfcs/0075-declarative-wrappers.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 512511c35..47da30991 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -12,19 +12,17 @@ related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/ [summary]: #summary Manage the environment of wrappers declaratively and deprecate shell based -methods for calculating runtime environment of packages. +methods for calculating runtime environment of packages. Make wrappers a +separate derivation so that mere changes to the environment will not trigger a +rebuild. Make it easier to debug why env vars are added to an executable, by +using Nix as the language to evaluate what env vars are needed, instead of not +documented good enough and not easily debug-able shell hooks. # Motivation [motivation]: #motivation -- Make wrappers a separate derivation to make mere changes to the environment - not trigger a heavy rebuild. -- Make it easier to debug why env vars are added to an executable, by using Nix - as the language to evaluate what env vars are needed, instead of not - documented good enough and not easily debug-able shell hooks. - We have numerous issues regarding wrappers and our wrapper shell hooks. Here's -a list of them, sorted by categories. +a list of them, sorted to categories. ## Closure related From d1cadb6b6513c0c86be4569788b97d7572a402aa Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:33:05 +0300 Subject: [PATCH 08/37] Rephrase a bit orchestrating motivation section --- rfcs/0075-declarative-wrappers.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 47da30991..ee3d05cd6 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -92,7 +92,8 @@ manual https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues-double-wrapped Declarative wrappers will deprecate the usage of our shell based hooks and will -wrap all executables automatically according to their needs. +wrap all executables automatically according to their needs, without requiring +the contributor a lot of knowledge of the wrapping system. - [issue 86369](https://github.com/NixOS/nixpkgs/issues/86369) From a570b1a9ae8015ac8ca87bfd37d2fc61644b3517 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:35:12 +0300 Subject: [PATCH 09/37] Add small () --- rfcs/0075-declarative-wrappers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index ee3d05cd6..709bbffc8 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -120,7 +120,7 @@ environment is used even by builders, not only user facing Python environments. `propagatedBuildInputs` and not regular `buildInputs`. I think this was done so in the past so it'd be easy to know how to wrap them? Declarative wrappers won't require runtime-env-requiring deps to be only in `propagatedBuildInputs` -or `buildInputs` - it should pick such deps from both lists. Hence, I think it +or `buildInputs` - it should pick such deps from both lists. Hence, (I think) it should be possible to make Python's static builds consistent with other ecosystems. From 8db3605c926a2e1225a7f40c2d4568113835b253 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:38:11 +0300 Subject: [PATCH 10/37] More rephrasings and additions --- rfcs/0075-declarative-wrappers.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 709bbffc8..220d9bc98 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -106,7 +106,7 @@ of](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666), I hope you'll see how declarative wrappers can solve this issue. -## Other Issues only _possibly_ fixable by declarative wrappers +## Other Issues _possibly_ fixable by declarative wrappers - [pull 61213](https://github.com/NixOS/nixpkgs/pull/61213) @@ -127,8 +127,8 @@ ecosystems. - [issue 86054](https://github.com/NixOS/nixpkgs/issues/86054) @ttuegel TBH I can't tell if declarative wrappers might help, but I'm linking -this issue here as well because @worldofpeace wrote it might related to -wrappers? Feel free to suggest removing this in the RFC review. +this issue here because @worldofpeace wrote it might be related to wrappers? +Feel free to suggest removing this in the RFC review. - [issue 49132](https://github.com/NixOS/nixpkgs/issues/49132) - [issue 54278](https://github.com/NixOS/nixpkgs/issues/54278) @@ -139,6 +139,10 @@ free @jtojnar to review me and tell me that declarative wrappers won't help. ## Unreported issues (AFAIK) +Issues that bother me personally, but I haven't bothered to open an issue since +I doubt it would be feasible to fix with our current wrapping ecosystem, excuse +my pessimism `wrap{G,Qt}AppsHook` authors. + `kdeconnect` has `kdoctools` in it's closure because it's wrapper has `kdoctools` due to it picked by `wrapQtAppsHook`: From 5ddd60b028facad7dcd64e3e2be017fbf2bbaa87 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:39:15 +0300 Subject: [PATCH 11/37] Fix possibly fixable title --- rfcs/0075-declarative-wrappers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 220d9bc98..2d963cdee 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -106,7 +106,7 @@ of](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666), I hope you'll see how declarative wrappers can solve this issue. -## Other Issues _possibly_ fixable by declarative wrappers +## Issues _possibly_ fixable by declarative wrappers (?) - [pull 61213](https://github.com/NixOS/nixpkgs/pull/61213) From 778bc993836e71fc1463d4ab29b0155fa43f857b Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:44:17 +0300 Subject: [PATCH 12/37] Design sec improvements --- rfcs/0075-declarative-wrappers.md | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 2d963cdee..1b7b5ea15 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -191,8 +191,8 @@ need feedback regarding whether `gdb` will play nice with this. This issue may not directly relate to declarative wrappers, and it is already addressed in @FRidh's [pull 95569](https://github.com/NixOS/nixpkgs/pull/95569), but perhaps both ideas could be integrated into an alternative, simpler creation method of -binary wrappers. See -[comment](https://github.com/NixOS/nixpkgs/pull/95569#issuecomment-674508806) +binary wrappers. See [my +comment](https://github.com/NixOS/nixpkgs/pull/95569#issuecomment-674508806). # Detailed design [design]: #detailed-design @@ -200,13 +200,16 @@ binary wrappers. See The current design is roughly implemented at [pull 85103](https://github.com/NixOS/nixpkgs/pull/85103) . -The idea is to have a Nix function, called `wrapGeneric` with an interface -similar to [`wrapMpv`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/video/mpv/wrapper.nix#L9-L23) and [`wrapNeovim`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/editors/neovim/wrapper.nix#L11-L24) which will accept a single derivation or -an array of them and it'll wrap all of their executables with the proper -environment, based on their inputs. +The idea is to have a Nix function, let us call it `wrapGeneric`, with an +interface similar to +[`wrapMpv`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/video/mpv/wrapper.nix#L9-L23) +and +[`wrapNeovim`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/editors/neovim/wrapper.nix#L11-L24) +which will accept a single derivation or an array of them and it'll wrap all of +their executables with the proper environment, based on their inputs. `wrapGeneric` should iterate recursively all `buildInputs` and -`propagatedBuildInputs` of the input derivations, and construct an attrset with +`propagatedBuildInputs` of the input derivation(s), and construct an attrset with which it'll calculate the necessary environment of the executables. Then either via `wrapProgram` or a better method, it'll create the wrappers. @@ -218,20 +221,16 @@ and with this information at hand, construct the necessary wrapper. In order for `wrapGenric` to know all of this information about our packaged libraries - the information about runtime env, we need to write in the -`passthru`s of these libraries, what env vars they need. - -This Nix function, let us call it `wrapGeneric`, should iterate recursively all -`buildInputs` and `propagatedBuildInputs` of a given derivation, and decide -what environment this derivation will need to run. Such information was added -in the [POC pull's +`passthru`s of these libraries, what env vars they need. Such information was +added in the POC pull at [commit @6283f15](https://github.com/NixOS/nixpkgs/pull/85103/commits/6283f15bb9b65af64571a78b039115807dcc2958). Additional features / improvements are [already available](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666) in the POC pull. For example: -- It should be impossible for multi-value env vars to have duplicates, as - that's guaranteed by Nix' behavior when constructing arrays. +- It should be **impossible** for multi-value env vars to have duplicates, as + that's guaranteed by Nix' behavior when constructing arrays / attrsets. - Asking the wrapper creator to use more links and less colon-separated values in env vars - should help avoid what [pull 84689](https://github.com/NixOS/nixpkgs/pull/84689) fixed. @@ -242,7 +241,7 @@ in the POC pull. For example: All examples are copied from and based on the [POC pull](https://github.com/NixOS/nixpkgs/pull/85103). -The new method of creating a python environment: +Here's a new method of creating a python environment: ```nix my-python-env = wrapGeneric python3 { From 44a3b87c00f1cd6f97fbb07e844b19ccf08d6f8c Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:49:52 +0300 Subject: [PATCH 13/37] Improve examples --- rfcs/0075-declarative-wrappers.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 1b7b5ea15..f7baa4c1e 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -269,7 +269,7 @@ derivation. Meaning, say `all-packages.nix` has: my-awesome-pkg = wrapGeneric (callPackage ../applications/my-awesome-pkg { }) { }; ``` -Assuming the user knows my-awesome-pkg is wrapped with wrapGeneric, they would +Assuming the user knows `my-awesome-pkg` is wrapped with `wrapGeneric`, they would need to use an overlay like this, to override the unwrapped derivation: ```nix @@ -286,7 +286,10 @@ self: super: } ``` -And to override the wrapper derivation, it should be possible using: +And to override the wrapper derivation, e.g to add new optional features not +strictly necessary (as in [pull +83482](https://github.com/NixOS/nixpkgs/pull/83482)), it should be possible +using: ```nix self: super: From 0b259fa7f53d89083124841ce29f7738c506b287 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 15:53:43 +0300 Subject: [PATCH 14/37] Improve ending --- rfcs/0075-declarative-wrappers.md | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index f7baa4c1e..6ae0a943f 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -329,19 +329,22 @@ comment](https://github.com/NixOS/nixpkgs/pull/88136#issuecomment-632674653). The POC implementation does 1 thing which I'm most sure could be done better, and that's iterating **recursively** all `buildInputs` and -`propagatedBuildInputs` of the input derivations. This is currently implemented -via a recursive (Nix) function, that's is prone to stack overflow due reach a -state of infinite recursion. But this risk is currently mitigated using an -array of packages we know don't need any env vars at runtime, and for sure are -very much at the bottom of the list of very common inputs. This is implemented [here](https://github.com/NixOS/nixpkgs/pull/85103/files#diff-44c2102a355f50131eb8f69fb7e7c18bR75-R131). - -There are other methods of doing this which might be better, but TBH I haven't -yet investigated all of them. For reference and hopefully for an advice, this -need was requested by others and discussed at: - -- [nix issue](https://github.com/NixOS/nix/issues/1245). -- [Interesting idea by @aszlig](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781) I haven't tested. -- [@nmattia's post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). +`propagatedBuildInputs` of the given derivations. This is currently implemented +via a recursive (Nix) function, that's prone to reach a state of infinite +recursion. But this risk is currently mitigated using an array of packages we +know don't need any env vars at runtime, and for sure are very much at the +bottom of the list of very common inputs. This is implemented +[here](https://github.com/NixOS/nixpkgs/pull/85103/files#diff-44c2102a355f50131eb8f69fb7e7c18bR75-R131). + +There are other methods of doing this recursive search, but TBH I haven't yet +investigated all of them. For reference and hopefully for an advice, this need +was requested by others and discussed at: + +- [nix issue 1245](https://github.com/NixOS/nix/issues/1245). +- [Interesting idea by @aszlig at nix issue + 1245](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781). +- [@nmattia's + post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). - [Discourse thread](https://discourse.nixos.org/t/any-way-to-get-a-derivations-inputdrvs-from-within-nix/7212/3). # Future work From d75c73ef9f6e7ee4e0a91cedb7e92a024c199ae7 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Aug 2020 16:17:13 +0300 Subject: [PATCH 15/37] Last (?) rephrasings --- rfcs/0075-declarative-wrappers.md | 76 +++++++++++++++---------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 6ae0a943f..694ae0e15 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -44,17 +44,16 @@ the environments to make the transition easier to review. @rnhmjoj & @timokau How unfortunate it is that Python's `buildEnv` doesn't know to do anything besides setting `NIX_PYTHONPATH` - it knows nothing about other -env vars, which is totally legitimate for dependencies of the environment to -rely upon runtime. Declarative wrappers don't care about the meaning of env -vars - all of them are treated equally, considering all of the inputs of a -derivation equally. +env vars, which some deps rely upon when eventually used. Declarative wrappers +don't care about the meaning of env vars - all of them are treated equally, +considering all of the inputs of a derivation equally. - [pull 75851](https://github.com/NixOS/nixpkgs/pull/75851) - [issue 87667](https://github.com/NixOS/nixpkgs/issues/87667) Fixable with our current wrapping tools (I guess?) but it's unfortunate that we have to trigger a rebuild of VLC and potentially increase it's closure size, -just because of a missing env var for some users. If only our wrapping +just because of a missing env var for only _some_ users. If only our wrapping requirements were accessible via Nix attrsets, we could have instructed our modules to consider this information when building the wrappers of the packages in `environment.systemPackages`. @@ -78,51 +77,52 @@ and I guess we don't wrap HPLIP because not everybody want to use these binaries and hence want these GUI deps in their closure (if they were wrapped with a -setup hook)? Declarative wrappers would allow some users to use the wrapped -binaries and others not need it, via an override or a NixOS config flag, -without triggering a rebuild of HPLIP itself. +setup hook)? Declarative wrappers would allow _some_ users to use the wrapped +binaries and others not to need this wrapping. Via an override or a NixOS +config flag, without triggering a rebuild of HPLIP itself, these users would be +easily satisfied. ## Orchestrating wrapping hooks - [issue 78792](https://github.com/NixOS/nixpkgs/issues/78792) -@worldofpeace you are correct. All of these setup-hooks are a mess, but at -least we have documented, yet not totally implemented this section of the -manual +@worldofpeace you are correct. All of these setup-hooks are a mess. At least we +have documented, (yet not totally implemented) this section of the manual https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues-double-wrapped Declarative wrappers will deprecate the usage of our shell based hooks and will wrap all executables automatically according to their needs, without requiring -the contributor a lot of knowledge of the wrapping system. +the contributor a lot of knowledge of the wrapping system. Also, double +wrappings will become a problem of the past. - [issue 86369](https://github.com/NixOS/nixpkgs/issues/86369) -@ttuegel I get the sense [you support this -idea](https://github.com/NixOS/nixpkgs/issues/86369#issuecomment-626732191). -But for anyone else interested, the issue is a bit complex, so once you'll read -the design of this RFC, and see examples of what the POC implementation of -declarative wrappers [is capable +@ttuegel I get the sense [you support this idea of declarative +wrappers](https://github.com/NixOS/nixpkgs/issues/86369#issuecomment-626732191). +For anyone else interested in a summary, the issue is a bit complex, so once +you'll read the design of this RFC, and see examples of what the POC +implementation of declarative wrappers [is capable of](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666), I hope -you'll see how declarative wrappers can solve this issue. +you'll see how declarative wrappers will solve this issue. ## Issues _possibly_ fixable by declarative wrappers (?) - [pull 61213](https://github.com/NixOS/nixpkgs/pull/61213) -I'm not sure what's the issue there. But, I'm sure that a Nix based builder of -a Python environment should make it easier to control and alter if needed, what -environment is used even by builders, not only user facing Python environments. +I'm not sure what's the issue there. But, I'm sure that a declarative, Nix +based builder of a Python environment, even if this environment is used only +for a build, should make it easier to control and alter it's e.g `$PATH`. - [issue 83667](https://github.com/NixOS/nixpkgs/issues/83667) @FRidh I see no reason for Python deps of Python packages to need to be in -`propagatedBuildInputs` and not regular `buildInputs`. I think this was done so -in the past so it'd be easy to know how to wrap them? Declarative wrappers -won't require runtime-env-requiring deps to be only in `propagatedBuildInputs` -or `buildInputs` - it should pick such deps from both lists. Hence, (I think) it -should be possible to make Python's static builds consistent with other -ecosystems. +`propagatedBuildInputs` and not regular `buildInputs` but please correct me if +I'm wrong. I think this was done so in the past so it'd be easy to know how to +wrap them? Declarative wrappers won't require runtime-env-requiring deps to be +only in `propagatedBuildInputs` or `buildInputs` - it should pick such deps +from both lists. Hence, (I think) it should be possible to make Python's static +builds consistent with other ecosystems. - [issue 86054](https://github.com/NixOS/nixpkgs/issues/86054) @@ -163,7 +163,7 @@ $ nix why-depends -f. kdeconnect kdeframeworks.kconfigwidgets.dev ``` Also similar (but possibly fixable by moving `gobject-introspection` to a -different inputs list? +different inputs list?): ``` $ nix why-depends -f. beets gobject-introspection.dev @@ -176,7 +176,7 @@ $ nix why-depends -f. beets gobject-introspection.dev - [issue 60260](https://github.com/NixOS/nixpkgs/issues/60260) -General, justified complain about wrappers. +General, justified complaint about wrappers. - [issue 95027](https://github.com/NixOS/nixpkgs/issues/95027) - [issue 23018](https://github.com/NixOS/nixpkgs/issues/23018) @@ -205,7 +205,7 @@ interface similar to [`wrapMpv`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/video/mpv/wrapper.nix#L9-L23) and [`wrapNeovim`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/editors/neovim/wrapper.nix#L11-L24) -which will accept a single derivation or an array of them and it'll wrap all of +which will accept a single derivation or an array of them and will wrap all of their executables with the proper environment, based on their inputs. `wrapGeneric` should iterate recursively all `buildInputs` and @@ -319,9 +319,9 @@ Perhaps our shell hooks _can_ be fixed / improved, and we could help make it easier to debug them via `NIX_DEBUG`. Then it might help us track down e.g why environment variables are added twice etc. Still though, this wouldn't solve half of the other issues presented here. Most importantly, the shell hooks rely -upon being in the inputs during build of the original derivation, hence mere -changes to an environment may trigger rebuilds that take a lot of time and -resources from avarage users. See [this +upon being in the inputs during build of the original derivation. Hence, mere +requests for changes to an environment a wrapper sets, trigger rebuilds that +take a lot of time and resources from average users. See [this comment](https://github.com/NixOS/nixpkgs/pull/88136#issuecomment-632674653). # Unresolved questions @@ -330,13 +330,13 @@ comment](https://github.com/NixOS/nixpkgs/pull/88136#issuecomment-632674653). The POC implementation does 1 thing which I'm most sure could be done better, and that's iterating **recursively** all `buildInputs` and `propagatedBuildInputs` of the given derivations. This is currently implemented -via a recursive (Nix) function, that's prone to reach a state of infinite -recursion. But this risk is currently mitigated using an array of packages we -know don't need any env vars at runtime, and for sure are very much at the -bottom of the list of very common inputs. This is implemented +with a recursive (Nix) function, prone to reach a state of infinite recursion. +This risk is currently mitigated using an array of packages we know don't need +any env vars at runtime, and for sure are very much at the bottom of the list +of all Nixpkgs' dependency graph. This part is implemented [here](https://github.com/NixOS/nixpkgs/pull/85103/files#diff-44c2102a355f50131eb8f69fb7e7c18bR75-R131). -There are other methods of doing this recursive search, but TBH I haven't yet +There are other methods of doing this recursive search, but I haven't yet investigated all of them. For reference and hopefully for an advice, this need was requested by others and discussed at: From 94038bd0038d665bb458394c9713da02b9fb8091 Mon Sep 17 00:00:00 2001 From: Frederik Rietdijk Date: Thu, 20 Aug 2020 13:32:59 +0200 Subject: [PATCH 16/37] 75: Introduction on run-time dependencies and wrappers. The question why wrappers are needed to begin with was not yet answered. It's also typically not the preferred choice so acknowledge that. --- rfcs/0075-declarative-wrappers.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 694ae0e15..08eb23699 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -21,6 +21,37 @@ documented good enough and not easily debug-able shell hooks. # Motivation [motivation]: #motivation +Every Nix build produced is stored in a separate path in the store. For a +build to find its runtime dependencies purely, they need to be hardcoded in +the build. Thus, a complete dependency specification is needed. + +In case of compiled languages this process is already largely automated; +absolute paths to shared libraries are encoded in the `rpath` of binaries. +Executables that are invoked by binaries need to be dealt with manually, +however. The preferred choice is here to manually patch the source so that +binaries are invoked using absolute paths as well. + +This is not always trivial and thus a common work-around is to wrap executables, +setting `$PATH` to include the locations of dependencies. This is a pragmatic +solution that typically works fine, but it comes with a risk: *environment +variables leak*. Executables that shell out may pass along the modified +variables, causing at times unwanted behaviour. + +Programs written in interpreted languages tend to import their runtime +dependencies using some kind of search path. E.g., in case of Python there is a +process for building up the `sys.path` variable which is then considered when +importing modules. Like with compiled languages, the preferred choice would also +here be to embed the absolute paths in the code, which is often not done. Note +that in case of Python this *is* done. Like with compiled languages, programs +may shell out and likewise the preferred solution is to patch the invocations to +use absolute paths. Similarly, in case a programs wants to `dlopen` a shared +library this should be patched to include an absolute path, instead of using +`LD_LIBRARY_PATH`. + +It is recognized that wrappers setting environment variables are typically not +the preferred choice because of the above mentioned leakage risk, however, often +there is simply not a better or reasonable alternative available. + We have numerous issues regarding wrappers and our wrapper shell hooks. Here's a list of them, sorted to categories. From ffacf5f06f3ee4de54317dca30ba1fd49e1d1c62 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 30 Aug 2020 15:49:08 +0300 Subject: [PATCH 17/37] Add SANE_CONFIG_DIR issue to motivation --- rfcs/0075-declarative-wrappers.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 08eb23699..62a4c017f 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -225,6 +225,29 @@ both ideas could be integrated into an alternative, simpler creation method of binary wrappers. See [my comment](https://github.com/NixOS/nixpkgs/pull/95569#issuecomment-674508806). +- `hardware.sane` module installs sane impurly +- [issue 90201](https://github.com/NixOS/nixpkgs/issues/90201) +- [issue 90184](https://github.com/NixOS/nixpkgs/issues/90184) + +The current way NixOS enables to configure access to scanner, is via the +`hardware.sane` module, which interacts [just a +bit](https://github.com/NixOS/nixpkgs/blob/5d8dd5c2598a74761411bc9bef7c9111d43d2429/nixos/modules/services/hardware/sane.nix#L34) +wish `saned`, but mostly does nothing besides setting `SANE_CONFIG_DIR` and +`LD_LIBRARY_PATH`. This is bad because: + +1. Running `nixos-rebuild` after a change to sane or it's configuration, + requires the user to logout and login, to see effects of the changes. + +2. +[Apparently](https://github.com/NixOS/nixpkgs/issues/90201#issuecomment-683304279) +`LD_LIBRARY_PATH` is cleared when an application runs with extra capabilities. +This causes the current gnome-shell wayland wrapper to unset the +`LD_LIBRARY_PATH` many scanner related programs now rely upon. + +Declarative wrappers should enable us to make such applications that use a +`SANE_CONFIG_DIR` and `LD_LIBRARY_PATH` to be configured during the wrap phase, +and get rid of these global environment variables. + # Detailed design [design]: #detailed-design From f754ad797f612dc3aded83604b9e2f0d9672a640 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Tue, 8 Sep 2020 20:47:11 +0300 Subject: [PATCH 18/37] Mark @nmattia's idea as unsuitable because of IFD. --- rfcs/0075-declarative-wrappers.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 62a4c017f..144296ac4 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -397,8 +397,9 @@ was requested by others and discussed at: - [nix issue 1245](https://github.com/NixOS/nix/issues/1245). - [Interesting idea by @aszlig at nix issue 1245](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781). -- [@nmattia's - post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). +- ~~[@nmattia's + post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html)~~ - + Using it will require IFD. - [Discourse thread](https://discourse.nixos.org/t/any-way-to-get-a-derivations-inputdrvs-from-within-nix/7212/3). # Future work From 4ab0448facdbc9bfb46f2c5cb506d83286c6fa4e Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Fri, 11 Sep 2020 09:47:45 +0300 Subject: [PATCH 19/37] Revert "Mark @nmattia's idea as unsuitable because of IFD." This reverts commit f754ad797f612dc3aded83604b9e2f0d9672a640. --- rfcs/0075-declarative-wrappers.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 144296ac4..62a4c017f 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -397,9 +397,8 @@ was requested by others and discussed at: - [nix issue 1245](https://github.com/NixOS/nix/issues/1245). - [Interesting idea by @aszlig at nix issue 1245](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781). -- ~~[@nmattia's - post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html)~~ - - Using it will require IFD. +- [@nmattia's + post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). - [Discourse thread](https://discourse.nixos.org/t/any-way-to-get-a-derivations-inputdrvs-from-within-nix/7212/3). # Future work From 60d3825fdd4e6574b7e5d70264445d1c801368c6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 19 Nov 2020 15:26:50 +0100 Subject: [PATCH 20/37] Add shepherds --- rfcs/0075-declarative-wrappers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 62a4c017f..e75fa3240 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -3,7 +3,7 @@ feature: Declarative Wrappers start-date: (fill me in with today's date, YYYY-MM-DD) author: Doron Behar co-authors: (find a buddy later to help out with the RFC) -shepherd-team: (names, to be nominated and accepted by RFC steering committee) +shepherd-team: @FRidh, @lheckemann, @edolstra shepherd-leader: (name to be appointed by RFC steering committee) related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/pull/85103). --- From f3416a00a671202028ec4bf1602e9c3cbebd1633 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 16:50:19 +0200 Subject: [PATCH 21/37] Remove out scope Nixpkgs issue #32790 from motivation Switching to a separate wrapper derivation for every package that uses GLib is a bloat, as discussed in the RFC meeting. --- rfcs/0075-declarative-wrappers.md | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index e75fa3240..01a2dac21 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -55,19 +55,6 @@ there is simply not a better or reasonable alternative available. We have numerous issues regarding wrappers and our wrapper shell hooks. Here's a list of them, sorted to categories. -## Closure related - -- [issue 95027](https://github.com/NixOS/nixpkgs/issues/95027) - -@jtojnar & @yegortimoshenko How -hard would it be to test all of our wrapped `gobject-introspection` using -packages that the equivalent, `GI_GIR_PATH` environment should work? If our -wrappers were declarative, and they were a separate derivation, at least we -wouldn't have to rebuild tons of packages to do so - we'd have to rebuild only -the wrappers. Plus, since all of the environment is available to us via -`builtins.toJSON`, it should be possible to write a script that will compare -the environments to make the transition easier to review. - ## Missing environment - [pull 83321](https://github.com/NixOS/nixpkgs/pull/83321) From c0928488d594e774018331ced6b69c7af183271e Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 17:14:15 +0200 Subject: [PATCH 22/37] Add date to header --- rfcs/0075-declarative-wrappers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 01a2dac21..8969618c7 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -1,6 +1,6 @@ --- feature: Declarative Wrappers -start-date: (fill me in with today's date, YYYY-MM-DD) +start-date: 2020-08-16 author: Doron Behar co-authors: (find a buddy later to help out with the RFC) shepherd-team: @FRidh, @lheckemann, @edolstra From 732246db26abd0be1994668f41b25eca6e10d7f9 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 17:14:36 +0200 Subject: [PATCH 23/37] Mention a new hplip wrapping issue --- rfcs/0075-declarative-wrappers.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 8969618c7..c4ab111af 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -92,6 +92,7 @@ and `git-remote-hg` and `qttools` are not wrapped properly. - [issue 86048](https://github.com/NixOS/nixpkgs/issues/86048) +- [issue 114051](https://github.com/NixOS/nixpkgs/issues/114051) I guess we don't wrap HPLIP because not everybody want to use these binaries and hence want these GUI deps in their closure (if they were wrapped with a From 67b002b51f6c8bfbca73712bafd9c38e3fd06898 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 17:15:01 +0200 Subject: [PATCH 24/37] Update link to gnome docs --- rfcs/0075-declarative-wrappers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index c4ab111af..17e9f5584 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -106,8 +106,8 @@ easily satisfied. - [issue 78792](https://github.com/NixOS/nixpkgs/issues/78792) @worldofpeace you are correct. All of these setup-hooks are a mess. At least we -have documented, (yet not totally implemented) this section of the manual -https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues-double-wrapped +have documented, (yet not totally implemented) [this section of the +manual](https://github.com/NixOS/nixpkgs/blob/2df97e4b0ab73f0087af2e6f33e694140150db1b/doc/languages-frameworks/gnome.section.md#L120-L166) Declarative wrappers will deprecate the usage of our shell based hooks and will wrap all executables automatically according to their needs, without requiring From cac0cbb1564ef1abc23767eb2196de0c8ce3f6d2 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 17:15:37 +0200 Subject: [PATCH 25/37] Better explain Nixpkgs issue #86369 --- rfcs/0075-declarative-wrappers.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 17e9f5584..a2d698aaa 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -116,14 +116,9 @@ wrappings will become a problem of the past. - [issue 86369](https://github.com/NixOS/nixpkgs/issues/86369) -@ttuegel I get the sense [you support this idea of declarative -wrappers](https://github.com/NixOS/nixpkgs/issues/86369#issuecomment-626732191). -For anyone else interested in a summary, the issue is a bit complex, so once -you'll read the design of this RFC, and see examples of what the POC -implementation of declarative wrappers [is capable -of](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666), I hope -you'll see how declarative wrappers will solve this issue. - +@ttuegel with declarative wrappers, we can symlink all qt plugins into 1 +directory and wrap the executable with only 1 `QT_PLUGIN_PATH` in their +environment, which should decrease the plugin load of every qt package. ## Issues _possibly_ fixable by declarative wrappers (?) From 7b0c9e11fa1c8f13f1789446c70d4fb5c8483b8e Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 17:17:00 +0200 Subject: [PATCH 26/37] Remove some not directly related issues to wrapping --- rfcs/0075-declarative-wrappers.md | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index a2d698aaa..45f85a412 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -128,22 +128,6 @@ I'm not sure what's the issue there. But, I'm sure that a declarative, Nix based builder of a Python environment, even if this environment is used only for a build, should make it easier to control and alter it's e.g `$PATH`. -- [issue 83667](https://github.com/NixOS/nixpkgs/issues/83667) - -@FRidh I see no reason for Python deps of Python packages to need to be in -`propagatedBuildInputs` and not regular `buildInputs` but please correct me if -I'm wrong. I think this was done so in the past so it'd be easy to know how to -wrap them? Declarative wrappers won't require runtime-env-requiring deps to be -only in `propagatedBuildInputs` or `buildInputs` - it should pick such deps -from both lists. Hence, (I think) it should be possible to make Python's static -builds consistent with other ecosystems. - -- [issue 86054](https://github.com/NixOS/nixpkgs/issues/86054) - -@ttuegel TBH I can't tell if declarative wrappers might help, but I'm linking -this issue here because @worldofpeace wrote it might be related to wrappers? -Feel free to suggest removing this in the RFC review. - - [issue 49132](https://github.com/NixOS/nixpkgs/issues/49132) - [issue 54278](https://github.com/NixOS/nixpkgs/issues/54278) - [issue 39493](https://github.com/NixOS/nixpkgs/issues/39493) From 034a13e50eb6e52ee6cceda0b66695d9181a5f22 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Wed, 24 Feb 2021 20:08:37 +0200 Subject: [PATCH 27/37] Rewrite most of RFC after 1st meeting --- rfcs/0075-declarative-wrappers.md | 238 +++++++++++++++--------------- 1 file changed, 115 insertions(+), 123 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 45f85a412..7227cd546 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -12,11 +12,12 @@ related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/ [summary]: #summary Manage the environment of wrappers declaratively and deprecate shell based -methods for calculating runtime environment of packages. Make wrappers a -separate derivation so that mere changes to the environment will not trigger a -rebuild. Make it easier to debug why env vars are added to an executable, by -using Nix as the language to evaluate what env vars are needed, instead of not -documented good enough and not easily debug-able shell hooks. +methods for calculating runtime environment of packages. Make it easier to +debug why env vars are added to an executable, by storing this information +inside `/nix/store/.../nix-support/` of the dependencies that using them +requires require runtime environment. Create a new `makeWrapperAuto` hook that +will make the `fixupPhase` read all of the deps environment that's needed and +automatically wrap the executables with the proper environment. # Motivation [motivation]: #motivation @@ -218,120 +219,132 @@ and get rid of these global environment variables. # Detailed design [design]: #detailed-design -The current design is roughly implemented at -[pull 85103](https://github.com/NixOS/nixpkgs/pull/85103) . +The end goal is to make the experience of getting a derivation wrapped as +automatic as possible. A derivation that needs a certain environment to run, +will put the dependency that is linked to this environment variable in +`envInputs` and `mkDerivation`'s standard `fixupPhase`. As a start, we'll (me +probably) will introduce a new `makeWrapperAuto` setup hook that will take care +in the way described as follows. + +As a start, we'll need to think about all packages in Nixpkgs that using them +requires some environment variables to be set. Every such package will put in +`$dev/nix-support/wrappers.json` a list of environment variables that are +"linked" to this package. "linked" means that using this package requires these +environment variables to be set in runtime. + +`makeWrapperAuto` will traverse all `buildInputs` and `propagatedBuildInputs` +of a derivation, and look for a `wrappers.json` file in these inputs. It will +collect all the environment variables that need to be set in the resulting +executables, by merging all of the values of the environment variables in all +of the inputs' `wrappers.json` files. `wrappers.json` might look like this for +a given package: + +```json +{ + "GI_TYPELIB_PATH": [ + "/nix/...gobject-introspection.../...", + "/nix/...librsvg.../...", + ], + "GIO_EXTRA_MODULES": [ + "/nix/...dconf.../lib/gio/modules" + ], + "XDG_DATA_DIRS": [ + "/nix/...gtk+3.../...", + "/nix/...gsettings-desktop-schemas.../..." + ], + "GDK_PIXBUF_MODULE_FILE": "/nix/...librsvg.../lib/gdk.../loaders.cache", +} +``` -The idea is to have a Nix function, let us call it `wrapGeneric`, with an -interface similar to -[`wrapMpv`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/video/mpv/wrapper.nix#L9-L23) -and -[`wrapNeovim`](https://github.com/NixOS/nixpkgs/blob/a5985162e31587ae04ddc65c4e06146c2aff104c/pkgs/applications/editors/neovim/wrapper.nix#L11-L24) -which will accept a single derivation or an array of them and will wrap all of -their executables with the proper environment, based on their inputs. - -`wrapGeneric` should iterate recursively all `buildInputs` and -`propagatedBuildInputs` of the input derivation(s), and construct an attrset with -which it'll calculate the necessary environment of the executables. Then either -via `wrapProgram` or a better method, it'll create the wrappers. - -A contributor using `wrapGeneric` shouldn't _care_ what type of wrapping needs -to be performed on his derivation's executables - whether these are Qt related -wrappings or a Gtk / gobject related. `wrapGeneric` should know all there is to -know about environment variables every library / input may need during runtime, -and with this information at hand, construct the necessary wrapper. - -In order for `wrapGenric` to know all of this information about our packaged -libraries - the information about runtime env, we need to write in the -`passthru`s of these libraries, what env vars they need. Such information was -added in the POC pull at [commit -@6283f15](https://github.com/NixOS/nixpkgs/pull/85103/commits/6283f15bb9b65af64571a78b039115807dcc2958). - -Additional features / improvements are [already -available](https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-614195666) -in the POC pull. For example: - -- It should be **impossible** for multi-value env vars to have duplicates, as - that's guaranteed by Nix' behavior when constructing arrays / attrsets. -- Asking the wrapper creator to use more links and less colon-separated values - in env vars - should help avoid what [pull - 84689](https://github.com/NixOS/nixpkgs/pull/84689) fixed. +The information found inside an input's `wrappers.json` will include all the +information about wrappers found in the input's inputs, and so on. Thus in +contrast to the [POC Nixpkgs PR](https://github.com/NixOS/nixpkgs/pull/85103) +and the [original design of the +RFC](https://github.com/doronbehar/rfcs/blob/60d3825fdd4e6574b7e5d70264445d1c801368c6/rfcs/0075-declarative-wrappers.md#L251), +prior to [the 1st +meeting](https://github.com/NixOS/rfcs/pull/75#issuecomment-760942876), +traversing all the inputs and the inputs' inputs, will not happen during eval +time and only partly, during build time - every package already built will be +able to give another package all the information there needs to know about the +related environment variables. + +Most of the work to do will be: + +1. Gather information about what environment variables are "linked" to each + package, and edit these derivations to include a `wrappers.json`, with + `makeWrapperAuto`. +2. Design the `makeWrapperAuto` shell hook: + - It should introduce a shell function (to be called `wrappersInfo`) that + will allow piping a JSON string from `builtins.toJSON` and spit a + `wrappers.json` that will include both what was piped into it, and the + content from the package's inputs' `wrappers.json`. + - It should make the executables in `$out/bin/` get wrapped according to + what's currently in this package's `wrappers.json`, during `fixupPhase`. + - The above should be also possible to do manually for executables outside + `$out/bin/` with say adding to a derivation a Nix variable: -# Examples and Interactions -[examples-and-interactions]: #examples-and-interactions +```nix + wrapExtraPrograms = [ "/libexec/" "/share/scripts" ]; +``` -All examples are copied from and based on the [POC -pull](https://github.com/NixOS/nixpkgs/pull/85103). +3. Most of the packages with linked environment variables, have lots of reverse + dependencies, so once `makeWrapperAuto` is ready, it'd nice to have a hydra + job that will build all of these packages with the `wrappers.json` file in + them. For instance these packages include: + - `gdk-pixbuf` + - `gsettings-desktop-schemas` + - `pango` + - `gtk3` -Here's a new method of creating a python environment: +# Examples and Interactions +[examples-and-interactions]: #examples-and-interactions -```nix - my-python-env = wrapGeneric python3 { - linkByEnv = { - # you can use "link" here and that will link only the necessary files - # to the runtime environment. - PYTHONPATH = "linkPkg"; - }; - extraPkgs = with python3.pkgs; [ - matplotlib - ]; - wrapOut = { - # tells wrapGeneric to add to the wrappers this value for PYTHONPATH. - # Naturally, this should play along with the values given in - # linkByEnv.PYTHONPATH. - PYTHONPATH = "$out/${python3.sitePackages}"; - }; - }; -``` +When switching to `makeWrapperAuto` from `makeWrapper` there shouldn't be +manual usage of `wrapProgram` for most cases. A package that uses `wrapProgram` +should be able to switch to `wrappersInfo` and declare any nontrivial +environment variables with it to get propagated to reverse dependencies and to +it's executables automatically. -Consider a package is wrapped without directly making accessible the unwrapped -derivation. Meaning, say `all-packages.nix` has: +Currently I imagine the usage of `wrappersInfo` (the name can be debated) as +so: ```nix - my-awesome-pkg = wrapGeneric (callPackage ../applications/my-awesome-pkg { }) { }; + # Propagate GST plugins' path + postInstall = '' + echo "${builtins.toJSON { + GST_PLUGIN_SYSTEM_PATH_1_0 = [ + # @out@ should be expanded by `wrappersInfo` to what's in `$out`, see: + # https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-613071343 + "@out@/lib/gstreamer-1.0" + ]; + }}" | wrappersInfo + ''; ``` -Assuming the user knows `my-awesome-pkg` is wrapped with `wrapGeneric`, they would -need to use an overlay like this, to override the unwrapped derivation: +`wrapQtAppsHook` and `wrapGAppsHook` should be replaced with `makeWrapperAuto` +while enable derivations to get rid of well known workarounds such as: ```nix -self: super: - -{ - my-awesome-pkg = super.wrapGeneric ( - super.my-awesome-pkg.unwrapped.overrideAttrs(oldAttrs: { - preFixup = '' - overriding preFixup from an overlay!! - ''; - }) - ) {}; -} + # hook for gobject-introspection doesn't like strictDeps + # https://github.com/NixOS/nixpkgs/issues/56943 + strictDeps = false; ``` -And to override the wrapper derivation, e.g to add new optional features not -strictly necessary (as in [pull -83482](https://github.com/NixOS/nixpkgs/pull/83482)), it should be possible -using: +And: ```nix -self: super: - -{ - my-awesome-pkg = super.wrapGeneric super.my-awesome-pkg.unwrapped { - extraPkgs = [ - super.qt5.certain-qt-plugin - ]; - }; -} + preFixup = '' + makeWrapperArgs+=("''${qtWrapperArgs[@]}") + ''; ``` # Drawbacks [drawbacks]: #drawbacks -The current design is heavily based on Nix, and knowing how to write and debug -Nix expressions is a skill not everyone are akin to learn. Also, overriding a -wrapped derivation is somewhat more awkward, due to this. Perhaps this -interface could be improved, and for sure proper documentation written should -help. +Using `wrapProgram` will be simpler then using `wrappersInfo` and it might be +hard to explain why is there no `wrapProgramAuto`. However, this interface +might get improved in design through this RFC or in the future and in any case +proper documentation should help. # Alternatives [alternatives]: #alternatives @@ -339,36 +352,15 @@ help. Perhaps our shell hooks _can_ be fixed / improved, and we could help make it easier to debug them via `NIX_DEBUG`. Then it might help us track down e.g why environment variables are added twice etc. Still though, this wouldn't solve -half of the other issues presented here. Most importantly, the shell hooks rely -upon being in the inputs during build of the original derivation. Hence, mere -requests for changes to an environment a wrapper sets, trigger rebuilds that -take a lot of time and resources from average users. See [this -comment](https://github.com/NixOS/nixpkgs/pull/88136#issuecomment-632674653). +many issues presented above. # Unresolved questions [unresolved]: #unresolved-questions -The POC implementation does 1 thing which I'm most sure could be done better, -and that's iterating **recursively** all `buildInputs` and -`propagatedBuildInputs` of the given derivations. This is currently implemented -with a recursive (Nix) function, prone to reach a state of infinite recursion. -This risk is currently mitigated using an array of packages we know don't need -any env vars at runtime, and for sure are very much at the bottom of the list -of all Nixpkgs' dependency graph. This part is implemented -[here](https://github.com/NixOS/nixpkgs/pull/85103/files#diff-44c2102a355f50131eb8f69fb7e7c18bR75-R131). - -There are other methods of doing this recursive search, but I haven't yet -investigated all of them. For reference and hopefully for an advice, this need -was requested by others and discussed at: - -- [nix issue 1245](https://github.com/NixOS/nix/issues/1245). -- [Interesting idea by @aszlig at nix issue - 1245](https://github.com/NixOS/nix/issues/1245#issuecomment-401642781). -- [@nmattia's - post](https://www.nmattia.com/posts/2019-10-08-runtime-dependencies.html). -- [Discourse thread](https://discourse.nixos.org/t/any-way-to-get-a-derivations-inputdrvs-from-within-nix/7212/3). +Discussing the design I guess, here or in the Nixpkgs PR that will follow this +RFC. # Future work [future]: #future-work -Not that I can think of. +Fix all wrapper related issues declaratively! From edbf315b98b81a4701acc92c15c6c6270df9a27e Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Fri, 19 Mar 2021 12:46:24 +0200 Subject: [PATCH 28/37] Small rephrasings --- rfcs/0075-declarative-wrappers.md | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 7227cd546..7bf71cbee 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -220,11 +220,11 @@ and get rid of these global environment variables. [design]: #detailed-design The end goal is to make the experience of getting a derivation wrapped as -automatic as possible. A derivation that needs a certain environment to run, -will put the dependency that is linked to this environment variable in -`envInputs` and `mkDerivation`'s standard `fixupPhase`. As a start, we'll (me -probably) will introduce a new `makeWrapperAuto` setup hook that will take care -in the way described as follows. +automatic as possible. A derivation that needs some environment variables in +order to work will get these environment variables set in the wrapper by +`mkDerivation`'s standard `fixupPhase`. As a start, we'll (me probably) will +introduce a new `makeWrapperAuto` setup hook that will take care of this in the +way described as follows. As a start, we'll need to think about all packages in Nixpkgs that using them requires some environment variables to be set. Every such package will put in @@ -264,20 +264,21 @@ RFC](https://github.com/doronbehar/rfcs/blob/60d3825fdd4e6574b7e5d70264445d1c801 prior to [the 1st meeting](https://github.com/NixOS/rfcs/pull/75#issuecomment-760942876), traversing all the inputs and the inputs' inputs, will not happen during eval -time and only partly, during build time - every package already built will be -able to give another package all the information there needs to know about the -related environment variables. +time and only partly, during build time - every package already built will +provide it's reverse dependencies all the information they need about +environment variables of itself and of all of it's inputs and it's inputs' +inputs. Most of the work to do will be: 1. Gather information about what environment variables are "linked" to each - package, and edit these derivations to include a `wrappers.json`, with - `makeWrapperAuto`. + package, and edit these derivations to include a `wrappers.json` in them. + This should be done with `makeWrapperAuto` as well, see (2). 2. Design the `makeWrapperAuto` shell hook: - It should introduce a shell function (to be called `wrappersInfo`) that will allow piping a JSON string from `builtins.toJSON` and spit a `wrappers.json` that will include both what was piped into it, and the - content from the package's inputs' `wrappers.json`. + content from the package's various inputs' `wrappers.json` files. - It should make the executables in `$out/bin/` get wrapped according to what's currently in this package's `wrappers.json`, during `fixupPhase`. - The above should be also possible to do manually for executables outside @@ -330,7 +331,7 @@ while enable derivations to get rid of well known workarounds such as: strictDeps = false; ``` -And: +And often seen in Python + Qt programs: ```nix preFixup = '' @@ -344,7 +345,7 @@ And: Using `wrapProgram` will be simpler then using `wrappersInfo` and it might be hard to explain why is there no `wrapProgramAuto`. However, this interface might get improved in design through this RFC or in the future and in any case -proper documentation should help. +proper documentation should help. # Alternatives [alternatives]: #alternatives From 48f91185366576e9ce71301c8e84413b0b1972b7 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Thu, 10 Jun 2021 15:42:59 +0200 Subject: [PATCH 29/37] Add shepherd leader metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim --- rfcs/0075-declarative-wrappers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 7bf71cbee..e36b7ccba 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -4,7 +4,7 @@ start-date: 2020-08-16 author: Doron Behar co-authors: (find a buddy later to help out with the RFC) shepherd-team: @FRidh, @lheckemann, @edolstra -shepherd-leader: (name to be appointed by RFC steering committee) +shepherd-leader: @FRidh related-issues: POC Implementation at [#85103](https://github.com/NixOS/nixpkgs/pull/85103). --- From 73421fdb49bc573c6898433cc1abe7dc50b4201f Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 18 Jul 2021 12:49:04 +0000 Subject: [PATCH 30/37] Apply suggestions by @lheckermann Co-authored-by: Linus Heckemann --- rfcs/0075-declarative-wrappers.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index e36b7ccba..97cdfbc27 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -256,8 +256,8 @@ a given package: } ``` -The information found inside an input's `wrappers.json` will include all the -information about wrappers found in the input's inputs, and so on. Thus in +The information found inside an input's `wrappers.json` will specify the +wrapper information not only for itself, but for all its dependencies (including transitives) as well. Thus, in contrast to the [POC Nixpkgs PR](https://github.com/NixOS/nixpkgs/pull/85103) and the [original design of the RFC](https://github.com/doronbehar/rfcs/blob/60d3825fdd4e6574b7e5d70264445d1c801368c6/rfcs/0075-declarative-wrappers.md#L251), @@ -265,9 +265,8 @@ prior to [the 1st meeting](https://github.com/NixOS/rfcs/pull/75#issuecomment-760942876), traversing all the inputs and the inputs' inputs, will not happen during eval time and only partly, during build time - every package already built will -provide it's reverse dependencies all the information they need about -environment variables of itself and of all of it's inputs and it's inputs' -inputs. +provide its reverse dependencies all the information they need about +environment variables. Most of the work to do will be: @@ -279,10 +278,10 @@ Most of the work to do will be: will allow piping a JSON string from `builtins.toJSON` and spit a `wrappers.json` that will include both what was piped into it, and the content from the package's various inputs' `wrappers.json` files. - - It should make the executables in `$out/bin/` get wrapped according to + - It should wrap the executables in `$out/bin/` according to what's currently in this package's `wrappers.json`, during `fixupPhase`. - The above should be also possible to do manually for executables outside - `$out/bin/` with say adding to a derivation a Nix variable: + `$out/bin/` by setting `wrapExtraPrograms` on the derivation: ```nix wrapExtraPrograms = [ "/libexec/" "/share/scripts" ]; From f0c253398d11fe571889bba216a3d5734c040c1d Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 18 Jul 2021 15:56:06 +0300 Subject: [PATCH 31/37] Rename wrappersInfo -> combineWrappersInfo --- rfcs/0075-declarative-wrappers.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 97cdfbc27..6cd0272de 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -274,7 +274,7 @@ Most of the work to do will be: package, and edit these derivations to include a `wrappers.json` in them. This should be done with `makeWrapperAuto` as well, see (2). 2. Design the `makeWrapperAuto` shell hook: - - It should introduce a shell function (to be called `wrappersInfo`) that + - It should introduce a shell function (to be called `combineWrappersInfo`) that will allow piping a JSON string from `builtins.toJSON` and spit a `wrappers.json` that will include both what was piped into it, and the content from the package's various inputs' `wrappers.json` files. @@ -301,11 +301,11 @@ Most of the work to do will be: When switching to `makeWrapperAuto` from `makeWrapper` there shouldn't be manual usage of `wrapProgram` for most cases. A package that uses `wrapProgram` -should be able to switch to `wrappersInfo` and declare any nontrivial +should be able to switch to `combineWrappersInfo` and declare any nontrivial environment variables with it to get propagated to reverse dependencies and to it's executables automatically. -Currently I imagine the usage of `wrappersInfo` (the name can be debated) as +Currently I imagine the usage of `combineWrappersInfo` (the name can be debated) as so: ```nix @@ -313,11 +313,11 @@ so: postInstall = '' echo "${builtins.toJSON { GST_PLUGIN_SYSTEM_PATH_1_0 = [ - # @out@ should be expanded by `wrappersInfo` to what's in `$out`, see: + # @out@ should be expanded by `combineWrappersInfo` to what's in `$out`, see: # https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-613071343 "@out@/lib/gstreamer-1.0" ]; - }}" | wrappersInfo + }}" | combineWrappersInfo ''; ``` @@ -341,7 +341,7 @@ And often seen in Python + Qt programs: # Drawbacks [drawbacks]: #drawbacks -Using `wrapProgram` will be simpler then using `wrappersInfo` and it might be +Using `wrapProgram` will be simpler then using `combineWrappersInfo` and it might be hard to explain why is there no `wrapProgramAuto`. However, this interface might get improved in design through this RFC or in the future and in any case proper documentation should help. From 391f5d057d5eefd2061ab07525c3650387b462e9 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 18 Jul 2021 15:56:33 +0300 Subject: [PATCH 32/37] Add subtitles for the motivation categories --- rfcs/0075-declarative-wrappers.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 6cd0272de..ea7c0866e 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -56,6 +56,12 @@ there is simply not a better or reasonable alternative available. We have numerous issues regarding wrappers and our wrapper shell hooks. Here's a list of them, sorted to categories. +* [Missing environment variables](#missing-environment) +* [Orchestrating wrapping hooks](#orchestrating-wrapping-hooks) +* [Issues _Possibly_ fixable by declarative wrappers](#issues-possibly-fixable-by-declarative-wrappers-) +* [Unreported Issues](#unreported-issues-afaik) +* [Other Issues](#other-issues) + ## Missing environment - [pull 83321](https://github.com/NixOS/nixpkgs/pull/83321) From fb0dd983a5ff48910e4d89b5882d45b27f4e6bcb Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Fri, 6 Aug 2021 08:40:15 +0000 Subject: [PATCH 33/37] Small rephrasings Co-authored-by: Linus Heckemann --- rfcs/0075-declarative-wrappers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index ea7c0866e..49429ae25 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -328,7 +328,7 @@ so: ``` `wrapQtAppsHook` and `wrapGAppsHook` should be replaced with `makeWrapperAuto` -while enable derivations to get rid of well known workarounds such as: +which enables removing frequent workarounds such as: ```nix # hook for gobject-introspection doesn't like strictDeps @@ -347,7 +347,7 @@ And often seen in Python + Qt programs: # Drawbacks [drawbacks]: #drawbacks -Using `wrapProgram` will be simpler then using `combineWrappersInfo` and it might be +Using `wrapProgram` is simpler than using `combineWrappersInfo` and it might be hard to explain why is there no `wrapProgramAuto`. However, this interface might get improved in design through this RFC or in the future and in any case proper documentation should help. From c51c9c05610875c69274abd8d3d88364e807f06f Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Fri, 6 Aug 2021 11:40:45 +0300 Subject: [PATCH 34/37] Use a list of glob patterns for the wrapping --- rfcs/0075-declarative-wrappers.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 49429ae25..5a2ed213f 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -284,13 +284,15 @@ Most of the work to do will be: will allow piping a JSON string from `builtins.toJSON` and spit a `wrappers.json` that will include both what was piped into it, and the content from the package's various inputs' `wrappers.json` files. - - It should wrap the executables in `$out/bin/` according to - what's currently in this package's `wrappers.json`, during `fixupPhase`. - - The above should be also possible to do manually for executables outside - `$out/bin/` by setting `wrapExtraPrograms` on the derivation: + - It should wrap a list of executables according to a list of glob patterns, + defaulting to `["$outputBin/bin/*" "$outputLib/libexec/**/*"]`, according + to what's currently in this package's `wrappers.json`, during `fixupPhase`. + - It should be possible to override this list of glob patterns, and specify a + new list instead of it. The other list can include glob patterns as well, + or specific executables instead. For example: ```nix - wrapExtraPrograms = [ "/libexec/" "/share/scripts" ]; + wrapPrograms = [ "$outputLib/libexec/${pname}/run-server" "$out/share/scripts/*" ]; ``` 3. Most of the packages with linked environment variables, have lots of reverse From 31a24a0348ab4fe41d39bbf0ac9a7f9a27721cf1 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sat, 14 Aug 2021 12:08:26 +0300 Subject: [PATCH 35/37] Small rephrasings --- rfcs/0075-declarative-wrappers.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 5a2ed213f..752bdcccb 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -15,8 +15,8 @@ Manage the environment of wrappers declaratively and deprecate shell based methods for calculating runtime environment of packages. Make it easier to debug why env vars are added to an executable, by storing this information inside `/nix/store/.../nix-support/` of the dependencies that using them -requires require runtime environment. Create a new `makeWrapperAuto` hook that -will make the `fixupPhase` read all of the deps environment that's needed and +requires runtime environment. Create a new `makeWrapperAuto` hook that will +make the `fixupPhase` read all of the deps environment that's needed and automatically wrap the executables with the proper environment. # Motivation @@ -232,8 +232,8 @@ order to work will get these environment variables set in the wrapper by introduce a new `makeWrapperAuto` setup hook that will take care of this in the way described as follows. -As a start, we'll need to think about all packages in Nixpkgs that using them -requires some environment variables to be set. Every such package will put in +As a start, we'll need to think about all packages in Nixpkgs that require +environment variables to be set at runtime. Every such package will put in `$dev/nix-support/wrappers.json` a list of environment variables that are "linked" to this package. "linked" means that using this package requires these environment variables to be set in runtime. @@ -263,9 +263,10 @@ a given package: ``` The information found inside an input's `wrappers.json` will specify the -wrapper information not only for itself, but for all its dependencies (including transitives) as well. Thus, in -contrast to the [POC Nixpkgs PR](https://github.com/NixOS/nixpkgs/pull/85103) -and the [original design of the +wrapper information not only for itself, but for all its dependencies +(including transitives) as well. Thus, in contrast to the [POC Nixpkgs +PR](https://github.com/NixOS/nixpkgs/pull/85103) and the [original design of +the RFC](https://github.com/doronbehar/rfcs/blob/60d3825fdd4e6574b7e5d70264445d1c801368c6/rfcs/0075-declarative-wrappers.md#L251), prior to [the 1st meeting](https://github.com/NixOS/rfcs/pull/75#issuecomment-760942876), From a6123b925130b01d4a8cbd5f9b6b682402348aa8 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sat, 14 Aug 2021 12:09:01 +0300 Subject: [PATCH 36/37] Explain default behaviour of order and separators Separators of multi value variables. --- rfcs/0075-declarative-wrappers.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 752bdcccb..3c0be8b3a 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -262,6 +262,20 @@ a given package: } ``` +There is a default behaviour that `makeWrapperAuto` will use, for example: + +- The separator between the paths should default to `:`, but for some known + environment variables that use `;` (e.g `LUA_PATH`), `makeWrapperAuto` should + know in advance that this is the separator they need. This information should + be coded in a nix / shell file. +- Some environment variables are not meant to be a list, like + `GDK_PIXBUF_MODULE_FILE`. This information should be understood by + `makeWrapperAuto` because it appears in `wrappers.json` as a single string. +- The default behavior for the order used eventually in the wrapper should be + the order in which the information is gathered, which leaves the lower level + dependencies' paths to the end. If someone reading this RFC can give examples + where a special order might be required, let us know. + The information found inside an input's `wrappers.json` will specify the wrapper information not only for itself, but for all its dependencies (including transitives) as well. Thus, in contrast to the [POC Nixpkgs From a65ac77de0702f6af8c00e177df9793e7218664d Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sat, 14 Aug 2021 12:09:46 +0300 Subject: [PATCH 37/37] Make combineWrappersInfo a nix function that returns a shell command --- rfcs/0075-declarative-wrappers.md | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 3c0be8b3a..b1442dfcc 100644 --- a/rfcs/0075-declarative-wrappers.md +++ b/rfcs/0075-declarative-wrappers.md @@ -333,17 +333,36 @@ so: ```nix # Propagate GST plugins' path - postInstall = '' - echo "${builtins.toJSON { + postInstall = makeWrapperAuto.combineWrappersInfo { + envInfo = { GST_PLUGIN_SYSTEM_PATH_1_0 = [ # @out@ should be expanded by `combineWrappersInfo` to what's in `$out`, see: # https://github.com/NixOS/nixpkgs/pull/85103#issuecomment-613071343 "@out@/lib/gstreamer-1.0" ]; - }}" | combineWrappersInfo + }; + }; ''; ``` +The above will put a shell command in `postInstall` that's generated by +`combineWrappersInfo` that will put the environment information into +`wrappers.json`. + +It should be possible to tell `combineWrappersInfo` to use a symlink directory +instead of a big environment variable, by using for example: + +Other nix attributes along `envInfo` such as: + +```nix + envInfo = { + # ... + }; + symlink = { + THIS_ENV_VAR = "to/this-directory/in-out"; + }; +``` + `wrapQtAppsHook` and `wrapGAppsHook` should be replaced with `makeWrapperAuto` which enables removing frequent workarounds such as: