Write most of the design and more

This commit is contained in:
Doron Behar 2020-08-16 15:26:48 +03:00
parent 2a0a6306eb
commit c9d70559e0

View file

@ -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.