nix-community / nix-direnv

A fast, persistent use_nix/use_flake implementation for direnv [maintainer=@Mic92 / @bbenne10]
MIT License
1.78k stars 101 forks source link

nix-direnv should not bundle nix #451

Closed szlend closed 1 month ago

szlend commented 9 months ago

Re: https://github.com/NixOS/nixpkgs/issues/270741

The nix-direnv derivation wraps direnvrc by prepending the file with NIX_BIN_PREFIX=${pkgs.nix}.

As a result:

In my opinion it doesn't make sense to bundle nix with nix-direnv. I think it's reasonable to expect nix to be in PATH on any system where people would actually want to use nix-direnv.

Mic92 commented 9 months ago

Here is my take: It should be bundled but $PATH should be preferred. Otherwise direnv exec /some/path command will not work in case nix is not in $PATH. Also see reports like: https://github.com/nix-community/nix-direnv/issues/446

Mic92 commented 9 months ago

To improve the situation on nixos: https://github.com/NixOS/nixpkgs/pull/275591/files

szlend commented 9 months ago

Here is my take: It should be bundled but $PATH should be preferred. Otherwise direnv exec /some/path command will not work in case nix is not in $PATH. Also see reports like: #446

That's alright too, though imo it's just an unnecessary dependency. If your system nix version is different, it will cause you to have two different versions of nix in your system closure. I can understand why someone would not have direnv in their PATH, but I'm really struggling to come up with a case where you'd want to use nix-direnv and not have nix in your PATH.

Mic92 commented 9 months ago

Here is my take: It should be bundled but $PATH should be preferred. Otherwise direnv exec /some/path command will not work in case nix is not in $PATH. Also see reports like: #446

That's alright too, though imo it's just an unnecessary dependency. If your system nix version is different, it will cause you to have two different versions of nix in your system closure. I can understand why someone would not have direnv in their PATH, but I'm really struggling to come up with a case where you'd want to use nix-direnv and not have nix in your PATH.

It can happen in non-interactive situations i.e. if you run it from systemd services.

simonzkl commented 9 months ago

This has caused a ton of hard to debug issues in our company where people installed nix-direnv through nix profile and left it out of date. From annoying warnings, to nix-direnv generating an incorrect flake.lock after updating flake.nix. This one was was especially frustrating because it invalidated our binary cache due to some transitive dependencies not being updated along with the root inputs.

bbenne10 commented 9 months ago

Here is my take: It should be bundled but $PATH should be preferred. Otherwise direnv exec /some/path command will not work in case nix is not in $PATH. Also see reports like: #446

Why would someone be using nix-direnv without nix on $PATH? What is the use case there?

(Further - why shouldn't direnv be on $PATH? You can come up with a bunch of esoteric reasons - but "it used to work" isn't generally a good enough reason to continue to support the scenario)

I'm :+1: on removing the bundled nix. It's confusing because the nix version may not line up with your profile's nix version (and you really have very little other way to investigate what is going on without digging into the source, which is very unfortunate) and it is unnecessary because we no longer need a bleeding-edge nix for the commands we run so just about any version available will do.

szlend commented 8 months ago

Happy to contribute this change if we can make a final decision.

It seems the only disagreement is on whether or not to include a fallback nix package? I still think it's unnecessary. I'd rather see nix-direnv fail than run with an unexpected version of nix in those contrived use cases. But I don't really mind it all that much either. We can package it as a fallback and reconsider later.

adrian-gierakowski commented 5 months ago

I just got bit by it twice, once using nix-direnv on NixOs via home-manager module which doesn't implement the same workaround as the nixos modules (as reported here). And another time helping my work colleague install nix-direnv on MacOS (we use nix v2.21 which doesn't suffer from the dot-in-path problem, while nix-direnv invoked whatever is the current default on unstable which still rejects dotted paths).

How about offer 2 packages: one with bundled nix (for people who need it) and one without?

bbenne10 commented 5 months ago

@szlend: If you're still willing and available: let's compromise with @Mic92's solution: bundle a version of nix but prefer the one in $PATH. Waiting for perfect is going to take forever and "good" is good enough (until it isn't, but we'll cross that bridge when we get there)

Mic92 commented 5 months ago

We need to fixup the packaging than by excluding nix from resolve somehow. Maybe like this

NIX_BIN=

if command -v nix 2>/dev/null; then
  NIX_BIN=nix
else
  NIX_BIN=${NIX_BIN-:nix} # make sure it's always defined
fi

and than do a sed -i 's!^NIX_BIN=$!${pkgs.nix}/bin/nix!' direnvrc in our package build.

euphemism commented 1 month ago

Bump.

lf- commented 1 month ago

nix-direnv ignoring ambient nix causes repeated support problems while working on Lix because of it trying to evaluate things with an unintended nix implementation and it would help a lot if it would use the ambient nix.

I don't think there needs to be a fallback nix package included either; I don't see basically any instance in which a system nix is missing from PATH and where pulling a version of nix out of a hat results in a usable nix with a correctly permissioned store &c. I would be happy to be wrong though.