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

stdenv: fix unbound NIX_LOG_FD in nix develop #330830

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

tie
Copy link
Member

@tie tie commented Jul 29, 2024

Description of changes

When running nix develop for a package, Nix records the stdenv environment with NIX_LOG_FD set. That is, when the actual development shell runs, it uses the functions that attempt to log to NIX_LOG_FD, but this variable is not actually set.

As a workaround, check whether NIX_LOG_FD is set at runtime.

Example (before this change):

$ nix develop --file . hello
$ echo "${NIX_LOG_FD-unset}"
unset
$ runPhase unpackPhase
bash: "$NIX_LOG_FD": Bad file descriptor
Running phase: unpackPhase
unpacking source archive /nix/store/pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz
bash: "$NIX_LOG_FD": Bad file descriptor
source root is hello-2.12.1
bash: "$NIX_LOG_FD": Bad file descriptor
setting SOURCE_DATE_EPOCH to timestamp 1653865426 of file hello-2.12.1/ChangeLog

After this change:

$ nix develop --file . hello
$ runPhase unpackPhase
Running phase: unpackPhase
unpacking source archive /nix/store/pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz
source root is hello-2.12.1
setting SOURCE_DATE_EPOCH to timestamp 1653865426 of file hello-2.12.1/ChangeLog

Notice that bash: "$NIX_LOG_FD": Bad file descriptor lines are gone.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Jul 29, 2024
@tie tie requested review from Qyriad and philiptaron July 29, 2024 09:21
When running `nix develop` for a package, Nix records the stdenv
environment with NIX_LOG_FD set. That is, when the actual development
shell runs, it uses the functions that attempt to log to NIX_LOG_FD, but
this variable is not actually set.

As a workaround, check whether NIX_LOG_FD is set at runtime.

Example (before this change):
```console
$ nix develop --file . bash
$ echo "${NIX_LOG_FD-unset}"
unset
$ runPhase unpackPhase
bash: "$NIX_LOG_FD": Bad file descriptor
Running phase: unpackPhase
unpacking source archive /nix/store/v28dv6l0qk3j382kp40bksa1v6h7dx9p-bash-5.2.tar.gz
bash: "$NIX_LOG_FD": Bad file descriptor
source root is bash-5.2
bash: "$NIX_LOG_FD": Bad file descriptor
setting SOURCE_DATE_EPOCH to timestamp 1663942708 of file bash-5.2/y.tab.h
```

After this change:
```console
$ nix develop --file . bash
$ runPhase unpackPhase
Running phase: unpackPhase
unpacking source archive /nix/store/v28dv6l0qk3j382kp40bksa1v6h7dx9p-bash-5.2.tar.gz
source root is bash-5.2
setting SOURCE_DATE_EPOCH to timestamp 1663942708 of file bash-5.2/y.tab.h
```
@tie tie force-pushed the nix-develop-unbound-variable branch from a08d02f to bd872b4 Compare July 29, 2024 10:09
@tie tie marked this pull request as ready for review July 29, 2024 10:10
@tie tie requested a review from Ericson2314 as a code owner July 29, 2024 10:10
@drupol
Copy link
Contributor

drupol commented Jul 29, 2024

Same issue here, but the origin seems to be different.

❯ rm -rf .venv && direnv reload
direnv: loading ~/Code/work/python-project/.envrc
direnv: using flake
direnv: nix-direnv: Renewed cache
Executing venvHook
Creating new venv environment in path: './.venv'
/home/pol/.config/direnv/lib/hm-nix-direnv.sh:1792: "$NIX_LOG_FD": Bad file descriptor
Installing dependencies from lock file

Package operations: 71 installs, 0 updates, 0 removals

  - Installing typing-extensions (4.12.2): Pending...
  - Installing typing-extensions (4.12.2)
  - Installing annotated-types (0.7.0)
  ...

@tie
Copy link
Member Author

tie commented Jul 29, 2024

direnv: using flake

@drupol, IIRC nix-direnv (ab)uses nix develop, so probably the same underlying issue (but with more layers of abstractions on top)…

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Yep.

@drupol drupol requested review from vcunat and jtojnar July 30, 2024 12:16
@drupol drupol merged commit 5efe6c1 into NixOS:staging Jul 30, 2024
47 of 49 checks passed
@trofi
Copy link
Contributor

trofi commented Jul 30, 2024

Is it a side-effect of NixOS/nix#8987 by chance?

@tie tie deleted the nix-develop-unbound-variable branch July 31, 2024 07:14
@tie
Copy link
Member Author

tie commented Jul 31, 2024

Is it a side-effect of NixOS/nix#8987 by chance?

@trofi, yes, these functions were recorded in a separate derivation where NIX_LOG_FD was set. I’m OK with this Nix behavior, but we should probably document that stdenv’s setup.sh and hooks shouldn’t have significant side effects when sourced (and use phases instead e.g. when creating files).

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.

4 participants