Skip to content

stdenv/setup.sh: Allow NIX_ATTRS_{JSON,SH}_FILE to be set correctly by Nix#214937

Merged
Artturin merged 1 commit intoNixOS:stagingfrom
hercules-ci:support-NIX_ATTRS-envs
Feb 6, 2023
Merged

stdenv/setup.sh: Allow NIX_ATTRS_{JSON,SH}_FILE to be set correctly by Nix#214937
Artturin merged 1 commit intoNixOS:stagingfrom
hercules-ci:support-NIX_ATTRS-envs

Conversation

@roberth
Copy link
Member

@roberth roberth commented Feb 6, 2023

Description of changes

Support a future where this environment variable is set correctly.

Tested with

nix-build -A tests.stdenv.structuredAttrsByDefault.test-structured-env-attrset
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@roberth roberth requested a review from Artturin February 6, 2023 13:18
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Feb 6, 2023
Artturin
Artturin approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@Artturin Artturin left a comment

Choose a reason for hiding this comment

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

Thanks

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Feb 6, 2023
@Artturin Artturin merged commit aa7d2fe into NixOS:staging Feb 6, 2023
arcnmx
arcnmx reviewed Apr 26, 2023
View reviewed changes
pkgs/stdenv/generic/setup.sh
# $NIX_ATTRS_JSON_FILE pointed to the wrong location in sandbox
# https://github.com/NixOS/nix/issues/6736; please keep around until the
# fix reaches *every patch version* that's >= lib/minver.nix
if ! [[ -e "$NIX_ATTRS_JSON_FILE" ]]; then
Copy link
Member

@arcnmx arcnmx Apr 26, 2023

Choose a reason for hiding this comment

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

I've been seeing the following error, presumably due to this change:

structuredAttrs is enabled
/nix/store/5s1yg5l36wzgy1dj0vv1ibarc4g7vrdr-stdenv-linux/setup: line 44: NIX_ATTRS_JSON_FILE: unbound variable
failed with exit code 1

Should this not be ${NIX_ATTRS_JSON_FILE-} and ditto for NIX_ATTRS_SH_FILE?

Copy link
Member

@sternenseemann sternenseemann May 29, 2023

Choose a reason for hiding this comment

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

#234883 should fix that.

@sternenseemann sternenseemann mentioned this pull request May 29, 2023
Merged
12 tasks
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request May 29, 2023
@sternenseemann
stdenv/setup.sh: deal with Nix < 2.4 structured attrs
1a29857 
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 that referenced this pull request May 31, 2023
@sternenseemann @github-actions
stdenv/setup.sh: deal with Nix < 2.4 structured attrs
ea67e69 
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)
@ConnorBaker ConnorBaker mentioned this pull request Sep 13, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sternenseemann sternenseemann sternenseemann left review comments

@Artturin Artturin Artturin approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

+1 more reviewer

@arcnmx arcnmx arcnmx left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@roberth @sternenseemann @arcnmx @Artturin