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

only set default values for nix-path if nothing else is set #11429

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fricklerhandwerk
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk commented Sep 4, 2024

TODO: this doesn't work yet.
we currently have no way of checking if a config value was set or not.

Motivation

this is the expected behavior for default values:
new values should override them, not prepend.

Context

the change fixes a logic bug introduced when fixing the previously confused
override mechanism in e062021.
thanks @roberth for pointing it out:

$ NIX_PATH= nix run nix -- eval --impure --expr 'builtins.nixPath' --show-trace --option nix-path ""
[ { path = "/home/user/.nix-defexpr/channels"; prefix = ""; } { path = "/nix/var/nix/profiles/per-user/root/channels"; prefix = ""; } ]

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Sep 4, 2024
@fricklerhandwerk
Copy link
Contributor Author

@Ericson2314 This is weird, the tests should fail. I noticed this locally too, but assumed that I'm doing it wrong, but apparently something is wrong with the tests?

TODO: this doesn't work yet.
we currently have no way of checking if a config value was set or not.

this is the expected behavior for default values:
new values should override them, not prepend.

the change fixes a logic bug introduced when fixing the previously confused
override mechanism in e062021.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-09-09-nix-team-meeting-minutes-176-175/51852/1

@roberth
Copy link
Member

roberth commented Sep 9, 2024

In today's meeting we've determined that the entries are only added when actually present in e.g. $NIX_STATE_DIR.
The test case needs to insert some content there. @fricklerhandwerk is on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants