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

Fix store imports from NixOS modules #77416

Merged
merged 2 commits into from Jan 10, 2020
Merged

Conversation

@Infinisil
Copy link
Member

@Infinisil Infinisil commented Jan 10, 2020

Motivation for this change

Fixes the issue reported by @utdemir in #76857 (comment), caused by that very PR. Turns out we can't use that small speedup after all @roberth

I'll merge this as soon as ofborg is happy.

Things done
  • Fixed the issue
  • Added a testcase so this doesn't happen again
Infinisil added 2 commits Jan 10, 2020
This fixes imports from the store not being possible, which was caused by
#76857

E.g. such a case:

  imports = [ "${home-manager}/nixos" ];
@Infinisil Infinisil requested review from edolstra and nbp as code owners Jan 10, 2020
@Infinisil
Copy link
Member Author

@Infinisil Infinisil commented Jan 10, 2020

Well we could use this slight speedup method still, but we'd need an builtins.unsafeDiscardStringContext (which wouldn't be unsafe in this case) and that makes the code even harder to understand than it already was, for very little gain.

We can still do that in the future, but for now I'll merge this PR to fix the issue.

@Infinisil Infinisil merged commit 91da4b3 into NixOS:master Jan 10, 2020
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@Infinisil Infinisil deleted the Infinisil:fix-store-imports branch Jan 10, 2020
@LnL7
Copy link
Member

@LnL7 LnL7 commented Jan 10, 2020

This test doesn't work on hydra https://hydra.nixos.org/build/110012375.

$ nix-instantiate -E 'import ./default.nix { modules = [ ./import-from-store.nix ];}' -A 'config.enable' --eval-only
building '/build/test-tmp/store/gi6gwzqhpfwz517wh9pgwkz9byhbazws-derivation.drv'...
sh: can't create /build/test-tmp/store/pnanwkr93hh14rlhjl6bf8j3igbqh6ji-derivation: nonexistent directory
builder for '/build/test-tmp/store/gi6gwzqhpfwz517wh9pgwkz9byhbazws-derivation.drv' failed with exit code 1
@Infinisil
Copy link
Member Author

@Infinisil Infinisil commented Jan 10, 2020

Oh, hydra calls nix-build lib/tests/release.nix which runs all tests in a nix-build, I'll work on a fix

Infinisil added a commit to Infinisil/nixpkgs that referenced this pull request Jan 10, 2020
Fix the broken test in NixOS#77416

Apparently hydra uses `nix-build lib/tests/release.nix` to run all
tests, where IFD isn't allowed. Fortunately we can get around this with
builtins.toFile, which doesn't require IFD, but still can test the
properties we want.
@Infinisil Infinisil mentioned this pull request Jan 10, 2020
2 of 2 tasks complete
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Jan 19, 2020
Fix the broken test in NixOS#77416

Apparently hydra uses `nix-build lib/tests/release.nix` to run all
tests, where IFD isn't allowed. Fortunately we can get around this with
builtins.toFile, which doesn't require IFD, but still can test the
properties we want.

(cherry picked from commit 092107c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.