Rewrite Makefile shellcheck rule to find all shellscripts #10512
Conversation
|
This change causes the We could either exclude this check (probably bad idea, I think vdev_id needs to be fixed here) or fix all those files.... |
Initially we excluded some shell scripts while we focused on getting the rest ship shape. The intention was to eventually expand shellcheck to all the scripts as you've done in this PR. However, due to the number of scripts which need to be fixed I think a reasonable next step would be expand the shellcheck to all scripts except the |
|
Okay, I will re-adjust the code to exclude ksh files. |
Thanks. You'll also need to make sure this PR fixes whatever issues are uncovered in the other scripts before we can merge it. Otherwise we'll break the style check for everyone. |
I'll do my best to address the issues. It stands at 1766 excluding the tests so it'll take a while. |
|
Update, not as bad, only 259, all the shell scripts in config/ are auto-generated. We'll have to fix those by auditing the m4 files, which for now I think is out-of-scope of this. |
Rewrite the find command which determines which files are passed to shellcheck in order to include any file with a shell-like shebang, since not all shell scripts have a ".sh" or ".ksh" extension. Exclude tests until huge number of issues can be addressed. Signed-off-by: Gabriel A. Devenyi <gdevenyi@gmail.com>
|
@gdevenyi: I should have some hours to spare this weekend an am familiar with POSIX shell. So if you have trivial work left by Friday evening and think another pair of hands would be useful, feel free to contact me. I'm just not sure if my help would be worth the extra effort for synchronizing between two people changing the code. So no hard feelings if I don't hear from you. 😉 |
|
@mschilli87 I would be happy to accept PRs on this branch on my fork if you want to help contribute to the fixes! |
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512 Closes openzfs#12101
This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512 Closes openzfs#12101
Motivation and Context
As noted by #10510 a number of bashisms and related issues have crept into the shell scripts in the codebase. The current code does not successfully locate all shell scripts, and hence doesn't test all of them.
Description
Rewrote the find code which locates shell scripts to explicitly pattern patch against the shebang of every file in order to locate sh/bash/dash/ksh scripts.
How Has This Been Tested?
Tested locally on Ubuntu 18.04 it finds more shell scripts now, such as vdev_id
Types of changes
Checklist:
Signed-off-by.