-
Notifications
You must be signed in to change notification settings - Fork 63
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
don't rely on NIX_PATH
#349
Conversation
nixpkgs_review/builddir.py
Outdated
@@ -63,7 +63,7 @@ def __init__(self, name: str) -> None: | |||
os.environ["NIX_PATH"] = self.nixpkgs_path() | |||
|
|||
def nixpkgs_path(self) -> str: | |||
return f"nixpkgs={self.worktree_dir}:nixpkgs-overlays={self.overlay.path}" | |||
return f"nixpkgs={self.worktree_dir} nixpkgs-overlays={self.overlay.path}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is space an allowed separator in the environment variable as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reading the source code, I don't think it is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote the patch to make nixpkgs-review not rely on NIX_PATH, hopefully I didn't accidently break something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually find it quite handy to have a NIX_PATH set. Can we not just keep it, but not rely on it for our own tooling?
Nix does not respect `NIX_PATH` when the `nix-path` setting in nix.conf is set, which breaks the `nix-store --verify-path` due to `<nixpkgs>` diverging from the temporary nixpkgs created by nixpkgs-review.
Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at e92c011 |
Nix does not respect
NIX_PATH
when thenix-path
setting in nix.confis set, which breaks the
nix-store --verify-path
due to<nixpkgs>
diverging from the temporary nixpkgs created by nixpkgs-review.
fixes #343