A worktree configures itself on first start - #13
Merged
Conversation
`ddev hostshift init` was a step every worktree needed and nobody could guess. The add-on now derives .ddev/.env from a pre-start hook, so the flow is `git worktree add`, install deps, `ddev add-on get`, `ddev start`. Three things decide the shape, and each was the wrong answer first. **In the add-on, not in a file the project commits.** config.hostshift.yaml already exists for a hook alone, carries #ddev-generated, and ships in the same tarball as the command it calls, so the two cannot drift. A committed bootstrap would be the one file `ddev add-on get` can never replace — the exact shape of the two costliest upgrades this add-on has had, made structural and copied into every client repo. **`init --no-restart`, never `env`.** Every refusal that makes init safe is gated on `[ "$cmd" = "init" ]`: the one that stops a worktree writing a map built from its own hostnames when the parent cannot be read, and the note about hostnames inherited from the parent. `env` prints past all of them, and `check` would then agree with the result, because it recomputes the same thing. Measured on the real thing: the hook prints the inherited-hostnames note that `env` would have skipped. **Gated on a linked worktree**, because `.git` is a file there and a directory in the parent. Running this in the parent would write it a map of its own hostnames and take the canonical name the database holds. "not a linked worktree" is a warning, not a refusal, so the gate is load-bearing rather than tidiness. The guard is a HOSTSHIFT_ line rather than the file existing: init writes through a temp file and a rename so a failure leaves nothing, but an empty .ddev/.env from any other source would otherwise skip the derive on every start while the post-start check short-circuits silently on the same file. Two test bugs found on the way, both of which had made assertions stop asserting: - the tag loop compared each tag's command against the working tree's and read "differs" as "old". That was a proxy for the property — whether the command carries #ddev-generated, which is what DDEV actually tests — and it broke the moment a third tag existed. v0.2.1 and v0.2.2 differ from HEAD and carry the marker, so they neither warn nor should. - the hook extraction took the first exec-host in the file. Adding a pre-start block above post-start made it test the wrong hook rather than fail.
…he hook Six defects, and two claims in the previous commit message were wrong. **The inherited-hostnames note is back off.** :1252 records why it was taken off `check`: the post-start hook printed "until `ddev restart`, those serve this worktree" from inside the very restart that handed them back. The pre-start hook is that hook again, and on a first-ever start nothing was ever serving them. The previous commit offered this as *evidence the design was right*. Gated on --no-restart; every refusal still runs. **`|| true` is gone.** DDEV prints `Task failed:` on a non-zero hook and only aborts under fail_on_hook_fail, so swallowing the status swallowed the only marker. And when the derive refuses there is no .ddev/.env, so the post-start check exits 0 at :148 without a word — the refusal scrolled past mid-start and nothing downstream reported it. Now it says so and still exits 0, because a project running fail_on_hook_fail must be able to start when init legitimately refuses. **The exit status is a real loss, now stated.** --no-restart skips the block where init re-runs check and answers with it — added because "an agent that checks $? was told everything was fine". `ddev start` exits 0 whatever check finds. The README says to run `ddev hostshift check` when a status is wanted. **The drift claim was too strong.** "Cannot drift" is false in the one case this file documents at length: DDEV will not replace a pre-marker command, so such a worktree runs this hook against a parser that exits 2 on --no-restart. The HOSTSHIFT_ guard covers every *configured* project, since v0.1.0 wrote those lines too. **The temp file could outrank the file it replaces.** `mktemp .ddev/.env.XXXXXX` matches DDEV's EnvFiles() glob, and DDEV passes each match as a *later* --env-file. The EXIT trap normally clears it; init now runs inside `ddev start`, where an interrupted start is routine. Renamed so it dodges the glob and falls under the existing ignore rule. **The hook is executed by the tests now, not grepped.** Three greps agreeing with a string nobody ran is exactly how the post-start extraction came to select the wrong block and still pass. Five cases: derives when absent, derives when *empty* — the whole reason the guard is a HOSTSHIFT_ line — no-ops when configured, writes nothing in the parent, and the temp file stays outside DDEV's glob. Two corrections to my own account: - "silently tested the wrong hook … asserts nothing rather than fails" — it fails loudly; the v0.1.0 iteration goes red. - "v0.2.1 and v0.2.2 differ from HEAD" — all four v0.2.x commands are byte-identical to master. What breaks the old assertion is this branch editing the command, i.e. the ordinary state between releases. The fix was still needed; the reason given for it was not. README, the flag table, both worked examples and install.yaml's closing line all said `ddev hostshift init` was required per worktree. It is not any more.
Eleven findings. The two that mattered are both in the tests I added to prove the last round of fixes. **A deleted stub left on PATH.** `rm -rf "$work/failddev"` ran *above* the --no-restart block that prepends it, so `command -v ddev` fell through to the real binary. On CI, with no ddev installed, the assertion passed whether or not the flag did anything; on a developer machine a regression would have run an actual `ddev restart` from a suite the Makefile calls "no Docker and no DDEV needed". Moved below. **The leftover-temp-file glob no longer matched the template.** Renaming to `.hostshift-env.XXXXXX` left `ls .ddev/.env.??????` matching nothing, so "a failed init leaves no temp file behind" became true however the code behaved — the EXIT trap could have been deleted. It now globs both. Two more tests passed for the wrong reason: - "the pre-start hook is a no-op once configured" compared .ddev/.env byte for byte, which holds with the guard removed, because init is idempotent anyway. It asserts no *output* now: the cost of running is a collision scan and a docker inspect on every start. - "the env temp file is outside DDEV's glob" grepped the source, which fails open — no match lands in the catch-all arm — and `mktemp -p .ddev .env.XXXXXX` would reintroduce the bug while defeating the pattern. It runs init against an unwritable file and globs the directory instead. **The hook conflated unreadable with unconfigured.** `grep -q … 2>/dev/null` on `.ddev/.env` is the thing the command rejects `grep -qs` for by name (:143). A chmod-000 file made the hook re-derive a configured project and print a raw `sed: Permission denied` ahead of `check`'s precise diagnosis. **A worktree without the CLI nagged on every start, permanently** — the guard never becomes satisfied, and install.yaml declares the CLI optional. That one failure is silent now; `check` still reports it. **The note gate keyed on the wrong thing.** `-z "$no_restart"` suppressed the inherited-hostnames note for `init --no-restart` run by hand, where nothing restarts and the note is *true*. HOSTSHIFT_HOOK is the property — "I am inside a start" — and the post-start hook already uses it. **The feature had no integration coverage.** Every integration case ran `ddev hostshift init` before the worktree's first start, making the hook a no-op in all of them. The premise — that pre-start lands before compose reads the file — is a claim about DDEV's Start() ordering that only a real `ddev start` can test. One case now starts unconfigured and asserts .ddev/.env appears. Three doc contradictions: README:177 still said "after `ddev restart`" thirty lines after saying there is no second start; README:194, the only place the ddev/ddev#5486 hazard is explained, still credited the note to `init`; and install.yaml's closing message was wrong for a non-worktree install, where the hook does not fire and init is still required.
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
2 participants
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.
ddev hostshift initwas a step every worktree needed and nobody could guess. The add-on now derives.ddev/.envfrom a pre-start hook, so the flow is:git worktree add ../acme-wt-a -b feature/ABC-123 cd ../acme-wt-a composer install:development ddev add-on get https://github.com/generoi/hostshift/releases/latest/download/hostshift-ddev.tar.gz ddev startVerified end to end on a real multisite worktree: from no
.env, oneddev restartderived it, containers picked it up in the same start,VIRTUAL_HOSTcorrect, site served 200. Second start is a no-op.Three decisions, each of which I got wrong first
In the add-on, not in a file the project commits.
config.hostshift.yamlalready exists for a hook alone, carries#ddev-generated, and ships in the same tarball as the command it calls — so the two cannot drift. A committed bootstrap would be the one fileddev add-on getcan never replace: the exact shape of the two costliest upgrades this add-on has had, made structural and copied into every client repo.init --no-restart, neverenv. Every refusal that makesinitsafe is gated on[ "$cmd" = "init" ]— including the one at:2717that stops a worktree writing a map built from its own hostnames when the parent cannot be read.envprints past all of them, andcheckwould then agree, because it recomputes the same thing. Confirmed on the real run: the hook printed the inherited-hostnames note thatenvwould have skipped.Gated on a linked worktree.
.gitis a file there and a directory in the parent. Running this in the parent would write it a map of its own hostnames and take the canonical name the database holds."not a linked worktree"is a warning, not a refusal, so the gate is load-bearing.The guard is a
HOSTSHIFT_line rather than the file existing:initwrites through a temp file and a rename so a failure leaves nothing, but an empty.ddev/.envfrom any other source would otherwise skip the derive forever while the post-start check short-circuits silently on the same file.Two test bugs found on the way
Both had made assertions stop asserting:
#ddev-generated— and broke as soon as a third tag existed.exec-hostin the file. Adding apre-startblock abovepost-startmade it test the wrong hook rather than fail.306 add-on assertions pass; Go suite green.
Deliberately not done
ddev add-on getstill runs once per worktree. Removing it needs a committed file that fetches and executes code duringddev start— unpinned, unchecksummed, bypassed by--skip-hooks, and a network dependency in CI. The better home for that line iscomposer install:development, where a fetch is expected and failure is visible.