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

fix: a Makefile’s SHELL variable is not an executable shebang #329

Merged
merged 2 commits into from
Oct 7, 2022

Conversation

jenstroeger
Copy link
Owner

I stumbled upon issues when I played with the .ONESHELL: special target:

> make audit
make: /usr/bin/env bash: No such file or directory
...

Documentation on this is a little sparse, I thought.


I also think we should consider using the special target .ONESHELL:: it runs the entire recipe in a single shell which would allow us to set & use environment variables within that recipe. So far we’ve danced around make running every recipe line in its own shell, and that has complicated some of our process. In that context, it would also make sense to discuss .EXPORT_ALL_VARIABLES:.

There’s also an option to silence make’s command-line echoing, which may be usful?

@jenstroeger jenstroeger requested a review from behnazh September 20, 2022 20:40
Makefile Outdated
# https://stackoverflow.com/questions/10376206/what-is-the-preferred-bash-shebang
SHELL := /usr/bin/env bash
SHELL := bash
.SHELLFLAGS += -e
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me remove this flag for now. From the docs:

You can modify .SHELLFLAGS to add the -e option to the shell which will cause any failure anywhere in the command line to cause the shell to fail, but this could itself cause your recipe to behave differently. Ultimately you may need to harden your recipe lines to allow them to work with .ONESHELL.

So perhaps we should have a separte discussion about .ONESHELL and .SHELLFLAGS and exporting variables to the subshells.

Commit 1ebcf37.

@behnazh
Copy link
Collaborator

behnazh commented Oct 5, 2022

In that context, it would also make sense to discuss .EXPORT_ALL_VARIABLES:.

Not sure what would be the purpose?

@jenstroeger
Copy link
Owner Author

In that context, it would also make sense to discuss .EXPORT_ALL_VARIABLES:.

Not sure what would be the purpose?

See my comment above — let’s discuss this separately?

@jenstroeger jenstroeger requested a review from behnazh October 6, 2022 23:54
@jenstroeger jenstroeger merged commit 89e8520 into staging Oct 7, 2022
@jenstroeger jenstroeger deleted the fix-makefile-shell branch October 30, 2022 05:54
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

Successfully merging this pull request may close these issues.

2 participants