Repository navigation
Fix check_shell: $(echo $0) is expanded by make, not the shell - #47
Open
richard-burhans wants to merge 1 commit into
Open
richard-burhans wants to merge 1 commit into
richard-burhans wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #46.
check_shellused$(echo $0), which make expands as a variable reference — a variable namedecho $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 theelsebranch ran. The target printed "successfully passed" unconditionally on any shell, and only everechod, socheckcould not gate on it either.Escaping to
$$(echo $$0)is not sufficient:$0in 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_VERSIONand setsSHELL := /bin/bashso 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 theSHELLline, aBASH_VERSIONtest would fail on any system where/bin/shis dash.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.