From 767470430b92a7e347fc6d7dbf02f7846a92433d Mon Sep 17 00:00:00 2001 From: mhoennig Date: Sat, 29 Aug 2026 11:26:42 +0200 Subject: [PATCH] Plan the removal of the legacy branches section (step 18) --- docs/plan/18-remove-branches-section.md | 79 +++++++++++++++++++++++++ docs/plan/README.md | 5 ++ 2 files changed, 84 insertions(+) create mode 100644 docs/plan/18-remove-branches-section.md diff --git a/docs/plan/18-remove-branches-section.md b/docs/plan/18-remove-branches-section.md new file mode 100644 index 0000000..9f6b103 --- /dev/null +++ b/docs/plan/18-remove-branches-section.md @@ -0,0 +1,79 @@ +# Step 18: Remove the legacy `branches` section + +Prerequisites: none in code — v0.9.19 already made `builds` and `branches` either-or. +Read `README.md` first. +Scheduled for roughly **2026-09-05**, one week after v0.9.19 (2026-08-29), and only once the precondition below holds. + +Build definitions describe a build completely since v0.9.19, and `branches` is read only while a configuration defines no build at all. +This step deletes the section, its deprecated `autoBuild` schedule, and the either-or branch in the loader, and turns a leftover `branches:` key into a named error instead of a silent loss of settings. + +## Precondition Check (run first, do not skip) + +The removal adds a *rejection by name*: a configuration file that still carries a `branches:` key is refused, because a silently ignored section is exactly the failure this step exists to prevent — and the `gitTally.version` check cannot catch it, since it only bites files that declare a version, which none of the hs.hsadmin.ng configs do. + +So no configuration still in play may contain the section. On vm4006: + +```bash +ssh tallyman@vm4006.hostsharing.net 'cd ~/hs.hsadmin.ng && grep -n "^branches:" .git/gittally/.gittally.yml; for b in $(git for-each-ref --format="%(refname:short)" refs/remotes/origin | sed "s|origin/||"); do if git cat-file -e origin/$b:.gittally.yml 2>/dev/null && git show origin/$b:.gittally.yml | grep -qE "^branches:"; then echo "still legacy: $b"; fi; done' +``` + +Expected output: nothing at all. + +As of 2026-08-29 this listed `master` and five `mihoe/…` branches, plus the machine config. +The plan was: merge `mihoe/reactivate-pi-test` (the first branch with the new shape) to master, rebase the other branches onto the new master, then run this step. +Branches without a committed `.gittally.yml` are fine — they build from the machine config. + +Note that `branches:` also exists as the *selector* key **inside** a build definition (`builds..branches: ["master"]`). That one stays. Only the top-level section goes. The grep above anchors at the line start for exactly that reason. + +## Code + +- `config/GitTallyConfig.kt`: drop the `branches` property and `AutoBuildConfig`, and `BranchConfig.autoBuild` with it. + Rename `BranchConfig` to `BuildSettings` — with the section gone it is no longer a schema type but the resolved answer to "what does this build run", which is all it is used for. + `buildSettings(branch, build)` then no longer needs the branch lookup: `effectiveBuildDefinitions()[build]?.applyTo(BuildSettings()) ?: BuildSettings()`. + Keep the `branch` parameter — the callers pass it and a later per-branch concern would need it back. +- `config/ConfigLoader.kt`: delete `mergeBranchDefaults`, the legacy arm of `resolveBuildSections` (it becomes just `mergeBuildDefaults`), the `"branches"` entry in the section loop of `stripPinned`, and `LEGACY_BRANCHES_WARNING`. + Add the rejection: a `branches` key in a file throws, scoped like the version check — the machine and project config abort the start (`checkVersion`'s call sites in `loadRaw`), a branch's committed config fails only that branch's builds (`withBranchLayer`). + Reuse `ConfigVersionException` or add a sibling; the message must name the file and say where the settings belong now. +- `watcher/Watcher.kt`: delete `enqueueDeprecatedAutoBuilds`, its call in `enqueueDueBranches`, and the `warnedDeprecatedAutoBuild` flag. +- `config/ConfigVersion.kt`: set `FORMAT_BROKE_IN` to this release's version and `FORMAT_BROKE_DESCRIPTION` to something like "the per-branch `branches` section was replaced by build definitions". + This is the first real use of that mechanism: a file declaring `gitTally.version.since` below this release is then refused with a message naming the change. +- `commands/InitCommand.kt`: nothing — the generated template has been `builds`-only since v0.9.19. Verify with `gittally init` in a scratch repo. + +## Tests + +`branches:`-shaped YAML in tests must move to `builds:` (as of 2026-08-29): + +- `config/ConfigLoaderTest.kt` — most occurrences; also delete the test "branches is honored while no build is defined and ignored as soon as one is" and replace it with one asserting the rejection. +- `build/BuildExecutorTest.kt` (the harness config and one inline config), `artifacts/BuildExecutorArtifactIntegrationTest.kt`, `artifacts/FileArtifactStoreTest.kt`, `build/DockerBuildRunnerTest.kt`, `build/DispatchingBuildRunnerTest.kt`, `server/UiControllerTest.kt`. +- `watcher/WatcherTest.kt` — the `autoBuildConfig` helper and the deprecated-schedule tests go; the `atTimes` tests stay. + +Do not blind-replace: `branches: [...]` inside a build definition is the selector and must be left alone. + +Add a test that a build whose definition was removed from the config still resolves to usable defaults — that path used to fall back to the branch settings. + +## Documentation + +- `docs/configuration.md`: delete the section "The legacy `branches` section"; drop the "only while nothing defines a build" qualifier from the branch-layer section. +- `AGENTS.md`: the invariant bullet starting "`builds` or the legacy `branches`, never both" becomes the rejection rule. +- `.claude/skills/architecture/SKILL.md`: `resolveBuildSections` no longer chooses between two sections. +- `docs/migration-from-legacy.md`: already maps to `builds.`; re-check it reads correctly without the legacy section existing. + +## Production + +The machine config on vm4006 was rewritten for v0.9.19 and still carries its legacy block as the rollback path to v0.9.18 (backup: `.git/gittally/.gittally.yml.20260829T085959Z.bak`). +Delete the block — everything below the comment marking it as legacy — before deploying this release, with a fresh timestamped backup. +The file keeps `git`, `server`, `builds.default`, and `builds.master`. +It is 600 by design; a shell redirect creates 644, so check the mode afterwards. + +Deploy as usual (`docs/plan/15-runtime-bundle-distribution.md`), only while `/api/builds/current` is `[]`. + +## Verification + +- `gittally config:print --full` on vm4006 before the restart: no `branches` in the output, `builds.default` and `builds.master` complete, `master` still inheriting `docker.enabled: true` and `network: host`. +- After the restart: no warnings about a branches section, the watcher polls without errors, and a branch build starts in `hsadmin-ng-build-env:latest`. +- Deliberately: point the running instance at a scratch repository whose config still has `branches:` and confirm the error names the file. + +## Rollback + +Keep `~/opt/gittally..bak` and the config backup. +A rollback needs both, because the previous version reads the machine config's `branches` block for its sandbox policy and this step deletes it. diff --git a/docs/plan/README.md b/docs/plan/README.md index d773608..61508d4 100644 --- a/docs/plan/README.md +++ b/docs/plan/README.md @@ -78,6 +78,10 @@ Added for the vm2176 → vm4006 migration (2026-08-10): - [x] `15-runtime-bundle-distribution.md` — self-contained runtime bundle (jlink JRE + jar) for hosts without a Java runtime - [x] `16-git-in-docker-builds.md` — read-only git metadata inside Docker build containers, with `.git/gittally/` masked +Added after v0.9.19 replaced the per-branch settings with build definitions (2026-08-29): + +- [ ] `18-remove-branches-section.md` — delete the legacy `branches` section and its `autoBuild` schedule; run around 2026-09-05, after the precondition check in the step file + Added for running GitTally on Hostsharing Managed Webspaces (2026-08-10): - [ ] `17-bwrap-build-runtime.md` — GitTally on a Managed Webspace: bubblewrap user-namespace build sandbox with a prepared rootfs (precondition check first — see the step file), plus web access under a domain via the platform's Apache proxy and Let's Encrypt @@ -89,3 +93,4 @@ Steps 11 and 12 are optional/deferrable; 10 only needs 04–06. Step 13 depends on 07, 11, and 12. Step 15 depends on 12 and 13 and revises the containerized-runtime sketch in `docs/bootstrapping.md` (ADR 0006 is written as part of the step; GraalVM native image was evaluated and rejected there). Step 17 depends on 11, 15, and 16, and starts with a hard precondition check on the target webspace (ADR 0007 is written as part of the step). +Step 18 depends on nothing in code but on the watched repository having migrated — its precondition check is a hard gate, not a formality.