search

cea-hpc / modules

Environment Modules: provides dynamic modification of a user's environment
http://modules.sourceforge.net/
GNU General Public License v2.0
668 stars 102 forks source link

Fix as many existing ShellCheck reports as possible #470

Closed lzaoral closed 1 year ago

lzaoral commented 1 year ago

Describe the bug

PR #469 added a ShellCheck linting on PRs with new changes. However, problems in already existing files are not reported (unless they are fixed). This issue tries to aggregate all such files.

List of affected shell scripts for commit 8f5f423958f5ee2818ace7c9a01e4f0351e2905b

ShellCheck version: 8.0.0

--- ./contrib/guide/get_started/00-cleanup.sh ---
      1 SHELLCHECK_WARNING
--- ./contrib/guide/get_started/12-datadir-create.sh ---
      1 SHELLCHECK_WARNING
--- ./contrib/scripts/resetgnome ---
      7 SHELLCHECK_WARNING
--- ./contrib/template/.bash_profile ---
      2 SHELLCHECK_WARNING
--- ./contrib/template/.bashrc ---
      2 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/bar-defaults.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/bar-loads.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/bar-switch.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/common_code.sh ---
      6 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/foo-avail1.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/foo-avail2.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/foo-defaults.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/foo-loads.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/foo-switch.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/modavail.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/modversion.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/ompi-defaults.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/ompi-loads1.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/ompi-switch.sh ---
      1 SHELLCHECK_WARNING
--- ./doc/example/compiler-etc-dependencies/example-sessions/ompi-switch.sh.m431 ---
      3 SHELLCHECK_WARNING
--- ./doc/example/source-script-in-modulefile/bar-2.1/bar-setup.sh ---
      3 SHELLCHECK_WARNING
--- ./doc/example/source-script-in-modulefile/foo-1.2/foo-setup.sh ---
      3 SHELLCHECK_WARNING
--- ./init/bash.in ---
      9 SHELLCHECK_WARNING
--- ./init/bash_completion.in ---
     63 SHELLCHECK_WARNING
--- ./init/ksh.in ---
      9 SHELLCHECK_WARNING
--- ./init/profile.sh.in ---
      4 SHELLCHECK_WARNING
--- ./init/sh.in ---
      9 SHELLCHECK_WARNING
--- ./script/add.modules.in ---
     52 SHELLCHECK_WARNING
--- ./script/commit-msg ---
      3 SHELLCHECK_WARNING
--- ./script/envml ---
      1 SHELLCHECK_WARNING
--- ./script/mkroot ---
     13 SHELLCHECK_WARNING
--- ./script/mt ---
     12 SHELLCHECK_WARNING
--- ./script/pre-commit ---
      9 SHELLCHECK_WARNING
--- ./testsuite/bin/install_test_sh ---
     16 SHELLCHECK_WARNING
--- ./testsuite/cmd.exe ---
      1 SHELLCHECK_WARNING
--- ./testsuite/example/mini-sh-to-mod.sh ---
      1 SHELLCHECK_WARNING
--- ./testsuite/example/sh-to-mod.sh ---
     10 SHELLCHECK_WARNING
--- ./testsuite/home/.module/.target ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/coll1 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/coll2 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/coll3 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/coll4.target ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/coll6.target ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/default ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.module/infocmdexp ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.modules.save ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.modules.saveempty ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/.modules.savenull ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-hide-once-loaded-nuasked ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-0 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-1 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-2 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-3 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-4 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-5 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll-sticky-6 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll10 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll11 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll12 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll13 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll14 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll15 ---
      7 SHELLCHECK_WARNING
--- ./testsuite/home/coll16 ---
     11 SHELLCHECK_WARNING
--- ./testsuite/home/coll26 ---
      9 SHELLCHECK_WARNING
--- ./testsuite/home/coll27 ---
      5 SHELLCHECK_WARNING
--- ./testsuite/home/coll28 ---
      5 SHELLCHECK_WARNING
--- ./testsuite/home/coll29 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll30 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll31 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll32 ---
      3 SHELLCHECK_WARNING
--- ./testsuite/home/coll33 ---
      3 SHELLCHECK_WARNING
--- ./testsuite/home/coll39 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll5 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll7 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll8 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/coll9 ---
      1 SHELLCHECK_WARNING
--- ./testsuite/home/rcnomagic ---
      1 SHELLCHECK_WARNING
--- ./testsuite/systest ---
      3 SHELLCHECK_WARNING

I've tried to filter all unrelated files from this list but It's possible that some of them are not actual shell scrips.

EDIT: Remove ./script/mb since it's not a shell script.

xdelaruelle commented 1 year ago

Thanks for this report. Could you please detail how shellcheck was run to obtain this report. Many files checked in the above report are modulefiles or module collection, not shell scripts.

xdelaruelle commented 1 year ago

Never mind my previous comment. I have checked the whole repository and find the files to check with shellcheck:

script/add.modules.in
script/commit-msg
script/envml                                                                                             
script/mkroot                                                                                            
script/modulecmd.in                                                                                      
script/mt                                                                                                
script/pre-commit                                                                                        
configure                                                                                                
init/bash_completion.in
init/profile.sh.in
init/bash.in
init/sh.in
init/ksh.in
testsuite/systest1
testsuite/not_installed
testsuite/systest0
testsuite/mode
testsuite/systest
testsuite/systest2
testsuite/stty
testsuite/cmd.exe
testsuite/is_module_defined
testsuite/bin/install_test_sh
contrib/get_started/*.sh
file $(find doc/example/) | grep 'Bourne-Again'

I will take this opportunity to also remove unmaintained and (I think) deprecated scripts from repository:

contrib/template/.bashrc
contrib/template/README.dotfiles
contrib/template/.mailcap
contrib/template/.fvwm2rc
contrib/template/resetenv.conf
contrib/template/.logout
contrib/template/.mime.types
contrib/template/.cshrc
contrib/template/.bash_profile
contrib/template/.aliases
contrib/template/.login
contrib/scripts/resetenv
contrib/scripts/resetgnome
lzaoral commented 1 year ago

Thank you, @xdelaruelle! I also went through the whole repository and I listed all text files that looked like shell scripts both to me and to ShellCheck. I've already started fixing some reports so I'll post a PR soon.

xdelaruelle commented 1 year ago

On my side, I will create a new lint testsuite that will be callable with make testlint or script/mt lint. It will detect all the script shells in the repository and run shellcheck on all of that.

I will also check the Tcl scripts with Nagelfar through this testsuite.

lzaoral commented 1 year ago

@xdelaruelle, thanks again for the fixes! I've not forgotten about this issue but, unfortunately, I don't know when I'll be able get to it again because right now I'm busy with finishing my thesis and other, more urgent, stuff related to RHEL.

xdelaruelle commented 1 year ago

No problem @lzaoral. I am closing this issue, as the work achieved for next version (5.2) on this topic is already a good step forward.

But do not hesitate to send other pull requests on this topic when you will have more time. This is much appreciated.