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
nixos/systemd: Don't use apply for $PATH #91092
Conversation
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.
Can you provide an example, or (even better a test)?
{ config, lib, ... }: with lib; {
config = {
warnings = mapAttrsToList (n: v: "${n}.service needs ${if isList v.path then concatStringsSep ", " v.path else v.path}") config.systemd.services;
};
} This will either print the colon-separated $PATH or a list of packages (depending on whether you applied this PR). |
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.
Big 👍 on this. I think this needs a backward incompatibility release note though.
91d4bc8
to
8965e73
Compare
@infinisil like this? |
There are conflicts in the release notes @dasJ |
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.
Looks good after conflicts resolved
When not using apply, other modules can use $PATH as a list instead of getting a colon-separated list to each /bin directory.
8965e73
to
8cf4ec8
Compare
Can anyone with write permissions do the mergy thing? |
Following changes in #91092 the `path` attribute is now a list instead of being a string. This resulted resulted in the following evaluation error: "cannot coerce a list to a string, at [...]/nixos/modules/services/networking/openvpn.nix:16:18" so we now need to convert it to the right type ourselves. Closes #97360.
Following changes in #91092 the `path` attribute is now a list instead of being a string. This resulted resulted in the following evaluation error: "cannot coerce a list to a string, at [...]/nixos/modules/services/networking/openvpn.nix:16:18" so we now need to convert it to the right type ourselves. Closes #97360. (cherry picked from commit cb14135)
When not using apply, other modules can use $PATH as a list instead of
getting a colon-separated list to each /bin directory.
Motivation for this change
Follow-up of #75510.
@ajs124 and I have an AppArmor module which would greatly benefit from being able to parse the path as a list.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)