diff --git a/docs/configuration.md b/docs/configuration.md index b5bbf34..4f14403 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -42,8 +42,13 @@ builds: # Changing this value requires a restart. maxConcurrent: 1 -# Build artifact retention. +# Build artifact storage and retention. artifacts: + # Root directory for stored build artifacts. + # Empty means the platform default: $XDG_STATE_HOME (or ~/.local/state) plus /gittally/artifacts/, + # where is the sanitized absolute repository path. + # A leading ~/ expands to the home directory; a relative path is resolved against the repository. + rootDir: "" # number of builds to keep per branch retentionPerBranch: 3 diff --git a/docs/plan/05-artifact-store.md b/docs/plan/05-artifact-store.md index 3d34301..e0c982f 100644 --- a/docs/plan/05-artifact-store.md +++ b/docs/plan/05-artifact-store.md @@ -40,3 +40,32 @@ Update `GitTallyConfig`, `InitCommand` templates, and `docs/configuration.md` to - `./gradlew ktlintFormat` then `./gradlew build` is green. - Step 04's executor persists artifacts through the real store (integration test). + +## Execution Notes (done 2026-07-07) + +Implemented as `FileArtifactStore` in `de.hoennig.gittally.artifacts`; build green, 12 new tests +(`FileArtifactStoreTest`, `BuildExecutorArtifactIntegrationTest`, plus a `repoKey` case in `ArtifactKeysTest`). +Deviations and decisions: + +- The `ArtifactStore` interface stays in `de.hoennig.gittally.build` (moving it would make `build` depend on `artifacts`). + It gained `prune(keptResults)` and `artifactDir(artifactKey)`; the `NoOpArtifactStore` placeholder was removed. +- Interface gap from step 04 resolved by an additional parameter: `persist(build, stagingDir, workspace)`. + The store copies the configured `artifactDirs` out of the branch worktree itself, so the archived layout stays store knowledge. + `workspace` is null when a build crashed before its worktree was prepared; only the logs are stored then. +- Key naming was reused from step 04's `ArtifactKeys` (UTC `Instant`, not local time like legacy); only `repoKey` was added. + The repo key sanitizes the absolute normalized working directory, not `git rev-parse --show-toplevel` — consistent + with step 04's decision to resolve everything relative to the working directory. +- The atomic move does not move the staging directory directly: staging is a temp dir (usually under `/tmp`) and may + be on a different filesystem than the artifact root, where a move is not atomic. + Like legacy `persist_build_artifacts` (`$artifact_dir.tmp.$$`), everything is assembled in `.incoming-` next to + the target and then moved with `ATOMIC_MOVE`; on failure the incoming dir is deleted, so no partial dirs appear. + The staging directory is deleted after a successful move. +- The legacy archive layout was ported: `build/reports` archives as `reports/`, every other artifact dir below `reports/`. +- Concurrency (builds run concurrently since the step 04 amendment): persists share a read lock — their target dirs are + disjoint because artifact keys are unique per build — while `prune` takes the write lock, so it never deletes mid-persist. + `prune` also removes `.incoming-*` leftovers of crashed persists, deletes symlinks without following them, + and returns the removed keys. +- `artifactDir` rejects keys outside `[A-Za-z0-9._-]+` and anything resolving outside `/branches/` (path traversal). +- `artifacts.rootDir` supports a leading `~/` and resolves relative paths against the repository; + when empty, the default is `$XDG_STATE_HOME` (or `~/.local/state`) + `/gittally/artifacts/` as designed. +- The bean is wired in `ArtifactsConfiguration` with the working directory defaulting to `.`, mirroring `BuildConfiguration`. diff --git a/docs/plan/README.md b/docs/plan/README.md index c0c3f37..de17d24 100644 --- a/docs/plan/README.md +++ b/docs/plan/README.md @@ -49,7 +49,7 @@ Foundation: Core engine: - [x] `04-build-executor.md` — async build execution with logs, cancellation, status transitions -- [ ] `05-artifact-store.md` — artifact persistence, naming, retention +- [x] `05-artifact-store.md` — artifact persistence, naming, retention - [ ] `06-watcher.md` — branch watching, scheduling, auto-builds Server and UI: diff --git a/src/main/kotlin/de/hoennig/gittally/artifacts/ArtifactsConfiguration.kt b/src/main/kotlin/de/hoennig/gittally/artifacts/ArtifactsConfiguration.kt new file mode 100644 index 0000000..bcf9954 --- /dev/null +++ b/src/main/kotlin/de/hoennig/gittally/artifacts/ArtifactsConfiguration.kt @@ -0,0 +1,17 @@ +package de.hoennig.gittally.artifacts + +import de.hoennig.gittally.build.ArtifactStore +import de.hoennig.gittally.config.ConfigLoader +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +@Configuration +class ArtifactsConfiguration { + /** + * Store relative to the working directory, matching how `ConfigLoader` and the + * `BuildResultRepository` bean resolve their files. Nothing is touched until the + * first build persists, so the bean is safe outside a git repository. + */ + @Bean + fun artifactStore(configLoader: ConfigLoader): ArtifactStore = FileArtifactStore(configLoader) +} diff --git a/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt new file mode 100644 index 0000000..e6892e5 --- /dev/null +++ b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt @@ -0,0 +1,239 @@ +package de.hoennig.gittally.artifacts + +import de.hoennig.gittally.build.ArtifactKeys +import de.hoennig.gittally.build.ArtifactStore +import de.hoennig.gittally.build.BuildResult +import de.hoennig.gittally.config.BranchConfig +import de.hoennig.gittally.config.ConfigLoader +import org.slf4j.LoggerFactory +import java.io.IOException +import java.nio.file.FileVisitResult +import java.nio.file.Files +import java.nio.file.LinkOption +import java.nio.file.Path +import java.nio.file.Paths +import java.nio.file.SimpleFileVisitor +import java.nio.file.StandardCopyOption +import java.nio.file.attribute.BasicFileAttributes +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.read +import kotlin.concurrent.write + +/** + * Stores build artifacts on the filesystem under `/branches//`. + * The root is `artifacts.rootDir` from the config, or the platform default + * `XDG_STATE_HOME` (falling back to `~/.local/state`) plus `/gittally/artifacts/` when unset — + * deliberately not `/tmp` like legacy, where artifacts vanished on reboot. + * + * Each build is assembled in a temporary directory next to its target and moved + * into place atomically, so a crashed persist never leaves a partial artifact dir. + * Builds persist concurrently (their keys are unique) under a shared read lock; + * [prune] takes the write lock so it never deletes a directory mid-persist. + */ +class FileArtifactStore( + private val configLoader: ConfigLoader, + private val workingDir: Path = Paths.get("."), + private val env: (String) -> String? = System::getenv, +) : ArtifactStore { + private val log = LoggerFactory.getLogger(FileArtifactStore::class.java) + + private val lock = ReentrantReadWriteLock() + + override fun persist( + build: BuildResult, + stagingDir: Path, + workspace: Path?, + ) { + lock.read { + val branchesDir = Files.createDirectories(branchesDir()) + val target = branchesDir.resolve(build.artifactKey) + val incoming = branchesDir.resolve(INCOMING_PREFIX + build.artifactKey) + try { + deleteRecursively(incoming) + Files.createDirectories(incoming) + copyChildren(stagingDir, incoming) + copyArtifactDirs(build, workspace, incoming) + deleteRecursively(target) + Files.move(incoming, target, StandardCopyOption.ATOMIC_MOVE) + } catch (e: Exception) { + deleteRecursively(incoming) + throw e + } + deleteRecursively(stagingDir) + log.info("persisted build artifacts of {} to {}", build.artifactKey, target) + } + } + + override fun prune(keptResults: Collection): List { + lock.write { + val branchesDir = branchesDir() + if (!Files.isDirectory(branchesDir, LinkOption.NOFOLLOW_LINKS)) { + return emptyList() + } + val keptKeys = keptResults.map { it.artifactKey }.toSet() + val removed = mutableListOf() + Files.list(branchesDir).use { children -> + children.forEach { child -> + val key = child.fileName.toString() + if (key !in keptKeys) { + deleteRecursively(child) + removed += key + } + } + } + if (removed.isNotEmpty()) { + log.info("pruned {} artifact dir(s): {}", removed.size, removed) + } + return removed + } + } + + override fun artifactDir(artifactKey: String): Path? { + if (!ARTIFACT_KEY_PATTERN.matches(artifactKey)) { + return null + } + val branchesDir = branchesDir() + val dir = branchesDir.resolve(artifactKey).normalize() + if (dir.parent != branchesDir) { + return null + } + return dir.takeIf { Files.isDirectory(it, LinkOption.NOFOLLOW_LINKS) } + } + + private fun branchesDir(): Path = rootDir().resolve("branches") + + private fun rootDir(): Path { + val configured = + configLoader + .load(workingDir) + .artifacts.rootDir + .trim() + if (configured.isNotEmpty()) { + return workingDir.resolve(expandHome(configured)).toAbsolutePath().normalize() + } + val stateHome = + env("XDG_STATE_HOME")?.takeIf { it.isNotBlank() }?.let { Paths.get(it) } + ?: Paths.get(System.getProperty("user.home"), ".local", "state") + return stateHome + .resolve("gittally") + .resolve("artifacts") + .resolve(ArtifactKeys.repoKey(workingDir)) + .toAbsolutePath() + .normalize() + } + + private fun expandHome(path: String): Path = + if (path == "~" || path.startsWith("~/")) { + Paths.get(System.getProperty("user.home"), path.removePrefix("~")) + } else { + Paths.get(path) + } + + private fun copyArtifactDirs( + build: BuildResult, + workspace: Path?, + targetDir: Path, + ) { + if (workspace == null) { + log.warn("build {} has no workspace; storing only its logs", build.artifactKey) + return + } + for (artifactDir in branchConfig(build.branch).artifactDirs) { + if (artifactDir.isBlank()) { + continue + } + val source = workspace.resolve(artifactDir) + if (!Files.isDirectory(source)) { + log.info("build {} did not produce artifact directory {}; skipped", build.artifactKey, artifactDir) + continue + } + copyRecursively(source, targetDir.resolve(archivedPath(artifactDir))) + } + } + + /** Legacy `archived_artefact_dir_path`: `build/reports` archives as `reports/`, everything else below `reports/`. */ + private fun archivedPath(artifactDir: String): String = + if (artifactDir == "build/reports") { + "reports" + } else { + "reports/$artifactDir" + } + + private fun branchConfig(branch: String): BranchConfig { + val branches = configLoader.load(workingDir).branches + return branches[branch] ?: branches["default"] ?: BranchConfig() + } + + private fun copyChildren( + sourceDir: Path, + targetDir: Path, + ) { + if (Files.isDirectory(sourceDir)) { + copyRecursively(sourceDir, targetDir) + } + } + + private fun copyRecursively( + source: Path, + target: Path, + ) { + Files.walkFileTree( + source, + object : SimpleFileVisitor() { + override fun preVisitDirectory( + dir: Path, + attrs: BasicFileAttributes, + ): FileVisitResult { + Files.createDirectories(target.resolve(source.relativize(dir))) + return FileVisitResult.CONTINUE + } + + override fun visitFile( + file: Path, + attrs: BasicFileAttributes, + ): FileVisitResult { + Files.copy(file, target.resolve(source.relativize(file)), StandardCopyOption.REPLACE_EXISTING) + return FileVisitResult.CONTINUE + } + }, + ) + } + + /** Does not follow symlinks — a link is deleted, its target is never touched. */ + private fun deleteRecursively(path: Path) { + if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) { + return + } + Files.walkFileTree( + path, + object : SimpleFileVisitor() { + override fun visitFile( + file: Path, + attrs: BasicFileAttributes, + ): FileVisitResult { + Files.delete(file) + return FileVisitResult.CONTINUE + } + + override fun postVisitDirectory( + dir: Path, + exc: IOException?, + ): FileVisitResult { + if (exc != null) { + throw exc + } + Files.delete(dir) + return FileVisitResult.CONTINUE + } + }, + ) + } + + companion object { + /** Work-in-progress dir next to the target; unreferenced leftovers are cleaned up by [prune]. */ + private const val INCOMING_PREFIX = ".incoming-" + + /** Same character set the key sanitization produces; everything else is rejected on lookup. */ + private val ARTIFACT_KEY_PATTERN = Regex("[A-Za-z0-9._-]+") + } +} diff --git a/src/main/kotlin/de/hoennig/gittally/build/ArtifactKeys.kt b/src/main/kotlin/de/hoennig/gittally/build/ArtifactKeys.kt index 701fb1f..fdbd13e 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/ArtifactKeys.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/ArtifactKeys.kt @@ -1,5 +1,6 @@ package de.hoennig.gittally.build +import java.nio.file.Path import java.security.MessageDigest import java.time.Instant @@ -16,6 +17,9 @@ object ArtifactKeys { startedAt: Instant, ): String = "${branchKey(branch)}-${sanitize(startedAt.toString())}-${sha256Prefix("$branch\t$startedAt")}" + /** Legacy `repository_key`: the sanitized absolute repository path. */ + fun repoKey(repoDir: Path): String = sanitize(repoDir.toAbsolutePath().normalize().toString()) + private fun sanitize(value: String): String = value.replace(Regex("[^A-Za-z0-9._-]"), "_") private fun sha256Prefix(value: String): String = diff --git a/src/main/kotlin/de/hoennig/gittally/build/ArtifactStore.kt b/src/main/kotlin/de/hoennig/gittally/build/ArtifactStore.kt index 0ea1eaf..61b1203 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/ArtifactStore.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/ArtifactStore.kt @@ -1,29 +1,31 @@ package de.hoennig.gittally.build -import org.slf4j.LoggerFactory -import org.springframework.stereotype.Component import java.nio.file.Path /** - * Takes over the staging directory of a finished build. - * The real store (naming, retention, serving) arrives in step 05. + * Persists the artifacts of finished builds (logs plus configured report directories) + * and prunes them together with the result retention. + * Implemented by `de.hoennig.gittally.artifacts.FileArtifactStore`. */ interface ArtifactStore { + /** + * Takes over the staging directory of the finished [build] (log files) plus the + * configured `artifactDirs` from [workspace] and stores them under the build's + * artifact key. [workspace] is null when the build crashed before its workspace + * was prepared; only the logs are stored then. + */ fun persist( build: BuildResult, stagingDir: Path, + workspace: Path?, ) -} -/** Placeholder until step 05: logs and leaves the staging directory untouched. */ -@Component -class NoOpArtifactStore : ArtifactStore { - private val log = LoggerFactory.getLogger(NoOpArtifactStore::class.java) + /** + * Deletes all stored artifact directories whose keys are not in [keptResults]; + * call after repository retention pruning. Returns the removed artifact keys. + */ + fun prune(keptResults: Collection): List - override fun persist( - build: BuildResult, - stagingDir: Path, - ) { - log.info("artifact store not implemented yet; leaving build output of {} in {}", build.artifactKey, stagingDir) - } + /** The stored artifact directory for [artifactKey], or null if none exists. */ + fun artifactDir(artifactKey: String): Path? } diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt index 3771114..745e808 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt @@ -111,6 +111,7 @@ class BuildExecutor( private fun execute(build: ActiveBuild) { var slot: Semaphore? = null var finalStatus = BuildStatus.FAILED + var workspace: Path? = null try { slot = slotsFor(build.workingDir) slot.acquire() @@ -120,13 +121,14 @@ class BuildExecutor( } build.running = true transition(build, BuildStatus.RUNNING, duration = null) - val workspace = + val preparedWorkspace = workspaces.prepare( branch = build.runningBuild.branch, commit = build.runningBuild.commit, repoDir = build.workingDir, ) - val exitCode = runBuildCommands(build, workspace) + workspace = preparedWorkspace + val exitCode = runBuildCommands(build, preparedWorkspace) finalStatus = when { build.cancelled.get() -> BuildStatus.CANCELLED @@ -141,7 +143,7 @@ class BuildExecutor( val duration = Duration.between(build.runningBuild.startedAt, Instant.now()) val result = transition(build, finalStatus, duration) try { - artifactStore.persist(result, build.runningBuild.stagingDir) + artifactStore.persist(result, build.runningBuild.stagingDir, workspace) } catch (e: Exception) { log.warn("could not persist artifacts of {}: {}", result.artifactKey, e.message) } diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index 56d805f..4ba2c43 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -117,8 +117,10 @@ class InitCommand( # how many branches may build at the same time (at most one build per branch regardless) maxConcurrent: 1 - # Build artifact retention. + # Build artifact storage and retention. artifacts: + # root directory for stored artifacts; empty = XDG_STATE_HOME (or ~/.local/state) + /gittally/artifacts/ + rootDir: "" # number of builds to keep per branch retentionPerBranch: 3 diff --git a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt index 7ed1a1e..b02ba47 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt @@ -33,6 +33,11 @@ data class BuildsConfig( data class ArtifactsConfig( val retentionPerBranch: Int = 3, + /** + * Root directory for stored build artifacts; empty means the platform default + * `XDG_STATE_HOME` (or `~/.local/state`) + `/gittally/artifacts/`. + */ + val rootDir: String = "", ) data class WatcherConfig( diff --git a/src/test/kotlin/de/hoennig/gittally/artifacts/BuildExecutorArtifactIntegrationTest.kt b/src/test/kotlin/de/hoennig/gittally/artifacts/BuildExecutorArtifactIntegrationTest.kt new file mode 100644 index 0000000..ec571c4 --- /dev/null +++ b/src/test/kotlin/de/hoennig/gittally/artifacts/BuildExecutorArtifactIntegrationTest.kt @@ -0,0 +1,63 @@ +package de.hoennig.gittally.artifacts + +import de.hoennig.gittally.build.BranchWorkspaces +import de.hoennig.gittally.build.BuildExecutor +import de.hoennig.gittally.build.FileBuildResultRepository +import de.hoennig.gittally.build.ProcessBuildRunner +import de.hoennig.gittally.config.ConfigLoader +import de.hoennig.gittally.gitea.GiteaClient +import io.kotest.assertions.nondeterministic.eventually +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain +import io.mockk.mockk +import org.springframework.context.ApplicationEventPublisher +import java.nio.file.Files +import kotlin.time.Duration.Companion.seconds + +/** Step 05 acceptance: the step 04 executor persists artifacts through the real store. */ +class BuildExecutorArtifactIntegrationTest : FunSpec() { + init { + test("the executor persists a successful build's logs and reports through the real store") { + val workingDir = Files.createTempDirectory("gittally-artifact-integration-test") + val root = workingDir.resolve("artifact-root") + Files.writeString( + workingDir.resolve(".gittally.yml"), + """ + artifacts: + rootDir: "$root" + branches: + default: + buildCommand: "mkdir -p build/reports && echo report-content > build/reports/index.html && echo built-ok" + cleanCommand: "" + artifactDirs: + - build/reports + """.trimIndent(), + ) + val workspace = Files.createDirectories(workingDir.resolve("workspace")) + val store = FileArtifactStore(ConfigLoader(), workingDir) + val executor = + BuildExecutor( + repository = FileBuildResultRepository(workingDir.resolve("build-results.json")), + configLoader = ConfigLoader(), + giteaClient = mockk(relaxed = true), + buildRunner = ProcessBuildRunner(), + workspaces = BranchWorkspaces { _, _, _ -> workspace }, + artifactStore = store, + eventPublisher = ApplicationEventPublisher { }, + ) + + val build = executor.startBuild("main", "abc123", workingDir) + + lateinit var artifactDir: java.nio.file.Path + eventually(30.seconds) { + artifactDir = store.artifactDir(build.artifactKey).shouldNotBeNull() + } + Files.readString(artifactDir.resolve("build.stdout.log")) shouldContain "built-ok" + Files.readString(artifactDir.resolve("build.log")) shouldContain "built-ok" + Files.readString(artifactDir.resolve("reports/index.html")) shouldContain "report-content" + Files.exists(build.stagingDir) shouldBe false + } + } +} diff --git a/src/test/kotlin/de/hoennig/gittally/artifacts/FileArtifactStoreTest.kt b/src/test/kotlin/de/hoennig/gittally/artifacts/FileArtifactStoreTest.kt new file mode 100644 index 0000000..ff695e0 --- /dev/null +++ b/src/test/kotlin/de/hoennig/gittally/artifacts/FileArtifactStoreTest.kt @@ -0,0 +1,228 @@ +package de.hoennig.gittally.artifacts + +import de.hoennig.gittally.build.ArtifactKeys +import de.hoennig.gittally.build.BuildResult +import de.hoennig.gittally.build.BuildStatus +import de.hoennig.gittally.config.ConfigLoader +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldContainExactly +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.shouldBe +import java.io.IOException +import java.nio.file.Files +import java.nio.file.Path +import java.time.Duration +import java.time.Instant +import kotlin.concurrent.thread +import kotlin.io.path.listDirectoryEntries + +class FileArtifactStoreTest : FunSpec() { + private val startedAt = Instant.parse("2026-07-07T10:00:00Z") + + private class Harness { + val workingDir: Path = Files.createTempDirectory("gittally-store-test") + val root: Path = workingDir.resolve("artifact-root") + val store = FileArtifactStore(ConfigLoader(), workingDir) + + init { + Files.writeString( + workingDir.resolve(".gittally.yml"), + """ + artifacts: + rootDir: "$root" + branches: + default: + artifactDirs: + - build/reports + - build/doc + """.trimIndent(), + ) + } + + fun branchesDir(): Path = root.resolve("branches") + } + + private fun buildResult( + branch: String = "main", + startedAt: Instant = this.startedAt, + ) = BuildResult( + branch = branch, + commit = "abc123", + status = BuildStatus.SUCCESS, + startedAt = startedAt, + duration = Duration.ofSeconds(10), + artifactKey = ArtifactKeys.buildKey(branch, startedAt), + ) + + private fun stagingDir(): Path { + val dir = Files.createTempDirectory("gittally-staging-test") + Files.writeString(dir.resolve("build.stdout.log"), "out") + Files.writeString(dir.resolve("build.stderr.log"), "err") + Files.writeString(dir.resolve("build.log"), "live") + return dir + } + + private fun workspace( + h: Harness, + vararg artifactFiles: String, + ): Path { + val workspace = Files.createDirectories(h.workingDir.resolve("workspace")) + for (file in artifactFiles) { + val path = workspace.resolve(file) + Files.createDirectories(path.parent) + Files.writeString(path, "content of $file") + } + return workspace + } + + init { + test("persist stores logs and configured artifact directories under the artifact key") { + val h = Harness() + val build = buildResult() + val staging = stagingDir() + val workspace = workspace(h, "build/reports/tests/index.html", "build/doc/readme.txt") + + h.store.persist(build, staging, workspace) + + val artifactDir = h.branchesDir().resolve(build.artifactKey) + Files.readString(artifactDir.resolve("build.stdout.log")) shouldBe "out" + Files.readString(artifactDir.resolve("build.stderr.log")) shouldBe "err" + Files.readString(artifactDir.resolve("build.log")) shouldBe "live" + // legacy layout: build/reports archives as reports/, other dirs below reports/ + Files.exists(artifactDir.resolve("reports/tests/index.html")) shouldBe true + Files.exists(artifactDir.resolve("reports/build/doc/readme.txt")) shouldBe true + Files.exists(staging) shouldBe false + } + + test("a missing artifact directory is skipped") { + val h = Harness() + val build = buildResult() + val workspace = workspace(h, "build/doc/readme.txt") // no build/reports + + h.store.persist(build, stagingDir(), workspace) + + val artifactDir = h.branchesDir().resolve(build.artifactKey) + Files.exists(artifactDir.resolve("reports/build/doc/readme.txt")) shouldBe true + Files.exists(artifactDir.resolve("reports/tests")) shouldBe false + } + + test("persist without a workspace stores only the logs") { + val h = Harness() + val build = buildResult() + + h.store.persist(build, stagingDir(), workspace = null) + + val artifactDir = h.branchesDir().resolve(build.artifactKey) + Files.readString(artifactDir.resolve("build.log")) shouldBe "live" + Files.exists(artifactDir.resolve("reports")) shouldBe false + } + + test("a failing persist leaves no partial artifact directory") { + val h = Harness() + val build = buildResult() + val workspace = workspace(h, "build/reports/secret.txt") + Files.setPosixFilePermissions(workspace.resolve("build/reports/secret.txt"), emptySet()) + + shouldThrow { + h.store.persist(build, stagingDir(), workspace) + } + + h.branchesDir().listDirectoryEntries() shouldBe emptyList() + } + + test("the artifact root defaults to XDG_STATE_HOME plus the repo key") { + val workingDir = Files.createTempDirectory("gittally-store-test") + val stateHome = Files.createTempDirectory("gittally-state-home-test") + val store = + FileArtifactStore(ConfigLoader(), workingDir) { name -> + if (name == "XDG_STATE_HOME") stateHome.toString() else null + } + val build = buildResult() + + store.persist(build, stagingDir(), workspace = null) + + val expectedDir = + stateHome + .resolve("gittally/artifacts") + .resolve(ArtifactKeys.repoKey(workingDir)) + .resolve("branches") + .resolve(build.artifactKey) + Files.readString(expectedDir.resolve("build.log")) shouldBe "live" + } + + test("prune deletes exactly the unreferenced artifact directories") { + val h = Harness() + val kept = buildResult(branch = "main") + val alsoKept = buildResult(branch = "feature/x") + val dropped = buildResult(branch = "main", startedAt = startedAt.plusSeconds(60)) + listOf(kept, alsoKept, dropped).forEach { + Files.createDirectories(h.branchesDir().resolve(it.artifactKey)) + } + + val removed = h.store.prune(listOf(kept, alsoKept)) + + removed shouldContainExactly listOf(dropped.artifactKey) + Files.exists(h.branchesDir().resolve(kept.artifactKey)) shouldBe true + Files.exists(h.branchesDir().resolve(alsoKept.artifactKey)) shouldBe true + Files.exists(h.branchesDir().resolve(dropped.artifactKey)) shouldBe false + } + + test("prune removes leftover incoming directories of crashed persists") { + val h = Harness() + val leftover = h.branchesDir().resolve(".incoming-${buildResult().artifactKey}") + Files.createDirectories(leftover) + Files.writeString(leftover.resolve("build.log"), "partial") + + val removed = h.store.prune(emptyList()) + + removed shouldContainExactly listOf(leftover.fileName.toString()) + Files.exists(leftover) shouldBe false + } + + test("prune deletes a symlink without touching its target outside the root") { + val h = Harness() + val outside = Files.createTempDirectory("gittally-outside-test") + Files.writeString(outside.resolve("keep-me.txt"), "precious") + Files.createDirectories(h.branchesDir()) + val link = h.branchesDir().resolve("evil-link") + Files.createSymbolicLink(link, outside) + + val removed = h.store.prune(emptyList()) + + removed shouldContainExactly listOf("evil-link") + Files.exists(link) shouldBe false + Files.readString(outside.resolve("keep-me.txt")) shouldBe "precious" + } + + test("artifactDir returns the stored directory and rejects unknown or unsafe keys") { + val h = Harness() + val build = buildResult() + h.store.persist(build, stagingDir(), workspace = null) + + h.store.artifactDir(build.artifactKey).shouldNotBeNull() shouldBe + h.branchesDir().resolve(build.artifactKey) + h.store.artifactDir("unknown-key").shouldBeNull() + h.store.artifactDir("..").shouldBeNull() + h.store.artifactDir("../secrets").shouldBeNull() + } + + test("concurrent persists and a prune do not interfere") { + val h = Harness() + val builds = (1..8).map { buildResult(startedAt = startedAt.plusSeconds(it.toLong())) } + val stagings = builds.associateWith { stagingDir() } + + val persists = + builds.map { build -> + thread { h.store.persist(build, stagings.getValue(build), workspace = null) } + } + val prune = thread { h.store.prune(builds) } + (persists + prune).forEach { it.join() } + + builds.forEach { build -> + Files.readString(h.branchesDir().resolve(build.artifactKey).resolve("build.log")) shouldBe "live" + } + } + } +} diff --git a/src/test/kotlin/de/hoennig/gittally/build/ArtifactKeysTest.kt b/src/test/kotlin/de/hoennig/gittally/build/ArtifactKeysTest.kt index 82b4c1f..3163120 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/ArtifactKeysTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/ArtifactKeysTest.kt @@ -5,6 +5,7 @@ import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.string.shouldMatch +import java.nio.file.Paths import java.time.Instant class ArtifactKeysTest : FunSpec() { @@ -34,5 +35,9 @@ class ArtifactKeysTest : FunSpec() { key shouldContain ArtifactKeys.branchKey("main") key shouldContain "2026-07-07T10_00_00Z" } + + test("repoKey sanitizes the absolute repository path") { + ArtifactKeys.repoKey(Paths.get("/home/user/my repo")) shouldBe "_home_user_my_repo" + } } } diff --git a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt index 691037a..cecdc18 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt @@ -130,7 +130,7 @@ class BuildExecutorTest : FunSpec() { verify { h.giteaClient.publishStatus("abc123", BuildStatus.PENDING, any(), null, h.workingDir) } verify { h.giteaClient.publishStatus("abc123", BuildStatus.RUNNING, any(), null, h.workingDir) } verify { h.giteaClient.publishStatus("abc123", BuildStatus.SUCCESS, any(), null, h.workingDir) } - verify { h.artifactStore.persist(match { it.status == BuildStatus.SUCCESS }, build.stagingDir) } + verify { h.artifactStore.persist(match { it.status == BuildStatus.SUCCESS }, build.stagingDir, h.workingDir) } } test("build commands run in the workspace prepared for the branch") {