fix(env): an empty .env entry no longer clobbers the caller's value#883
Merged
Conversation
.env is sourced under 'set -o allexport' so it can hold arbitrary shell, which made every KEY= line an unconditional assignment. Merely listing a name blanked it, so 'BASHUNIT_OUTPUT_FORMAT=tap ./bashunit' stopped working in any project whose .env mentioned that setting -- and a project could not document a setting without breaking it. Snapshot the caller's non-empty BASHUNIT_* values and restore any the file blanked. An entry with a value still wins, which is what a committed project config is for; only an empty one is treated as 'not configured here', matching how .bashunitrc already applies values. This had been concealing defects rather than merely annoying: two bugs fixed in #879 were not reproducible from inside a repo checkout because .env blanked the very variable under test. The two tests that needed --skip-env-file as a workaround now exercise the real path. Verified on real bash 3.0 (compgen -v accepts a prefix there) and under CI conditions with 'cp .env.example .env'. Closes #865
…new test CI caught SC2164/SC1007 in the new test file that 'make sa' had passed. Cause: since #863 the file list comes from 'git ls-files', which only lists TRACKED files, so a brand-new script was invisible locally and first failed in CI -- exactly backwards. Add --others --exclude-standard. Verified by dropping an untracked offending script in the tree: make sa now fails on it.
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.
🤔 Background
Related #865
.envis sourced underset -o allexportso it can hold arbitrary shell, which madeevery
KEY=line an unconditional assignment. Merely listing a name blanked it, soBASHUNIT_OUTPUT_FORMAT=tap ./bashunitsilently stopped working in any project whose.envmentioned that setting — and a project could not document a setting withoutbreaking it.
This was concealing defects, not just causing annoyance: two of the bugs fixed in #879
were not reproducible from inside a repo checkout, because
.envblanked the veryvariable under test.
💡 Changes
BASHUNIT_*values before sourcing.envand restore any the file blanked; an entry with a value still wins, only an empty one is treated as "not configured here"--skip-env-fileworkaround from the two threshold tests, so they exercise the real path; correct the.env.exampleheader and the precedence list in Configuration, which conflated.envwith environment variablescompgen -vaccepts a prefix there) and under CI conditions withcp .env.example .env