Let init see a configuration under its previous name
`init --systemd` runs `init`, and its "already exists" check knew only the current name. On the one repository still carrying `.gittally.yml` it therefore wrote a fresh template `.werkator.yml` beside it — and since the current name wins, that repository would have built the template's `./gradlew test` instead of what its own configuration says. Found on vm4006, where the file was created in the watched working tree and removed again by hand. Both checks now ask `ConfigFiles`, so init decides existence by the same rule the loader uses to read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
39dac4b51b
commit
89b4254a95
@@ -141,6 +141,9 @@ The deployment to vm4006 found the gap the hard way: the move renames the *direc
|
|||||||
The instance came up with empty credentials and no host build definitions, without an error, which is the failure this PR exists to prevent.
|
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.
|
`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.
|
||||||
|
|
||||||
|
The same deployment found the second half of it: `init --systemd` runs `init`, whose "does it already exist" check knew only the current name, so it wrote a fresh template `.werkator.yml` beside the repository's committed `.gittally.yml` — and the current name wins, so the repository would have started building the template's `./gradlew test` instead of what it says it builds.
|
||||||
|
Both checks now ask `ConfigFiles`, which is the same question the loader asks.
|
||||||
|
|
||||||
**A one-time move for the state.**
|
**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.
|
[`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.
|
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.
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package de.hoennig.werkator.commands
|
package de.hoennig.werkator.commands
|
||||||
|
|
||||||
import de.hoennig.werkator.SecretFiles
|
import de.hoennig.werkator.SecretFiles
|
||||||
|
import de.hoennig.werkator.config.ConfigFiles
|
||||||
import de.hoennig.werkator.git.GitService
|
import de.hoennig.werkator.git.GitService
|
||||||
import org.springframework.beans.factory.ObjectProvider
|
import org.springframework.beans.factory.ObjectProvider
|
||||||
import org.springframework.boot.info.BuildProperties
|
import org.springframework.boot.info.BuildProperties
|
||||||
@@ -99,7 +100,8 @@ class InitCommand(
|
|||||||
detected: DetectedValues,
|
detected: DetectedValues,
|
||||||
normalizedWorkingDir: Path,
|
normalizedWorkingDir: Path,
|
||||||
) {
|
) {
|
||||||
val file = root.resolve(".git/werkator/.werkator.yml")
|
val existing = ConfigFiles.firstExisting(root, ConfigFiles.repoInstall)
|
||||||
|
val file = root.resolve(existing)
|
||||||
if (file.toFile().exists()) {
|
if (file.toFile().exists()) {
|
||||||
println("${file.toFile().relativeTo(normalizedWorkingDir.toFile())} already exists — not overwritten")
|
println("${file.toFile().relativeTo(normalizedWorkingDir.toFile())} already exists — not overwritten")
|
||||||
return
|
return
|
||||||
@@ -123,7 +125,11 @@ class InitCommand(
|
|||||||
detected: DetectedValues,
|
detected: DetectedValues,
|
||||||
normalizedWorkingDir: Path,
|
normalizedWorkingDir: Path,
|
||||||
) {
|
) {
|
||||||
val file = root.resolve(".werkator.yml")
|
// under either name: writing a second one beside a config under the previous
|
||||||
|
// name would shadow it, and a repository would silently start building
|
||||||
|
// something else than what it says it builds
|
||||||
|
val existing = ConfigFiles.firstExisting(root)
|
||||||
|
val file = root.resolve(existing)
|
||||||
if (file.toFile().exists()) {
|
if (file.toFile().exists()) {
|
||||||
println("${file.toFile().relativeTo(normalizedWorkingDir.toFile())} already exists — not overwritten")
|
println("${file.toFile().relativeTo(normalizedWorkingDir.toFile())} already exists — not overwritten")
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package de.hoennig.werkator.commands
|
|||||||
|
|
||||||
import de.hoennig.werkator.git.GitService
|
import de.hoennig.werkator.git.GitService
|
||||||
import io.kotest.core.spec.style.FunSpec
|
import io.kotest.core.spec.style.FunSpec
|
||||||
|
import io.kotest.matchers.booleans.shouldBeFalse
|
||||||
import io.kotest.matchers.file.shouldExist
|
import io.kotest.matchers.file.shouldExist
|
||||||
import io.kotest.matchers.shouldBe
|
import io.kotest.matchers.shouldBe
|
||||||
import io.kotest.matchers.string.shouldContain
|
import io.kotest.matchers.string.shouldContain
|
||||||
@@ -38,6 +39,25 @@ class InitCommandTest : FunSpec() {
|
|||||||
repoContent shouldContain "account: \"\"" // no user in https URL
|
repoContent shouldContain "account: \"\"" // no user in https URL
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("does not write a second config beside one under the previous name") {
|
||||||
|
val tempDir = Files.createTempDirectory("werkator-init-test")
|
||||||
|
initCommand.workingDir = tempDir
|
||||||
|
Files.createDirectories(tempDir.resolve(".git/gittally"))
|
||||||
|
tempDir.resolve(".gittally.yml").toFile().writeText("gitea:\n owner: kept\n")
|
||||||
|
tempDir.resolve(".git/gittally/.gittally.yml").toFile().writeText("git:\n account: kept\n")
|
||||||
|
|
||||||
|
every { gitService.getTopLevel(tempDir) } returns tempDir
|
||||||
|
every { gitService.getOriginUrl(tempDir) } returns "https://git.example.org/my-org/my-repo.git"
|
||||||
|
|
||||||
|
initCommand.run()
|
||||||
|
|
||||||
|
// a second file would shadow the first one, and the repository would
|
||||||
|
// silently build something else than what its configuration says
|
||||||
|
Files.exists(tempDir.resolve(".werkator.yml")).shouldBeFalse()
|
||||||
|
Files.exists(tempDir.resolve(".git/werkator/.werkator.yml")).shouldBeFalse()
|
||||||
|
tempDir.resolve(".gittally.yml").toFile().readText() shouldContain "owner: kept"
|
||||||
|
}
|
||||||
|
|
||||||
test("creates the secrets config and its directory readable only by the owner") {
|
test("creates the secrets config and its directory readable only by the owner") {
|
||||||
val tempDir = Files.createTempDirectory("werkator-init-test")
|
val tempDir = Files.createTempDirectory("werkator-init-test")
|
||||||
initCommand.workingDir = tempDir
|
initCommand.workingDir = tempDir
|
||||||
|
|||||||
Reference in New Issue
Block a user