NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.1k stars 14.14k forks source link

doc: base installed tools should not be wrapped #209374

Open urandom2 opened 1 year ago

urandom2 commented 1 year ago

It is unreasonable amounts of work to add those packages to all packages that require them. The simpler and better to maintain solution for nixpkgs maintainers is to add those packages on systems that do not ship them.

And PATH has a size limit. It cannot be expanded indefinitely and if every program adds the same or similar entries without deduping them some badly written programs will crash.

_Originally posted by @SuperSandro2000 in https://github.com/NixOS/nixpkgs/pull/208333#discussion_r1063508244_

this notion of "not wrapping base tools" is novel, and does not appear documented in the manual or other nixpkgs usage. should we add something so that other maintainers are not also confused?

symphorien commented 1 year ago

I don't agree. There are a number of situations where programs are run in nearly empty paths (notably systemd units), so wrapping even basic tools is useful. Besides, makeWrapper now dedupes entries, iirc.

urandom2 commented 1 year ago

as a meta point, maintainers have the option to also replace bare program names with /nix/store paths, as is done for shared objects; generally this is a lot of work, and wrapping is just easier, but for well described languages it is possible to do this automatically: bash has resholve, see docs.

peterhoeg commented 1 year ago

Instead of wrapping by adding each individual item to PATH, how about building an environment using symlinkJoin or similar and than add that instead? That's what I've been doing in general to avoid exactly the issue of PATH length limitations.

urandom2 commented 1 year ago

interesting, it might be nice to have that as a turnkey function like:

symlinkBinPath = paths: symlinkJoin { name = "bin-path"; inherit paths; };
"wrapProgram $out/bin/cmd --prefix PATH : ${symlinkBinPath [
  coreutils
  gnugrep
  git
]}"
SuperSandro2000 commented 1 year ago

I don't agree. There are a number of situations where programs are run in nearly empty paths (notably systemd units), so wrapping even basic tools is useful. Besides, makeWrapper now dedupes entries, iirc.

systemd units have a default of coreutils, findutils, gnugrep, gnused, systemd for bin and sbin. Wrapping those tools is not required.

as a meta point, maintainers have the option to also replace bare program names with /nix/store paths, as is done for shared objects; generally this is a lot of work, and wrapping is just easier

That is not generally correct. For example for libraries it is general way easier to hardcode the store path to avoid having to wrap every program that links against the program.

Instead of wrapping by adding each individual item to PATH, how about building an environment using symlinkJoin or similar and than add that instead? That's what I've been doing in general to avoid exactly the issue of PATH length limitations.

Thats a really good idea. We should be able to migrate almost all makeBinPath to that.


You're code example has several problems which make it harder to work with NixOS. If we formalize this, it is the perfect time to do it right.

How about we expose the default path in top-level as some core package and advice people to install that on anything that is not a recent linux with gnu tools?

urandom2 commented 1 year ago

They ignore users PATH priority and tool choice. If I have another compatible implementation in my PATH already or use a wrapper then they are overwritten. If there is no need to overlay a binary such fallback paths should be always suffixed. We had this problem with xdg-utils already and suffix is in most cases the correct choice.

I am pretty mixed on this one. On the one hand, sure some users may have configured ssh to their liking with builtin configuration options, but plenty of non-nixos systems also bundle fundamentally incompatible system utilities. I am concerned about assuming that an implementation is "compatible", simply because it is in PATH. Another situation is tools that both link against a shared object and shell out to an executable. In this case we pin to the nixpkgs version for the library, but defer to PATH for the binary.

it makes delivering security updates for programs harder because they will trigger more rebuilds for more packages. See the recent openssh removal from rustPlatforms inner workings.

Mind giving a pointer to this?

it will make using overlays harder because they would cause more rebuilds which are not justified.

IIUC, this issue is greater than just PATH wrapping, since updating a shared object dependency will trigger a similarly "unjustified build". If the old build could just reference the new, compatible library out of /usr/lib/, then a full rebuild would not be necessary.

maybe we can implement this in a way to avoid rebuilding packages completely.

Yeah, I got this working a while back by doing all the makeWrapper stuff in a separate derivation that consumed the main one, but was cautioned that this would close to double the number of derivations in nixpkgs. If we are already paying that cost with symlinkJoin trick, it seems reasonable to have that derivation also setup PATH instead of using the main derivation's installPhase. I would even wager we could write it up such that the mkDerviation api need not change too much.

Maybe saving phases to the nix store could help but that's in the work for a long time and will probably not ready in the next months.

Is there an thread tracking this? I would be keen to follow along.

not every program works with wrapping and we are not there yet that the wrappers are completely hidden. Also we want to migrate to binary wrappers by default before starting with this.

Personal curiosity, but should we be using binary wrappers wherever possible today?

How about we expose the default path in top-level as some core package and advice people to install that on anything that is not a recent linux with gnu tools?

@roberth seemed amenable to even going so far as to offering pkgs install as part of the nix install process, over here: https://github.com/NixOS/nix/issues/7566#issuecomment-1374821066. Does somebody want to spawn that new issue, or should I?

SuperSandro2000 commented 1 year ago

Mind giving a pointer to this?

openssh a while ago, also currently git. If we want to deliver security updates faster we cannot wrap everything that might be required into a program.

Generally the more we wrap, the more rebuilds will be triggered unless we move the wrapping to an extra build steps or indicate required binaries in another way that does not trigger a rebuild.

Is there an thread tracking this? I would be keen to follow along.

I don't know the exact title of and couldn't find it.

Personal curiosity, but should we be using binary wrappers wherever possible today?

Yes because eg. on macos a script interpreter must be a binary and cannot be a shellscript. Also they are slightly faster.