I’ve been working on a few nixpkgs PRs to address some ShellCheck lints, and I’ve been wondering what to do with scripts inlined into Nix code. I’m thinking I could run them through shellcheck (via stdin or builtins.toFile) during the test phase - would that be a good approach to allow people to mix shell code into Nix?
(Personally I’m not a fan of mixing languages because of all the syntax clashes, extra faff escaping things, and not having any syntax highlighting, but if this is a common pattern we’re going to have to live with for the foreseeable future at least the code should be linted.)
Most of the inclined code, at least I feel, will be in one of the phases that derivations like to override. So maybe checking all of pkgs/ for .*Phase\s*=\s*'' (regex) will return a fair share of all inlined shell code.
After that it’s only a matter of extracting it (should also be pretty doable with regex, although beware that '' is also used to escape ${}!) and running shell check over the result.
Oh, that gives me another idea. You can use the repl to easily print (or even check it right in there) the shell code of all (and here’s the big upside!) only modified phases. <package>.drvAttrs.<tab>.
You can easily iterate over all since it’s just an attribute set.
I’m curious if ~3 years later anyone has an example somewhere of doing this? I tend to keep my scripts as separate files so I can use shfmt and shellcheck via pre-commit hooks, but since it’s not uncommon to have shell scripts that need/want nix expressions variables (like options) set, they need to be inlined.
Worth noting while looking into the pkgs.substituteAll function you used, I ran across this which recommends not using that function, but instead using pkgs.substitute. So if you end up actually breaking out a bunch more scripts, maybe worth considering not using it.
I’m sure there are wild examples of this as well, but if nothing else writeShellApplication was added to nixpkgs in the interim:
FWIW, I’ve come to see lint/format checks in the check/installCheck phases as a minor antipattern. Unlike a normal test suite that’s developing with your codebase, version bumps of these tools in nixpkgs can cause scripts to stop ~building even when their source is unchanged.
This can be easy enough to cope with if they’re only in your local config, but it can be a chore if they aren’t (i.e., they’re in nixpkgs, third-party code, or even your own code that you’ve extracted into a separate project/flake for others to depend on).
I think it’s ideal if you can run these checks as part of your development process and not otherwise. If the inline scripts are their own package/derivation (as with writeScriptBin) I’d probably just check these from another derivation that depends on them. If I had to put one in a package’s check/test phases, I’d probably guard it behind an argument that defaults to false and have my own process for building the package with it set to true.