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

doc: Clarify that nix-shell still uses shell from host environment #8809

Merged
merged 6 commits into from
Nov 21, 2024

Conversation

CyberShadow
Copy link
Contributor

Motivation

This came as a surprise to me, and even other experienced users from the Matrix channel.

Context

This is a documentation improvement which adds clarity to the semantics of the nix-shell command.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@CyberShadow

This comment was marked as outdated.

@CyberShadow
Copy link
Contributor Author

will clarify and reopen

I think the source of my confusion is just that shell.nix is not the right tool for when you want to fully encapsulate your dependencies in a single file. Unless I'm missing something, nix-shell will still use the search path from its environment to pick the shell to run in the environment created by a shell.nix file, and now I'm not sure if this is something that can/should be expressed in the documentation.

@CyberShadow
Copy link
Contributor Author

Demonstration:

$ cat > shell.nix <<EOF
#!/usr/bin/env -S -u NIX_PATH nix-shell --pure
let
  pkgs = import (fetchTarball "https://github.com/NixOS/nixpkgs/archive/854fdc68881791812eddd33b2fed94b954979a8e.tar.gz") {};
in
pkgs.mkShell {
  buildInputs = with pkgs; [
    bash
  ];
}
EOF
$ nix-shell
error:
       … while calling the 'import' builtin

         at «string»:1:2:

            1| (import <nixpkgs> {}).bashInteractive
             |  ^

       … while calling the 'findFile' builtin

         at «string»:1:9:

            1| (import <nixpkgs> {}).bashInteractive
             |         ^

       error: file 'nixpkgs' was not found in the Nix search path (add it using $NIX_PATH or -I)

       at «none»:0: (source not available)
will use bash from your environment

[nix-shell:~/tmp]$ readlink /proc/$$/exe
/usr/bin/bash

@CyberShadow
Copy link
Contributor Author

CyberShadow commented Jul 16, 2024

Revisiting this, I still think that at least a small clarification would help.

My confusion was that I expected the shell (Bash) executable to be obtained from the same pkgs as the one used to call mkShell, but the logic is actually different (explained lower in the "Environment variables" section), which can lead nix-shell to use one nixpkgs version for the shell executable, and another for mkShell.

Combined with --pure specifically, I was further confused at where this impurity was coming from.

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Demonstration:

Aghr, nix-shell is the worst, and just about everything involved in it, inlcuding pkgs.mkShell. The number of issues this keeps spawning is staggering. I suggest we document all of that in excruciating detail, and re-think the entire complex from scratch: NixOS/nix.dev#135 (comment) #4702 (comment)

@CyberShadow could you please fix up the details and also add your example? Then I'll merge quickly. I suggest putting the full explanation under NIX_BUILD_SHELL and linking there from --pure. Please check other files for how to make anchors, and test the resulting manual and man and --help pages that it renders correctly.

doc/manual/src/command-ref/nix-shell.md Outdated Show resolved Hide resolved
@CyberShadow
Copy link
Contributor Author

I tried to add the example but it doesn't look very good in man:


Environment variables
       •  NIX_BUILD_SHELL

          Shell used to start the interactive environment. Defaults to the bash from
          bashInteractive found in <nixpkgs>, falling back to the bash found in PATH
          if not found.

          Note  that  the  default  shell  obtained  using  the method above may not
          necessarily be the same as any shells  requested  in  path.  For  example,
          consider:

       #!/usr/bin/env -S nix-shell --pure
       let
       pkgs = import (fetchTarball "https://github.com/NixOS/nixpkgs/archive/854fdc6888
1791812eddd33b2fed94b954979a8e.tar.gz") {};
       in
       pkgs.mkShell {
       buildInputs = pkgs.bashInteractive;
       }

              Despite  --pure,  the  above  will  not result in a fully reproducible
              shell environment.

Common Environment Variables

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

So much for "merge quickly"... Please excuse the long delay, other things took priority. I need a bit of clarification on what exactly you want to say, otherwise this should be good to go.

I'd say don't worry about the rendering. lowdown is janky and all of this should work differently anyway; this would another piece of motivation to change it.

doc/manual/src/command-ref/nix-shell.md Outdated Show resolved Hide resolved
doc/manual/src/command-ref/nix-shell.md Outdated Show resolved Hide resolved
@fricklerhandwerk fricklerhandwerk enabled auto-merge (squash) November 21, 2024 19:35
@fricklerhandwerk
Copy link
Contributor

Thanks @CyberShadow for your patience. Sometimes things appear in my queue at an unfortunate time and then slip through the cracks.

@fricklerhandwerk fricklerhandwerk merged commit ba07446 into NixOS:master Nov 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants