Skip to content

Fix check_shell: $(echo $0) is expanded by make, not the shell - #47

Open
richard-burhans wants to merge 1 commit into
hillerlab:mainfrom
richard-burhans:fix-check-shell
Open

richard-burhans wants to merge 1 commit into
hillerlab:mainfrom
richard-burhans:fix-check-shell

Conversation

@richard-burhans

Copy link
Copy Markdown

Fixes #46.

check_shell used $(echo $0), which make expands as a variable reference — a variable named echo $0, undefined, so empty — before the recipe reaches the shell. The test became [ != "-bash" ], a syntax error with no left operand, so [ exited non-zero and the else branch ran. The target printed "successfully passed" unconditionally on any shell, and only ever echod, so check could not gate on it either.

Escaping to $$(echo $$0) is not sufficient: $0 in a recipe is the shell make spawned (/bin/sh), never the string -bash, which is argv[0] of an interactive login shell. That variant fails unconditionally instead.

This tests $BASH_VERSION and sets SHELL := /bin/bash so recipes actually run under bash rather than dash on Debian/Ubuntu — which is what the check was asserting all along. The two changes go together: without the SHELL line, a BASH_VERSION test would fail on any system where /bin/sh is dash.

$ make check_shell            # bash
bash detected (5.3.9(1)-release); shell type check successfully passed   # exit 0

$ make SHELL=/bin/dash check_shell
ERROR: TOGA2 currently only supports bash as operating shell. ...
make: *** [Makefile:60: check_shell] Error 1                             # exit 1

Recipes were scanned for sh-isms (echo -e, source, [[, local, shell functions) — there are none — and every phony target dry-runs clean, so running them under bash changes nothing else.

check_shell used $(echo $0), which make expands as a variable reference (a
variable named "echo $0", undefined -> empty) before the recipe reaches the
shell. The test became `[  != "-bash" ]`, a syntax error with no left operand,
so `[` exited non-zero and the else branch ran: the target printed
"successfully passed" unconditionally, on any shell, and never exited non-zero
so `check` could not gate on it.

Escaping to $$(echo $$0) is not sufficient: $0 in a recipe is the shell make
spawned (/bin/sh), never the string "-bash", which is argv[0] of an interactive
login shell. That variant fails unconditionally instead.

Test $BASH_VERSION instead, and set SHELL := /bin/bash so recipes actually run
under bash rather than dash on Debian/Ubuntu -- which is what the check was
asserting all along. The two go together: without the SHELL line a BASH_VERSION
test would fail on any system where /bin/sh is dash.

  under bash:  bash detected (5.3.9(1)-release); shell type check successfully passed  (exit 0)
  under dash:  ERROR: TOGA2 currently only supports bash ...                            (exit 1)

Recipes were checked for sh-isms; there are none, so running them under bash
changes nothing else.
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.

make check_shell always reports success: $(echo $0) is expanded by make, not the shell

1 participant