Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC 0075] Declarative wrappers #75

Draft
wants to merge 38 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
ef81400
[RFC 55]: Declarative Wrappers
doronbehar May 10, 2020
b780229
Declarative wrappers
doronbehar Aug 15, 2020
d09a058
Finish motivation
doronbehar Aug 16, 2020
37c7a8c
Remove old RFC file
doronbehar Aug 16, 2020
2a0a630
Add more relevant issues regarding compiling wrappers
doronbehar Aug 16, 2020
c9d7055
Write most of the design and more
doronbehar Aug 16, 2020
abb7609
Small improvements to summary vs motivation
doronbehar Aug 16, 2020
d1cadb6
Rephrase a bit orchestrating motivation section
doronbehar Aug 16, 2020
a570b1a
Add small ()
doronbehar Aug 16, 2020
8db3605
More rephrasings and additions
doronbehar Aug 16, 2020
5ddd60b
Fix possibly fixable title
doronbehar Aug 16, 2020
778bc99
Design sec improvements
doronbehar Aug 16, 2020
44a3b87
Improve examples
doronbehar Aug 16, 2020
0b259fa
Improve ending
doronbehar Aug 16, 2020
d75c73e
Last (?) rephrasings
doronbehar Aug 16, 2020
94038bd
75: Introduction on run-time dependencies and wrappers.
FRidh Aug 20, 2020
0fa64ab
Merge pull request #1 from FRidh/75-intro
doronbehar Aug 25, 2020
ffacf5f
Add SANE_CONFIG_DIR issue to motivation
doronbehar Aug 30, 2020
f754ad7
Mark @nmattia's idea as unsuitable because of IFD.
doronbehar Sep 8, 2020
4ab0448
Revert "Mark @nmattia's idea as unsuitable because of IFD."
doronbehar Sep 11, 2020
60d3825
Add shepherds
edolstra Nov 19, 2020
f3416a0
Remove out scope Nixpkgs issue #32790 from motivation
doronbehar Feb 24, 2021
c092848
Add date to header
doronbehar Feb 24, 2021
732246d
Mention a new hplip wrapping issue
doronbehar Feb 24, 2021
67b002b
Update link to gnome docs
doronbehar Feb 24, 2021
cac0cbb
Better explain Nixpkgs issue #86369
doronbehar Feb 24, 2021
7b0c9e1
Remove some not directly related issues to wrapping
doronbehar Feb 24, 2021
034a13e
Rewrite most of RFC after 1st meeting
doronbehar Feb 24, 2021
edbf315
Small rephrasings
doronbehar Mar 19, 2021
48f9118
Add shepherd leader metadata
lheckemann Jun 10, 2021
73421fd
Apply suggestions by @lheckermann
doronbehar Jul 18, 2021
f0c2533
Rename wrappersInfo -> combineWrappersInfo
doronbehar Jul 18, 2021
391f5d0
Add subtitles for the motivation categories
doronbehar Jul 18, 2021
fb0dd98
Small rephrasings
doronbehar Aug 6, 2021
c51c9c0
Use a list of glob patterns for the wrapping
doronbehar Aug 6, 2021
31a24a0
Small rephrasings
doronbehar Aug 14, 2021
a6123b9
Explain default behaviour of order and separators
doronbehar Aug 14, 2021
a65ac77
Make combineWrappersInfo a nix function that returns a shell command
doronbehar Aug 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Last (?) rephrasings
  • Loading branch information
doronbehar committed Aug 16, 2020
commit d75c73ef9f6e7ee4e0a91cedb7e92a024c199ae7
76 changes: 38 additions & 38 deletions rfcs/0075-declarative-wrappers.md
Original file line number Diff line number Diff line change
@@ -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: