diff --git a/rfcs/0075-declarative-wrappers.md b/rfcs/0075-declarative-wrappers.md index 379ab3b..512511c 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.