Fix operator precedence bug and unbound variable in tfenv-version-name.sh#461
Merged
Fix operator precedence bug and unbound variable in tfenv-version-name.sh#461
Conversation
…e.sh Fix #406: Replace undefined ${requested} variable with ${TFENV_VERSION} in the error message on line 82. The variable was introduced in commit 3dc5819 but never existed in scope, causing an unbound variable crash under set -u before the error message could be displayed. Fix #431: Fix shell operator precedence in the version file read on line 13. The original "cat ... || true | tr" meant tr only ran on the failure path due to || binding tighter than |. Restructured to "cat ... | tr ... || true" so carriage returns are always stripped. Also addresses #447 (WSL carriage return symptoms) which shares the same root cause as #431.
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
1 participant
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.
Summary
Two bugs fixed in
lib/tfenv-version-name.sh:1. Shell operator precedence bug (line 13) — Fixes #431, #447
Before:
TFENV_VERSION="$(cat "${TFENV_VERSION_FILE}" || true | tr -d '\r')"Due to shell precedence,
||bindscattotrue, making the pipelinecat ... || (true | tr ...). Thetr -d '\r'only runs on the failure path. Whencatsucceeds (the normal case), carriage returns are never stripped.After:
TFENV_VERSION="$(cat "${TFENV_VERSION_FILE}" | tr -d '\r' || true)"tris now always in the pipeline. The|| trueensures the overall command does not fail if the file is missing.This fixes the root cause of #431 (trailing whitespace breaking version resolution) and #447 (WSL/Windows carriage returns causing garbled output).
2. Undefined variable in error message (line 82) — Fixes #406
Before:
${requested}was never defined in this scope, causing an unbound variable crash underset -ubefore the error message could be displayed.After:
Testing
./test/run.sh test_install_and_use.shon Linux (Ubuntu) — 41/41 tests pass, 0 failures.