From 39dac4b51bb6fd8fe7fb5c39b0922f1e3239b7a7 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Mon, 31 Aug 2026 09:10:16 +0200 Subject: [PATCH] Rename the machine configuration along with its directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the deployment to vm4006: the move renames the directory and leaves the file inside it alone, so the machine configuration ended up at `.git/werkator/.gittally.yml` — a pair of names the lookup did not expect, because it pairs directory and file name. The instance resolved empty credentials, no public URL and none of the host's build definitions, and said nothing about it. That is the exact failure this change exists to prevent, produced by the change itself. `StateDirMigration` now renames the configuration with the directory, unless one under the current name is already there. `ConfigFiles` carries `.git/werkator/.gittally.yml` as a third candidate as well, for a directory somebody moved by hand, where the migration never runs and so can rename nothing. Co-Authored-By: Claude Opus 5 --- ...-08-31-PR#1-rename-gittally-to-werkator.md | 21 +++++++++++++++++++ docs/werkator-migrationsplan.md | 5 ++++- .../de/hoennig/werkator/StateDirMigration.kt | 20 ++++++++++++++++++ .../de/hoennig/werkator/config/ConfigFiles.kt | 16 +++++++++++--- .../hoennig/werkator/StateDirMigrationTest.kt | 21 +++++++++++++++++++ .../werkator/config/ConfigLoaderTest.kt | 12 +++++++++++ 6 files changed, 91 insertions(+), 4 deletions(-) diff --git a/docs/prs/2026-08-31-PR#1-rename-gittally-to-werkator.md b/docs/prs/2026-08-31-PR#1-rename-gittally-to-werkator.md index b0276f7..405ba08 100644 --- a/docs/prs/2026-08-31-PR#1-rename-gittally-to-werkator.md +++ b/docs/prs/2026-08-31-PR#1-rename-gittally-to-werkator.md @@ -108,6 +108,22 @@ So that an operator who moved the directory by hand does not lose the live state - [StateDirMigrationTest: "leaves both alone when the current directory already exists"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt) - [StateDirMigrationTest: "does nothing where there is no pre-rename directory"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt) +#### Scenario#1.06: The machine configuration survives a directory that moved without it + +So that the move cannot produce a pair of names its own lookup does not expect. + +- **Given** a state directory that has moved to `.git/werkator/` + - **and** the machine configuration inside it still named `.gittally.yml` +- **When** the configuration is loaded +- **Then** its settings take effect + - **and** where the move did it, the file carries the current name afterwards + +##### Verified by + +- [StateDirMigrationTest: "renames the machine configuration inside the moved directory"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt) +- [StateDirMigrationTest: "never overwrites a machine configuration that already carries the current name"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt) +- [ConfigLoaderTest: "finds the machine config left under its old name in an already-moved directory"](../../src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt) + ## The Solution **One spelling rule.** @@ -120,6 +136,11 @@ This is what turns a rename into a decidable question instead of a matter of tas The current name wins and the old file is then ignored rather than merged — two files side by side are a half-done rename, not a layering. Error messages name the file that was actually read, so they never point at a file that does not exist. +**A move that finishes the job.** +The deployment to vm4006 found the gap the hard way: the move renames the *directory* and leaves the file inside it alone, so the machine configuration ended up at `.git/werkator/.gittally.yml` — a pair of names the lookup did not expect. +The instance came up with empty credentials and no host build definitions, without an error, which is the failure this PR exists to prevent. +`StateDirMigration` now renames the configuration along with the directory, unless one under the current name is already there, and `ConfigFiles` carries the intermediate path as a third candidate for a directory somebody moved by hand. + **A one-time move for the state.** [`StateDirMigration`](../../src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt) renames `.git/gittally` to `.git/werkator` from `CliRunner`, before any command resolves a path under it and therefore before the second Spring context of `server` exists. A fallback was rejected here: unlike a configuration, the state is written, so a fallback would have to decide on every write which of two directories wins, where a one-time move decides once and leaves a single path behind. diff --git a/docs/werkator-migrationsplan.md b/docs/werkator-migrationsplan.md index 8546bd3..1923624 100644 --- a/docs/werkator-migrationsplan.md +++ b/docs/werkator-migrationsplan.md @@ -170,7 +170,10 @@ tar xzf ~/werkator-runtime-linux-x64.tar.gz -C ~/opt # 4. artifact root: a move on the same filesystem, instant in both directions mv ~/.local/state/gittally ~/.local/state/werkator -# 5. let the state directory move itself, and read what it says +# 5. let the state directory move itself, and read what it says. +# Check the output: publicBaseUrl, git.account and the host's build +# definitions must be there. Empty ones mean the machine configuration +# was not found — nothing fails on its own in that case. cd ~/hs.hsadmin.ng ~/opt/werkator/jre/bin/java -jar ~/opt/werkator/lib/werkator.jar config:print --full diff --git a/src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt b/src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt index 434803a..a0dd206 100644 --- a/src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt +++ b/src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt @@ -1,5 +1,6 @@ package de.hoennig.werkator +import de.hoennig.werkator.config.ConfigFiles import org.slf4j.LoggerFactory import java.io.IOException import java.nio.file.Files @@ -44,10 +45,29 @@ object StateDirMigration { return } log.info("moved {} to {}: build history, control token and configuration kept", LEGACY_DIR, DIR) + renameMachineConfig(current) dropWorktrees(current) warnAboutUnits(current) } + /** + * Moving the directory leaves the file inside it under its old name, and the pair + * of names is then one the lookup does not expect. Renaming it here is what makes + * the move complete instead of half-done. + */ + private fun renameMachineConfig(stateDir: Path) { + val legacy = stateDir.resolve(ConfigFiles.LEGACY_COMMITTED) + val current = stateDir.resolve(ConfigFiles.COMMITTED) + if (!Files.isRegularFile(legacy) || Files.exists(current)) return + try { + Files.move(legacy, current) + } catch (e: IOException) { + log.warn("could not rename {} to {} in {}: {}", ConfigFiles.LEGACY_COMMITTED, ConfigFiles.COMMITTED, stateDir, e.message) + return + } + log.info("renamed the machine configuration to {}", ConfigFiles.COMMITTED) + } + /** * The moved worktrees point at their old path in both directions, so they are dropped * rather than repaired: the next build of a branch creates its worktree again, and diff --git a/src/main/kotlin/de/hoennig/werkator/config/ConfigFiles.kt b/src/main/kotlin/de/hoennig/werkator/config/ConfigFiles.kt index 13f7863..ad95d74 100644 --- a/src/main/kotlin/de/hoennig/werkator/config/ConfigFiles.kt +++ b/src/main/kotlin/de/hoennig/werkator/config/ConfigFiles.kt @@ -19,14 +19,24 @@ object ConfigFiles { /** The machine-specific configuration inside `.git`; secrets live here. */ const val REPO_INSTALL = ".git/werkator/$COMMITTED" - private const val LEGACY_COMMITTED = ".gittally.yml" + /** The name the committed configuration had before the rename. */ + const val LEGACY_COMMITTED = ".gittally.yml" + private const val LEGACY_REPO_INSTALL = ".git/gittally/$LEGACY_COMMITTED" + /** + * The state directory moves without touching the file inside it, so between the + * move and the rename the machine configuration sits under the old name in the + * new directory. `StateDirMigration` closes that gap where it moves the directory + * itself; this candidate covers a directory somebody moved by hand. + */ + private const val MOVED_REPO_INSTALL = ".git/werkator/$LEGACY_COMMITTED" + /** Both names of the committed configuration, current first. */ val committed = listOf(COMMITTED, LEGACY_COMMITTED) - /** Both paths of the machine-specific configuration, current first. */ - val repoInstall = listOf(REPO_INSTALL, LEGACY_REPO_INSTALL) + /** Every path the machine-specific configuration can sit at, current first. */ + val repoInstall = listOf(REPO_INSTALL, MOVED_REPO_INSTALL, LEGACY_REPO_INSTALL) /** * The first of [candidates] that exists under [dir], or the current name when none diff --git a/src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt b/src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt index 6a3ac9e..f5878e8 100644 --- a/src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt @@ -25,6 +25,27 @@ class StateDirMigrationTest : FunSpec() { dir.resolve(".git/werkator/build-results.json").toFile().readText() shouldBe "[]" } + test("renames the machine configuration inside the moved directory") { + val dir = repoWithLegacyState() + dir.resolve(".git/gittally/.gittally.yml").toFile().writeText("git:\n account: ci-user\n") + + StateDirMigration.migrateIfNeeded(dir) + + Files.exists(dir.resolve(".git/werkator/.gittally.yml")).shouldBeFalse() + dir.resolve(".git/werkator/.werkator.yml").toFile().readText() shouldBe "git:\n account: ci-user\n" + } + + test("never overwrites a machine configuration that already carries the current name") { + val dir = repoWithLegacyState() + dir.resolve(".git/gittally/.gittally.yml").toFile().writeText("git:\n account: old\n") + dir.resolve(".git/gittally/.werkator.yml").toFile().writeText("git:\n account: current\n") + + StateDirMigration.migrateIfNeeded(dir) + + dir.resolve(".git/werkator/.werkator.yml").toFile().readText() shouldBe "git:\n account: current\n" + Files.exists(dir.resolve(".git/werkator/.gittally.yml")).shouldBeTrue() + } + test("drops the moved worktrees, because they point at their old path") { val dir = repoWithLegacyState() Files.createDirectories(dir.resolve(".git/gittally/worktrees/main")) diff --git a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt index ce841d8..cd34a4f 100644 --- a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt @@ -74,6 +74,18 @@ class ConfigLoaderTest : FunSpec() { loader.load(dir).git.account shouldBe "ci-user" } + test("finds the machine config left under its old name in an already-moved directory") { + val dir = Files.createTempDirectory("werkator-test") + Files.createDirectories(dir.resolve(".git/werkator")) + dir.resolve(".git/werkator/.gittally.yml").toFile().writeText( + """ + git: + account: ci-user + """.trimIndent(), + ) + loader.load(dir).git.account shouldBe "ci-user" + } + test("the current name wins where both exist, so a half-done rename is not merged") { val dir = Files.createTempDirectory("werkator-test") dir.resolve(".werkator.yml").toFile().writeText("gitea:\n owner: current\n")