Skip to content

Set inNixShell#677

Merged
bbenne10 merged 1 commit into
nix-community:masterfrom
ncfavier:inNixShell
Feb 8, 2026
Merged

Set inNixShell#677
bbenne10 merged 1 commit into
nix-community:masterfrom
ncfavier:inNixShell

Conversation

@ncfavier
Copy link
Copy Markdown
Member

nix-shell passes a inNixShell = true argument when evaluating a file, so we should replicate this behaviour. There is another mechanism for signaling that we are in a nix shell, namely the IN_NIX_SHELL environment variable (which both nix-shell and nix-direnv also set), but this is by nature not a good solution since it leaks into e.g. further nix-build invocations in nix shells.

This only concerns non-flake setups, of course.

ping @roberth @sternenseemann

`nix-shell` passes a `inNixShell = true` argument when evaluating a
file, so we should replicate this behaviour. There is another mechanism
for signaling that we are in a nix shell, namely the `IN_NIX_SHELL`
environment variable (which both nix-shell and nix-direnv also set),
but this is by nature not a good solution since it leaks into e.g.
further `nix-build` invocations in nix shells.

This only concerns non-flake setups, of course.
@roberth
Copy link
Copy Markdown

roberth commented Jan 25, 2026

EDIT: nvm the "migitation" - it's actually pointless

It did require adjustment in some shell expressions, where Nix can't trivially detect that the "auto-arg" is not accepted.
You could limit this to shell.nix files to limit the impact of that drawback, but doing so introduces complexity and a bit of risk on its own.
I'd accept this PR as is, and only implement that partial mitigation if it proves necessary.

@ncfavier
Copy link
Copy Markdown
Member Author

Limiting this to shell.nix would pretty much defeat the purpose, which is to combine shell.nix into default.nix. Maybe that is a bad idea though, and in my use case I ended up writing a shell.nix.

I guess you're referring to NixOS/nix#5543? That was closed inconclusively; I'll try to replicate the problem there later.

@ncfavier
Copy link
Copy Markdown
Member Author

So the problem in NixOS/nix#5543 was that inNixShell was previously simply added to autoArgs, but the heuristic for inserting autoArgs into function arguments was too eager, so an expression like

{ ... }@args: ({ foo, bar }: qux) args

would be given inNixShell and fail. This was fixed in NixOS/nix#6664 by modifying the heuristic specifically for the internally-generated inNixShell to only insert it for functions that explicitly take an inNixShell argument (e.g. { inNixShell, ... }: … but not { ... }: …). Of course it's undecidable whether an expression will crash when given an inNixShell argument, so heuristics are the best we can do.

This PR adds --arg inNixShell true to use_nix calls that do not use -p. So, while this would indeed fail for expressions such as the above (because --arg arguments are not subject to the modified heuristic), we are unlikely to run into the breakage mentioned in NixOS/nix#5543 which was specifically about wrapped nixpkgs expressions (which get evaluated when using -p).

Come to think of it, since nix-direnv calls nix print-dev-env maybe the proper fix is to make nix print-dev-env replicate the inNixShell logic of nix-shell.

@bbenne10
Copy link
Copy Markdown
Contributor

I have to give this some thought and investigate the linked context before I make a call.

Based on what I am seeing so far though, I think I agree that print-dev-env should send inNixShell and I believe this is a reasonable compromise in the short term.

We are just coming up for air after a pretty hectic weekend of winter storms here - sorry for not getting back to you faster.

@bbenne10 bbenne10 added this pull request to the merge queue Feb 8, 2026
Merged via the queue into nix-community:master with commit 608f784 Feb 8, 2026
5 checks passed
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.

3 participants