Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shellcheck for checking commands inside recipes #83

Open
EmilyGraceSeville7cf opened this issue Mar 18, 2023 · 8 comments
Open

Comments

@EmilyGraceSeville7cf
Copy link

Expected behaviour

Shell code is expected to be linted.

Actual behaviour

Now no errors are thrown for incorrect shell commands.

Output of checkmake --version

Output of checkmake --debug <your makefile>

Output of make --version

Sample Makefile to reproduce issue

(some of these things might not apply but the more you can provide the easier
it will be to fix this bug. Thanks!)

@jenstroeger
Copy link

If the linted Makefile sets the shell then running shellcheck on individual recipes would make a lot of sense.

@cooljeanius
Copy link

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

@jenstroeger
Copy link

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

In which case it should be escaped using $$(...) as you can see in this Makefile.

@cooljeanius
Copy link

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

In which case it should be escaped using $$(...) as you can see in this Makefile.

Is there a way to edit shellcheck's output to get it to say that instead, though?

@jenstroeger
Copy link

Is there a way to edit shellcheck's output to get it to say that instead, though?

You mean ask shellcheck to propose $$(...) to replace backticks if it checks Makefile recipes?

@cooljeanius
Copy link

Is there a way to edit shellcheck's output to get it to say that instead, though?

You mean ask shellcheck to propose $$(...) to replace backticks if it checks Makefile recipes?

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

@jenstroeger
Copy link

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

Can checkmate relay the output? That’d be very useful, esp if it can readjust the line numbers. Regarding the escaping, I think checkmate should warn and suggest these two options — hopefully the user understands the difference and chooses whichever works best for her in context.

@cooljeanius
Copy link

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

Can checkmate relay the output?

I don't know; hey @mrtazz can it do so?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants