-
Notifications
You must be signed in to change notification settings - Fork 87
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
env
has a weird format, should be an attrset
#217
Comments
The attrset approach also means we'll get module-level enforcement that each env var is only specified once. With this, it would be nice to support merging I also wonder if the submodule definition, where it has 4 options of which exactly one must be set, could be instead replaced by a |
I agree that it's cumbersome and would be happy to find another way to arrange this. The main reason it's using a list is that Consider:
Because Nix orders the keys alphabetically, HELLO would appear first in an iteration. One thing I considered for a while was to split the set/unset from the eval/prefix ones. Set/unset would be an attrset that gets applied first, and then the eval/prefix is still in a list. I think that would work too but wasn't overwhelmingly better. |
I didn't consider the idea that an [[env]]
name = "FOO"
prefix = "bin"
[[env]]
name = "FOO"
prefix = "bin2" I suppose the implicit realpath-ing of the value means you can't just say I suggested using That said, I do like the idea that it would apply set/unset first, then prefix, and then eval. I suppose it could apply this ordering only when processing the attrset version, otherwise you might be surprised at the behavior of {
env = [
{ name = "OLD_BAR"; eval = ''"$BAR"''; }
{ name = "BAR"; value = "some value"; }
];
} though explaining the behavior would be simpler if it just applied always. Folks can use scripts if they really need to do fancy things like that (or use But of course applying it always is a backwards compatibility hazard, so we could also just say ordering isn't guaranteed when using the attrset version, use the list version if order matters or write a script to set the variables. But if you're okay with changing ordering then I would prefer to do set/unset, then prefix, then eval. This way set/unset won't ever overwrite prefix/eval, and then prefix next because it only depends on itself, and then eval because it can depend on anything. |
If you're up for sending a PR, I suggest the following: Change the [env]
FOO = "bar" Introduce [[envs]]
name = "FOO"
value = "bar" Have an adapter so that if a list is passed to Something like that? |
That sounds like a good idea. And perhaps it can be defined/documented such that What I've done locally is defined a similar {
env' = {
FOO = "bar";
BAR.eval = "$BAZ";
QUX = null; # this unsets it, as does { value = null; }
};
} The unset-on-null-value part was a bit complicated, I did that by removing the default on Unfortunately I can't actually submit a PR without running afoul of my employer's OSS contribution policies. |
I'm sorry to ask but are you using this project at work? |
@zimbatm My employer has strict policies about employees contributing to OSS even for personal usage. |
I don't know what to think. Reciprocity is important for the OSS ecosystem's health. We spend a lot of time building things for free, helping people out. It's human to do that. If your company has very strict IP policies, another way to contribute back is to set up a support contract with us. This then helps us fund our lives and in consequence, allows us to do more projects like this ( sales@numtide.com ). Thanks for contributing your ideas and code samples, I think for now this issue will stay like that until some new event will revive it. |
Not exactly trying to revive this issue, but would like to share a code snippet i'm using to set the env vars: env = lib.attrsets.attrsToList {
FOO = "one";
BAR = "two";
}; From the NixOS 23.11 Release Notes
It won't handle those edge cases, but it's good enough most of the time. |
Another idea [
{ x = null; }
{ a = "b"; c = "d"; }
{ e.eval = "$b"; }
] |
Is your feature request related to a problem? Please describe.
The
env
options has a bit of an awkward format. I'm doing my configuration in Nix so I have to write it likeThe TOML isn't much better, having to specify name/value repeatedly.
Describe the solution you'd like
I'd like to be able to write
I believe the TOML would also benefit from this shorthand, which I expect would look something like the following (although I'm a bit rusty at TOML):
You should be able to define the type using
coercedTo
so the old version is coerced to this new version, thus allowing for merging modules that use both. The values themselves could also be done with coercedTo in order to convert the bare value form into{ value = …; }
(or just update the code to handle either).Describe alternatives you've considered
A library function could be written that does the conversion, which would only be usable in Nix code, and is less discoverable and more awkward.
Allowing bare values (e.g.
DEVSHELL_NO_MOTD = 1
) could also be skipped in favor of writing this likealthough it's nice to make the most common expected form easier to use.
The text was updated successfully, but these errors were encountered: