Skip to content

Use ambient Nix when available, with a two stage fallback#513

Merged
Mic92 merged 2 commits into
nix-community:masterfrom
K900:ambient-nix-with-fallback
Aug 22, 2024
Merged

Use ambient Nix when available, with a two stage fallback#513
Mic92 merged 2 commits into
nix-community:masterfrom
K900:ambient-nix-with-fallback

Conversation

@K900
Copy link
Copy Markdown
Contributor

@K900 K900 commented Aug 21, 2024

First, we try to use the ambient Nix version.
Then, we try to use $NIX_DIRENV_FALLBACK_NIX, which is set by default, but can also be overridden by the user.
Only then, if neither is available, we fail.

Fixes #451.

Copy link
Copy Markdown
Contributor

@bbenne10 bbenne10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me. I appreciate the effort.

I will wait for @Mic92 to have a chance to review before I merge (though they may get around to it first).

Comment thread default.nix
prologue =
(writeText "prologue.sh" ''
NIX_DIRENV_SKIP_VERSION_CHECK=1
NIX_DIRENV_FALLBACK_NIX=''${NIX_DIRENV_FALLBACK_NIX:-${lib.getExe nix}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you mention this environment variable in the README? Otherwise it looks good to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@K900 K900 force-pushed the ambient-nix-with-fallback branch from 8494447 to d7803d9 Compare August 21, 2024 18:54
First, we try to use the ambient Nix version.
Then, we try to use $NIX_DIRENV_FALLBACK_NIX, which is set by default,
but can also be overridden by the user.
Only then, if neither is available, we fail.

Fixes nix-community#451.
@K900 K900 force-pushed the ambient-nix-with-fallback branch from d7803d9 to 94def84 Compare August 21, 2024 18:55
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Aug 21, 2024

@mergify queue

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Aug 21, 2024

queue

🛑 The pull request has been merged manually

Details

The pull request has been merged manually at a3139c8

@K900
Copy link
Copy Markdown
Contributor Author

K900 commented Aug 22, 2024

Is it just broken now...?

@Mic92 Mic92 merged commit a3139c8 into nix-community:master Aug 22, 2024
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Aug 22, 2024

Is it just broken now...?

"required checks" in mergify.yml needs an update.

9999years added a commit to 9999years/nixpkgs that referenced this pull request Sep 4, 2024
Upstream recently changed their resholve solutions somewhat to support
using the ambient Nix on the user's `$PATH` or falling back to the
default nixpkgs stable `nix` otherwise. Let's update our definitions to
match.

See: nix-community/nix-direnv#513
lf- pushed a commit to lf-/nixpkgs that referenced this pull request Oct 14, 2024
Upstream recently changed their resholve solutions somewhat to support
using the ambient Nix on the user's `$PATH` or falling back to the
default nixpkgs stable `nix` otherwise. Let's update our definitions to
match.

See: nix-community/nix-direnv#513
(cherry picked from commit ea8ab50)
lf- added a commit to lf-/nixpkgs that referenced this pull request Aug 21, 2025
We are hitting a long tail of problems at work related to old nix-direnv
sneaking old nix versions in that would be fixed by
nix-community/nix-direnv#513, except that the
issue is that nix-direnv itself is too old.

It would at least be very helpful to print out *what* ancient nix
version is at fault.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nix-direnv should not bundle nix

3 participants