[shellcheck] Question about/support for source/source-path
MichaIng opened this issue ยท 9 comments
shellcheck allows the source
and source-path
directives (inside .shellcheck
) to allow sourcing a specific file or searching the defined path for files sourced by the shell script.
I tried using this with codefactor via paths relative to the repo root, but this didn't work. Is this supported at all, and if yes, how can a path be given relative to the project root? From what I know relative paths are always relative to the working directory from where shellcheck was executed. Most convenient would be if this was the repository root.
If this is currently not supported, take this issue as a feature request ๐.
@MichaIng thanks again for the feedback. Could you describe - or point to commits that shows how your tried using this feature (via paths relative to the repo root)?
Here is the .shellcheck
file history: https://github.com/MichaIng/DietPi/commits/dev/.shellcheckrc
The file that is sourced by most of our scripts on the final system is located at /boot/dietpi/func/dietpi-globals
but in dietpi/func/dietpi-globals
relative to the repository root: https://github.com/MichaIng/DietPi/blob/dev/dietpi/func/dietpi-globals
Setting only the source-path
directive in .shellcheck
leads this being ignored by codefactor and a related syntax error is shown in web UI. So I added some disable=
directives (we anyway needed) to the first line which made the .shellcheck
being recognised and used, but source-path
stays without effect.
Related shellcheck docs: https://github.com/koalaman/shellcheck/blob/master/shellcheck.1.md#directives
To be true, I did not try using the special SCRIPTDIR
path but this means that I'd need to add it to every script or multiple times in .shellcheck
for every possible relative-to-script path. It would be most convenient to only use a single entry that is valid for all scripts that source the same file.
EDIT: I should have made some research first. The problem is that those directives to not include -x
/--external-sources
as one would probably expect. So the shellcheck
command still requires this argument to support external sources: koalaman/shellcheck#1818
Probably codefactor can invoke shellcheck
with -x
/--external-sources
, like super-liner did: https://github.com/github/super-linter/pull/537
@MichaIng thanks for the thorough info. We've now enabled -x,\ --external-sources
option for ShellCheck process so in theory this feature should work.
In practice, there seems to be a limitation related to paths. Results after testing https://github.com/MichaIng/DietPi repo (dev
branch):
- for
.shellcheckrc
bothsource-path=dietpi/func
andsource-path=./dietpi/func
are working as expected - for source files, absolute paths (e.g.
/boot/dietpi/func/dietpi-globals
) are not working. Working values were one of:. dietpi-globals
,. dietpi/func/dietpi-globals
,. ./dietpi/func/dietpi-globals
,. SCRIPTDIR/func/dietpi-globals
. There could be more.
As a side note, file PREP_SYSTEM_FOR_DIETPI.sh
includes globals because of https://github.com/MichaIng/DietPi/blob/dev/PREP_SYSTEM_FOR_DIETPI.sh#L202
Wow, that was fast, many thanks! Will test it ASAP.
for source files, absolute paths (e.g. /boot/dietpi/func/dietpi-globals) are not working.
That makes sense as your test systems are not DietPi systems. Probably we should add some nice CI/code check bundles to make DietPi suitable for such tasks ๐. Makes sense for security reasons, or simply the case that .shellcheck
is used for codefactor + manual checks on own systems with two source-path
entries, absolute and relative, to limit even read access for shellcheck on the test/check system to the git/repo directory only.
As a side note, file PREP_SYSTEM_FOR_DIETPI.sh includes globals because of
Yes exactly, this script turns Debian systems into DietPi systems, so the dietpi-globals
script is downloaded and sourced from current working dir initially.
I added source-path=dietpi/func
to our .shellcheckrc
but so far it's not yet working, the related "variable appears unused" issues still appear on CodeFactor for all scripts.
I just verified that it's working on a local system with our current .shellcheckrc
: https://github.com/MichaIng/DietPi/blob/dev/.shellcheckrc
curl -sSfLO https://github.com/MichaIng/DietPi/archive/dev.tar.gz
tar xf dev.tar.gz
rm dev.tar.gz
cd DietPi-dev
shellcheck -x PREP_SYSTEM_FOR_DIETPI.sh
Thanks, there was still an issue on our side, you should now see updated results for PREP_SYSTEM_FOR_DIETPI.sh
file.
Okay for scripts inside the repo root it works now, for the others not yet. Does the checker navigate through the directories?
Locally I can do:
shellcheck -x dietpi/dietpi-autostart
shellcheck -x dietpi/func/dietpi-optimal_mtu
shellcheck -x rootfs/var/lib/dietpi/services/dietpi-firstboot.bash
The path dietpi/func
, relative to where the shellcheck
command was called, stays valid and dietpi-globals
is hence sourced as expected. So for .shellcheckrc
to work nicely, the checker must not navigate through the repo but stay in repo root instead. Would be great, as long as this does not imply any negative effects on all the other test tools, otherwise it is no problem for us to add a few more relativ paths to source-path
to cover all scripts ๐.
@MichaIng the checker does stay at the root, but e.g. currently file dietpi/dietpi-autostart
does not exclude false positives (unused variable warnings) due to absolute path import - https://github.com/MichaIng/DietPi/blob/dev/dietpi/dietpi-autostart#L20
Okay, I misunderstood the the source-path
works and on last test I forgot to move the /boot/dietpi/func/dietpi-globals
from the system out of the way.
The shellcheck docs say "Absolute paths will also be rooted in these paths.
" but this means that e.g. . /boot/dietpi/func/dietpi-globals
with source-path=dietpi/func
leads to . dietpi/func/boot/dietpi/func/dietpi-globals
being searched for, so the sourced file path in the script is basically prefixed with the source-path
.
In this case I need to use source=dietpi/func/dietpi-globals
to source this file explicitly. Just tested it and indeed works as expected. So many thanks for this quick implementation, I close this issue now.