fix(dev-env): read the export-prefixed lines a .env may carry - #43
Conversation
A `.env` is sourced, so `export FOO=bar` is as valid in it as `FOO=bar`, and a file copied from somewhere else very often carries the prefix. The pattern required `=` immediately after the name, so those lines were skipped — **silently**, which is the same failure #42 removed: a variable present in the file, absent from the server, and nothing anywhere saying so. Only the name has to be recognised. The value needs no unwrapping, because `set -a` and the `.` above already sourced the file: the shell treated `export FOO=bar` as the assignment it is, so `${!name-}` reads what the file set, prefix or not. Nothing else in this script has to know about the two spellings — worth saying in the comment, or the next reader wonders whether the prefix must be stripped somewhere. Named and not fixed: two assignments on one line, `export FOO=bar baz=qux`. Valid shell, and the pattern takes only the first. Nobody writes that in a `.env` and no file here does, but the silence has the same shape as the bug above, so it is written down rather than left to be found. # Checked by hand, in a fresh shell The test that means something is not that the name is recognised — it is that it survives the round trip with the right value. Sourced, emitted, re-evaluated in a separate `bash -c`, read back: - `export PREFIXED=with_export` → `with_export`, where `dev` gives nothing at all - `export SPACED_PREFIX=…` with several spaces → read - `export APOSTROPHE="it's exported"` → `it's exported`, the two difficulties at once, since an apostrophe is what breaks the emitted quoting - `export EMPTY_EXPORT=` → empty string, not unset - `PLAIN=bare` still read, and `DATABASE_URL` still derived per branch One trap worth recording, because it cost both of us an hour between us: a test `.env` must itself be valid shell. A bare apostrophe in it makes the sourcing fail silently and every value comes out empty, which reads exactly like a broken patch.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9ff372e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # shell, and the pattern would take only the first. Nobody writes that in a `.env` and no file in | ||
| # this repository does, but the silence is the same shape as the bug above, so it is named here | ||
| # rather than left to be discovered. | ||
| sed -n 's/^[[:space:]]*\(export[[:space:]]\{1,\}\)\{0,1\}\([A-Za-z_][A-Za-z0-9_]*\)=.*/\2/p' "$env_file" | while read -r name; do |
There was a problem hiding this comment.
When .env contains the newly supported export WHISPEE_DEV_DATABASE=..., this loop emits that value after the script's computed WHISPEE_DEV_DATABASE because the derived exclusion list omits it (and WHISPEE_DEV_BRANCH). Since the callers evaluate the output in order, scripts/dev-server.sh then checks or creates the overridden database while DATABASE_URL still points to the computed branch database, causing a fresh branch server to fail because its actual database was never created. Add both computed development identifiers to the exclusion list.
Useful? React with 👍 / 👎.
1 participant
Follow-up to #42, from a limit a review caught there rather than from a new symptom.
A
.envis sourced, soexport FOO=baris as valid in it asFOO=bar— and a file copied from another project very often carries the prefix. The pattern required=immediately after the name, so those lines were skipped silently. That is the same failure #42 removed: a variable present in the file, absent from the server, and nothing anywhere saying so.What changes
exportbecomes an optional prefix in the pattern. Only the name has to be recognised: the value needs no unwrapping, becauseset -aand the.above already sourced the file — the shell treatedexport FOO=baras the assignment it is, so${!name-}reads what the file set either way. Nothing else in the script has to know about the two spellings, which is worth stating in the comment or the next reader wonders whether the prefix must be stripped somewhere too.Named, not fixed
export FOO=bar baz=qux— two assignments on one line. Valid shell, and the pattern takes only the first. Nobody writes that in a.envand no file in this repository does, but the silence has the same shape as the bug above, so it is written into the comment rather than left to be discovered.What was run
A shell script whose output is
evaled is not unit-testable, and the test that means something is not that a name is recognised — it is that it survives the round trip with the right value. Sourced, emitted, re-evaluated in a separatebash -c, read back:dev)export PREFIXED=with_exportwith_exportexport SPACED_PREFIX=…(several spaces)export APOSTROPHE="it's exported"it's exportedexport EMPTY_EXPORT=PLAIN=barebarebare— no regressionDATABASE_URLThe apostrophe case is the one worth having: it puts both difficulties together, since an apostrophe is what breaks the emitted quoting.
bash -n scripts/dev-env.shcargo test -p server --release— nine suites, greenA trap worth recording
A test
.envmust itself be valid shell. A bare apostrophe in it makes the sourcing fail silently and every value comes out empty — which reads exactly like a broken patch. It cost an hour across two sessions before either of us noticed the fixture was the problem, not the code.