Skip to content
This repository was archived by the owner on Nov 30, 2021. It is now read-only.

fix(Makefile): specify /bin/bash as Makefile $(SHELL)#1380

Closed
mboersma wants to merge 1 commit intomasterfrom
fix-make-run-error
Closed

fix(Makefile): specify /bin/bash as Makefile $(SHELL)#1380
mboersma wants to merge 1 commit intomasterfrom
fix-make-run-error

Conversation

@mboersma
Copy link
Copy Markdown
Member

We have one bash-ism in the main Makefile, and we generally expect
to use /bin/bash, so we should make it explicit. See:
https://www.gnu.org/software/make/manual/make.html#Choosing-the-Shell

Fixes #1341.

We have one bash-ism in the main Makefile, and we generally expect
to use bash, so we should make it explicit. See:
https://www.gnu.org/software/make/manual/make.html#Choosing-the-Shell
@carmstrong
Copy link
Copy Markdown
Contributor

Wondering if the better solution here isn't to remove the bash-ism entirely. I proposed a change to this line to @Xe to test - not sure where we got with that.

@mboersma
Copy link
Copy Markdown
Member Author

I think that would be better too if line 51 is the only actual bash-ism, which I think is the case. I am just trying to tidy the CI output mostly.

@mboersma
Copy link
Copy Markdown
Member Author

Jenkins, test this please.

@carmstrong
Copy link
Copy Markdown
Contributor

-               if [[ `$(FLEETCTL) list-units | grep $$UNIT` ]]; then \
+               EXISTS=`$(FLEETCTL) list-units | grep $$UNIT` ; \
+               if [ "$EXISTS" != "" ]; then \

This should be safe for sh and have the same logic...

@Xe
Copy link
Copy Markdown
Contributor

Xe commented Jul 21, 2014

This change messes up the ANSI color code output:

\033[0;33mWaiting for deis-controller to start...\033[0m

Xe added a commit that referenced this pull request Jul 21, 2014
Mac OSX's make has support for `[[`, Ubuntu's does not. This fix (as
suggested and written up by @carmstrong on a comment[1]) removed the
bashism in favor of a more compatible syntatic trick.

[1]: #1380 (comment)
@mboersma
Copy link
Copy Markdown
Member Author

Closing in favor of #1382.

@mboersma mboersma closed this Jul 21, 2014
@mboersma mboersma deleted the fix-make-run-error branch July 21, 2014 19:47
Xe added a commit that referenced this pull request Jul 21, 2014
Mac OSX's make has support for `[[`, Ubuntu's does not. This fix (as
suggested and written up by @carmstrong on a comment[1]) removed the
bashism in favor of a more compatible syntatic trick.

[1]: #1380 (comment)
Xe added a commit that referenced this pull request Jul 21, 2014
Mac OSX's make has support for `[[`, Ubuntu's does not. This fix (as
suggested and written up by @carmstrong on a comment[1]) removed the
bashism in favor of a more compatible syntatic trick.

[1]: #1380 (comment)

Amended to fix a "$$" in place of a "$"
carmstrong added a commit that referenced this pull request Jul 21, 2014
fix(Makefile): remove bashism in the main Makefile
Xe added a commit that referenced this pull request Jul 22, 2014
Mac OSX's make has support for `[[`, Ubuntu's does not. This fix (as
suggested and written up by @carmstrong on a comment[1]) removed the
bashism in favor of a more compatible syntatic trick.

[1]: #1380 (comment)

Amended to fix a "$$" in place of a "$"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell errors on make run

3 participants