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

structuredAttrs $NIX_ATTRS_JSON_FILE points to the wrong location in sandbox #6736

Closed
Artturin opened this issue Jun 29, 2022 · 5 comments
Closed
Labels

Comments

@Artturin
Copy link
Member

Artturin commented Jun 29, 2022

Describe the bug

it points to /comp-temp/nix-build-bison-3.8.2.drv-2/.attrs.json

++++ realpath /build
/build

++++ ls -la /build
total 13
drwx------  3  1000 100  120 Jun 29 00:21 .
drwxr-x---  9 65534 100    9 Jun 29 00:21 ..
-rw-r--r--  1  1000 100 1510 Jun 29 00:21 .attrs.json
-rw-r--r--  1  1000 100 1662 Jun 29 00:21 .attrs.sh
drwxr-xr-x 14  1000 100  760 Sep 25  2021 bison-3.8.2
-rw-r--r--  1  1000 100 2796 Jun 29 00:21 env-vars

++++ cat /build/.attrs.json
<attrs here>

++++ echo /comp-temp/nix-build-bison-3.8.2.drv-4/.attrs.json
/comp-temp/nix-build-bison-3.8.2.drv-4/.attrs.json

++++ cat /comp-temp/nix-build-bison-3.8.2.drv-4/.attrs.json
cat: /comp-temp/nix-build-bison-3.8.2.drv-4/.attrs.json: No such file or directory

Steps To Reproduce

these commands were run in postPatch of bison on the NixOS/nixpkgs#175649 branch

    realpath $NIX_BUILD_TOP
    ls -la $NIX_BUILD_TOP
    cat $NIX_BUILD_TOP/.attrs.json

    echo $NIX_ATTRS_JSON_FILE
    cat $NIX_ATTRS_JSON_FILE

auto jsonSh = writeStructuredAttrsShell(json);
writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites));
chownToBuilder(tmpDir + "/.attrs.sh");
env["NIX_ATTRS_SH_FILE"] = tmpDir + "/.attrs.sh";
writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites));
chownToBuilder(tmpDir + "/.attrs.json");
env["NIX_ATTRS_JSON_FILE"] = tmpDir + "/.attrs.json";

@Artturin Artturin added the bug label Jun 29, 2022
@stale stale bot added the stale label Jan 8, 2023
@Artturin Artturin removed the stale label Jan 25, 2023
@Artturin
Copy link
Member Author

i worked around this with https://github.com/NixOS/nixpkgs/blob/d85d5f1723a0fd37577a09620be9527ed368f74b/pkgs/stdenv/generic/setup.sh#L30-L33

but nix develop still has a broken location because NIX_BUILD_TOP is wrong(/build) when setup.sh is sourced

easy way to test
nix develop ".#stdenv.__bootPackages.stdenv.__bootPackages.stdenv.__bootPackages.m4"

$ nix develop ".#stdenv.__bootPackages.stdenv.__bootPackages.stdenv.__bootPackages.m4"
gnum4> structuredAttrs is enabled

[artturin@artturinlinux:~/nixgits/my-nixpkgs]$ echo $NIX_BUILD_TOP
/tmp/nix-shell.0VA6y7

[artturin@artturinlinux:~/nixgits/my-nixpkgs]$ echo $NIX_ATTRS_JSON_FILE
/comp-temp/nix-build-gnum4-1.4.19-env.drv-0/.attrs.json

[artturin@artturinlinux:~/nixgits/my-nixpkgs]$ echo $NIX_ATTRS_SH_FILE
/comp-temp/nix-build-gnum4-1.4.19-env.drv-0/.attrs.sh

$ nix develop ".#stdenv.__bootPackages.stdenv.__bootPackages.stdenv.__bootPackages.m4"

[artturin@artturinlinux:~/nixgits/my-nixpkgs]$ echo $NIX_ATTRS_JSON_FILE
/build/.attrs.json

[artturin@artturinlinux:~/nixgits/my-nixpkgs]$ echo $NIX_ATTRS_SH_FILE
/build/.attrs.sh

@roberth roberth mentioned this issue Feb 7, 2023
14 tasks
@roberth
Copy link
Member

roberth commented Feb 7, 2023

The bug seems to be in LocalDerivationGoal::writeStructuredAttrs, making no distinction between the physical and sandbox paths. We already had a good test for this, but no good test driver.

@roberth
Copy link
Member

