Fix #524: add writability check for TFENV_CONFIG_DIR before install lock#527
Merged
Fix #524: add writability check for TFENV_CONFIG_DIR before install lock#527
Conversation
When TFENV_CONFIG_DIR is read-only (e.g. Homebrew Cellar on CI runners), the mkdir-based install lock fails with EACCES, which was misreported as lock contention. This adds a writability check early in tfenv-install: - Non-interactive (CI/pipes): fails immediately with a clear error message explaining how to set TFENV_CONFIG_DIR to a writable path. - Interactive terminals: prompts the user to fall back to ~/.tfenv. The mkdir -p and writability check are placed before dst_path and lockdir are derived, so fallback to ~/.tfenv correctly updates all dependent paths. Also adds test/test_install_lock.sh with 5 tests covering: 1. Install with non-existent TFENV_CONFIG_DIR (regression test for #487/#525) 2. Read-only config dir, non-interactive — expects clear error 3. Read-only config dir, interactive fallback accepted 4. Read-only config dir, interactive fallback declined 5. Lock directory cleanup after successful install
BSD script expects the command as separate positional arguments, not as
a single string. Passing "${TFENV_ROOT}/bin/tfenv install ${version}" as
one argument made BSD script try to exec a program whose filename
literally contained spaces, producing "No such file or directory".
Split into separate args: "${TFENV_ROOT}/bin/tfenv" install "${version}"
Also switch echo to printf for portability across platforms.
Fixes macOS CI failure on PR #527.
BSD script(1) on macOS does not reliably forward piped stdin to the child process through the PTY. When the stdin pipe closes (after printf exits), BSD script closes the PTY master before the child reaches read(1), causing the interactive prompt to receive no input. Rather than maintaining fragile platform-specific script(1) invocations (GNU vs BSD syntax, stdin forwarding differences), add a TFENV_FORCE_INTERACTIVE env var that bypasses the [[ -t 0 ]] check. Tests set this env var and pipe input directly — simple, portable, and reliable on both Linux and macOS CI.
…nfig dir parent is not writable When TFENV_CONFIG_DIR does not exist AND its parent directory is not writable (e.g. Homebrew Cellar owned by linuxbrew), mkdir -p fails and the previous || log "error" killed the process immediately, preventing the interactive fallback prompt from ever firing. Change mkdir -p to suppress errors (2>/dev/null) and add a ! -d check to the existing writability guard. This covers all three failure cases with one unified check: 1. Dir exists but not writable -> caught by ! -w 2. Dir does not exist, parent writable -> mkdir -p succeeds, check passes 3. Dir does not exist, parent NOT writable -> mkdir -p fails, caught by ! -d Add two new test cases for scenario 3 (non-interactive and interactive fallback). Ref #524
Replace fire-and-forget mkdir -p 2>/dev/null with captured error output and systematic diagnosis: - Capture mkdir stderr for precise error relay - Walk up path ancestors to identify the exact blocking component - Report ownership via stat (GNU -c format with BSD -f fallback) - Report permissions mode for existing-but-unwritable directories - Distinguish: not-a-directory, non-writable parent, non-writable dir, and unexpected mkdir failures — each with specific messaging - Tests verify diagnostics include owner and parent identification
Add four new test cases (Tests 8-11) covering previously untested code paths in the config directory diagnostic section of tfenv-install: - Test 8: Config dir path exists but is a file, not a directory - Test 9: Ancestor in config dir path is a file, not a directory - Test 10: Interactive fallback to ~/.tfenv fails when HOME is non-writable - Test 11: Non-existent config dir with non-writable parent, interactive fallback declined by user These cover all remaining branches in the config dir validation logic (lines 89-141 of tfenv-install). Existing tests renumbered accordingly.
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.
Fixes #524
Problem
When
TFENV_CONFIG_DIRpoints to a read-only directory (e.g. Homebrew Cellar on CI runners where the install is owned bylinuxbrewbut the runner user cannot write),mkdir "${lockdir}"fails with EACCES. PR #523 (v3.2.1) improved the error message, but the install still fails.Solution
Add a writability check (
[ -w "${TFENV_CONFIG_DIR}" ]) early inlibexec/tfenv-install, after the existingmkdir -pand BEFOREdst_pathandlockdirare derived:TFENV_CONFIG_DIRto a writable path[[ -t 0 ]]): prompts the user to fall back to~/.tfenvBy placing the check before
dst_pathandlockdirare derived, the fallback correctly updates all dependent paths.Tests
New test file
test/test_install_lock.shwith 5 tests:TFENV_CONFIG_DIR(regression test for tfenv install fails on a clean machine: lock mkdir ENOENT misreported as "Another process is installing" #487/cannot install terraform through tfenv #525)~/.tfenvAll 5 tests pass. Signal-handler cleanup (test 6) is intentionally skipped as inherently racy.