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

Jump to bottom

Fix store imports from NixOS modules #77416

Merged
merged 2 commits into from Jan 10, 2020
Merged

Fix store imports from NixOS modules #77416

merged 2 commits into from Jan 10, 2020

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Jan 10, 2020 •
edited

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
lib/modules: Fix store imports
e0ea5f4 
This fixes imports from the store not being possible, which was caused by
NixOS#76857

E.g. such a case:

  imports = [ "${home-manager}/nixos" ];
@infinisil
lib/tests: Add test case for imports from derivations
2955e6b
@infinisil
Copy link
Member Author

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
@infinisil infinisil deleted the fix-store-imports branch January 10, 2020 03:45
@LnL7
Copy link
Member

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

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
@infinisil
lib/tests: Fix module tests
092107c 
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
Merged
2 tasks
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jan 19, 2020
@infinisil @dtzWill
lib/tests: Fix module tests
02acb6f 
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)
@infinisil infinisil added the 6.topic: module system About NixOS module system internals label Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra Awaiting requested review from edolstra

@nbp nbp Awaiting requested review from nbp

Assignees
No one assigned
Labels
6.topic: module system About NixOS module system internals 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@infinisil @LnL7