shellcheck
shellcheck copied to clipboard
Add support for #!/usr/bin/env nix-shell
For new checks and feature suggestions
- [x] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
- [x] I searched through https://github.com/koalaman/shellcheck/issues and found https://github.com/koalaman/shellcheck/issues/977#issuecomment-373405520, which shows that there are at least two persons who want this.
Here's a snippet or screenshot that shows the problem:
#!/usr/bin/env nix-shell
true
Here's what shellcheck currently says:
^-- SC1008: This shebang was unrecognized. Note that ShellCheck only handles sh/bash/dash/ksh.
Here's what I wanted or expected to see:
Empty output, because it's a standard way of using nix-shell.
I don't know much about nix-shell, from [0], it seems like it downloads nix deps and opens a bash shell, so will it be okay to treat the file as bash?
[0] https://nixos.org/nix/manual/#idm140737318501472
What the file is supposed to contain depends on the -i argument.
This is an example of correct usage:
#!/usr/bin/env nix-shell
#!nix-shell -i bash -p qemu jq ec2_api_tools awscli
The -i can also be sh or zsh, etc.
-i <interpreter> so it can be bash/ruby/python etc. shellcheck could disable itself if != sh/bash.
Can we get some feedback from whoever maintains this that this is a feature that would be accepted?
I am very hesitant to add first-class support for new shells, because it's easy to add but hard to maintain and remove.
I'd prefer a simple third party wrapper script nix-shellcheck that invokes shellcheck -s bash on such files, similar to how hadolint and bats testing works.
@koalaman Thanks for your reply.
I don't know what you mean by first-class support for new shells in this particular case.
The only thing that needs to happen is a conditional to check for the first line being equal to #!/usr/bin/env nix-shell
and in that case, it should look for the -i option. If the -i option is equal to sh it should check the file ignoring the first two lines. If the -i option is equal to zsh (or bash), it should do something similar, etc.
I don't understand your concerns.
@coretemp Would the shell directive be useful in this case?
The shell directive does solve this issue for me! Thanks @qubidt. I just wanted to share an example of how to use it here for the next person stumbling upon this issue:
$ cat test-shell.sh
#!/usr/bin/env nix-shell
#!nix-shell -i bash -p hello
#shellcheck shell=bash
hello
$ ./test-shell.sh
Hello, world!
$ shellcheck test-shell.sh
$ echo $?
0
I would really love to see shellcheck gain native support for this, so I don't end up with errors in VS Code every time I edit a shell script that uses nix-shell. In my own scripts I can add shell directives, but I'm often opening scripts I don't control.
Agree with @lilyball, if a PR implementing support for this (identifying the -i parameter in the second line) be accepted?
@expipiplus1 Sure, it makes sense for autoidentifying purpose
Great, it looks like it would require a slight change to the parser to not ignore additional "shebang"s at the top of the file, at the moment the pretend #!nix-shell -i foo line is ignored as a comment.
This is not a nix-shell specific bug. env is part of the GNU Coreutils package that is used in most *Nix based systems. Any interpreter (indeed any executable) can be passed the current environment using the GNU 'env' command. It is very common to write #!/usr/bin/env sh or #!/usr/bin/env bash, etc. As the shebang line. It is also used for non shell interpreters, like perl, python, and many others.
Effectively it does the same as something like:
envvar1="a" envvar2="b" myprog
but for every current environment variable.
Uses:
- Ensure the sub-process adopts the same environment, not just 'exported' parameters
- Alter the environment for the new interpreter
- Pass switches to the interpreter (using -S otherwise exec() would try and run an interpreter with a filename, e.g.
bash --posix)
Ref:
@the-moo to expand on your point, env isn't just part of the gnu coreutils.
env is part of POSIX.
Thus it is safe to assume it is available on all plattforms of interest.
Shellcheck already supports /usr/bin/env in general, so that's a red herring. This issue is indeed specific to nix-shell.
Shellcheck already supports
/usr/bin/envin general, so that's a red herring. This issue is indeed specific tonix-shell.
Hmm, well when running in vscode it seems to disagree, at least for me....
Could it be that the vscode plugin that uses this project is somehow disabling this feature? (Shellcheck/VSCode wrapper v0.34.0) Note the debug console states that it is using the latest 0.9.0 shellcheck binary.
That's because you're using /usr/bin/env wrong.
The correct line here would be #!/usr/bin/env bash
That's because you're using
/usr/bin/envwrong. The correct line here would be#!/usr/bin/env bash
Interesting, you are right, the error goes away if I remove the explicit path to the interpreter.
(BTW as I am not allowed to post my actual scripts here I made a cut and paste error in the empty file that I snipped the error from, in doing that I omitted the leading /)
As we all know, despite age, there are still things to learn....
And so I've probably been using this wrong for as long as I can remember! (~20+ years ?? though my memory is not that good thse days). I am fairly sure I always wrote /path/interpreter for bash (or sh or dash) and now I need to know why I am 'doing it wrong' by being explicit with the path to the exe?
It has always been my take that without an explicit path I am at the mercy of whatever path the requested interpreter is found at (note, I am on a huge system with ~10k+ users and make use of gnu environemt modules), so without being explicit, surely it could be dangerous? e.g. a user could accidently (or deliberately) insert line into their own path and use an interpreter that behaved differently to the intended one? I just did a quick test to examine this and it seems to behave as I would expect. Gist
Well wrong might be a strong word here, considering it still works.
Let's check man 1p env which goes into this in some detail.
I'll quote parts I believe are relevant, (...) signifying omission.
NAME
env — set the environment for command invocation
SYNOPSIS
env [‐i] [name=value]... [utility [argument...]]
(...)
OPERANDS
The following operands shall be supported:
(...)
utility The name of the utility to be invoked. If the utility operand names any of the special built‐in utilities in Section
2.14, Special Built‐In Utilities, the results are undefined.
(...)
ENVIRONMENT VARIABLES
The following environment variables shall affect the execution of env:
(...)
PATH Determine the location of the utility, as described in the Base Definitions volume of POSIX.1‐2017, Chapter 8, Environ‐
ment Variables. If PATH is specified as a name=value operand to env, the value given shall be used in the search for
utility.
You are correct that by ommiting the path you're at the mercy of PATH.
You can make a semantic argument that if the user is able to alter their PATH, they can alter behaviour.
However this argument leads us astray, as we already have an alternative if we do not want the operating system to search the PATH flexibly.
In such cases we should specify the path directly to the shebang, as env provides no additional value in this case.
#!/bin/bash > #!/usr/bin/env /bin/bash (goal: consistent path)
#!/usr/bin/env bash > #!/usr/bin/env /bin/bash (goal: dynamic lookup)
Mixing env and a static path combines the subtle disadvantages of both.
Does that make sense?
The fact that #!/usr/bin/env /bin/bash does not work should be filed as a separate bug. It's a rather useless construct, as the only reason to invoke /usr/bin/env with an absolute path is if you want to modify the environment (e.g. #!/usr/bin/env FOO=bar /bin/bash) or wish to make use of flags (e.g. on many systems, the shebang does not split options and so you may need #!/usr/bin/env -S /bin/bash --login --posix to instruct env to split the arguments), neither of which is being used in your example. But shellcheck should probably still recognize it, so you should file an issue on it, but it's unrelated to the nix-shell issue.