roberth commented Feb 7, 2023

I'll resume that PR later; meanwhile, help is of course appreciated.

@lheckemann
Copy link
Member

Fixed in #8053 -- sorry I missed that there was already work happening on this!

@Artturin
Copy link
Member Author

Fixed in #8053 -- sorry I missed that there was already work happening on this!

Thanks!

sternenseemann added a commit to sternenseemann/nixpkgs that referenced this issue May 29, 2023
Nix does not (as far it is documented) guarantee that NIX_ATTRS_*_FILE
is set, the only [documented] guarantee seems to be:

> […] made available to the builder via the file .attrs.json in the
> builder’s temporary directory.

This guarantee is of course affected by NixOS/nix#6736,
so it seems to be prudent to fall back to the Nix 2.3 style ATTRS_*_FILE
env vars before defaulting to the expected location in case neither is
available.

See also:

- NixOS#214937 (comment)
- NixOS@afef6588e250

[documented]: https://nixos.org/manual/nix/stable/language/advanced-attributes.html#adv-attr-structuredAttrs
github-actions bot pushed a commit to NixOS/nixpkgs that referenced this issue May 31, 2023
Nix does not (as far it is documented) guarantee that NIX_ATTRS_*_FILE
is set, the only [documented] guarantee seems to be:

> […] made available to the builder via the file .attrs.json in the
> builder’s temporary directory.

This guarantee is of course affected by NixOS/nix#6736,
so it seems to be prudent to fall back to the Nix 2.3 style ATTRS_*_FILE
env vars before defaulting to the expected location in case neither is
available.

See also:

- #214937 (comment)
- afef6588e250

[documented]: https://nixos.org/manual/nix/stable/language/advanced-attributes.html#adv-attr-structuredAttrs

(cherry picked from commit 1a29857)
Ma27 added a commit to Ma27/nixpkgs that referenced this issue Oct 4, 2023
Relying on `.attrs.sh` to exist in `$NIX_BUILD_TOP` is problematic
because that's not compatible with how `nix-shell(1)` behaves. It places
`.attrs.{json,sh}` into a temporary directory and makes them accessible via
`$NIX_ATTRS_{SH,JSON}_FILE` in the environment[1]. The sole reason that
`nix-shell(1)` still works with structured-attrs enabled derivations
is that the contents of `.attrs.sh` are sourced into the
shell before sourcing `$stdenv/setup` (if `$stdenv` exists) by `nix-shell`.

However, the assumption that two files called `.attrs.sh` and
`.attrs.json` exist in `$NIX_BUILD_TOP` is wrong in an interactive shell
session and thus an inconsistency between shell debug session and actual
builds which can lead to unexpected problems.

To be precise, we currently have the following problem: an expression
like

  with import ./. {};
  runCommand "foo" { __structuredAttrs = true; foo.bar = [ 1 2 3 ]; }
    ''
      echo "''${__structuredAttrs@Q}"
      touch $out
    ''

prints `1` in its build-log. However when building interactively in a
`nix-shell`, it doesn't.

Because of that, I'm considering to propose a full deprecation of
`$NIX_BUILD_TOP/.attrs.{json,sh}`. A first step is to only mention the
environment variables, but not the actual paths anymore in Nix's
manual[2]. The second step - this patch - is to fix nixpkgs' stdenv
accordingly.

Please note that we cannot check for `-e "$NIX_ATTRS_JSON_FILE"` because
certain outdated Nix minors (that are still in the range of supported
Nix versions in `nixpkgs`) have a bug where `NIX_ATTRS_JSON_FILE` points
to the wrong file while building[3].

Also, for compatibility with Nix 2.3 which doesn't provide these
environment variables at all we still need to check for the existence of
.attrs.json/.attrs.sh here. As soon as we bump nixpkgs' minver to 2.4,
this can be dropped.

Finally, dropped the check for ATTRS_SH_FILE because that was never
relevant. In nix#4770 the ATTRS_SH_FILE variable was introduced[4] and
in a review iteration prefixed with NIX_[5]. In other words, these
variables were never part of a release and you'd only have this problem
if you'd use a Nix from a git revision of my branch from back then. In
other words, that's dead code.

[1] NixOS/nix#4770 (comment)
[2] NixOS/nix#9032
[3] NixOS/nix#6736
[4] NixOS/nix@3944a12
[5] NixOS/nix@27ce722
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants