nixos/systemd: allow running shellcheck on generated systemd scripts#311394
nixos/systemd: allow running shellcheck on generated systemd scripts#311394flokli merged 1 commit intoNixOS:masterfrom
Conversation
a9a7eb6 to
e2a08b4
Compare
e2a08b4 to
b3253bd
Compare
faf5a04 to
cedb0c7
Compare
SuperSandro2000
left a comment
There was a problem hiding this comment.
Very good idea. I think the only thing that really needs some discussion, is what we are going to do about the shellcheck mkdir -p error. Maybe we ignore it by default for now?
Also please remind me in a week or so, that I want to test this on my deployments.
ElvishJerricco
left a comment
There was a problem hiding this comment.
Seems like this should be something that can be enabled per-unit, with the global setting acting as the default.
@ElvishJerricco that's already how it's done. There is a global switch (defaults to So it's possible to enable or disable this on a per-unit basis, independent of the global option. |
|
Ah, I misunderstood what I was reading. Great! |
ee85fe4 to
fbba7c8
Compare
fbba7c8 to
b9784ca
Compare
b9784ca to
350ec3e
Compare
|
I removed the fixes for different services. I will make separate PRs for these. You won't be able to build a NixOS system with shellcheck for services enabled until we also merge fixes for some of those. |
Mindavi
left a comment
There was a problem hiding this comment.
I think splitting this up was the right call and on surface this looks good to me and I ACK the idea too.
34c6e05 to
df3b9bb
Compare
Changes to services have been split out into separate PRs, all but one of which are already merged.
df3b9bb to
86db826
Compare
86db826 to
2b224f0
Compare
phaer
left a comment
There was a problem hiding this comment.
Very nice feature! Code looks good to me and feature works as intended. PR quite easy to review IMO.
Description of changes
With this PR we can optionally run shellcheck on generated systemd scripts.
Our nixos closure already depends on GHC (and shellcheck), so this does not affect the runtime closure.
I fixed the scripts that I use on my machines, but there is definitely a lot more stuff to fix before this could become the default.I will submit these as separate PRs.
For now, this sits behind an enable-option. It would be great if some people would enable this and submit fixes for existing scripts.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.