diff --git a/.claude/skills/architecture/SKILL.md b/.claude/skills/architecture/SKILL.md index e46f1f0..f698443 100644 --- a/.claude/skills/architecture/SKILL.md +++ b/.claude/skills/architecture/SKILL.md @@ -44,11 +44,11 @@ GitTally is configured by two YAML files, deep-merged by `ConfigLoader` (later w 1. `.gittally.yml` at the repo root — committed, shared team settings. 2. `.git/gittally/.gittally.yml` — not committed; machine-specific overrides and secrets (`git.account`, `git.token`). -On top of those comes the **branch layer**: the `.gittally.yml` committed on a branch, applied by `loadWithBranchLayer` (the watcher passes the content read via `git show`, `loadForWorktree` the file in the build worktree). A branch describes its own CI and wins over both layers — the whole `builds` section — so a configuration can be tried out on a branch without touching other branches' builds. `stripPinned` removes what is not a description of this branch's build: `git`, `server`, `gitea`, `executor`, `watcher`, and — inside every `builds` definition as well as every legacy `branches` entry — `requirePullRequest` and `docker.enabled`/`docker.network`. +On top of those comes the **branch layer**: the `.gittally.yml` committed on a branch, applied by `loadWithBranchLayer` (the watcher passes the content read via `git show`, `loadForWorktree` the file in the build worktree). A branch describes its own CI and wins over both layers — the whole `builds` section — so a configuration can be tried out on a branch without touching other branches' builds. `stripPinned` removes what is not a description of this branch's build: `git`, `server`, `gitea`, `executor`, `watcher`, and — inside every `builds` definition as well as every legacy `branches` entry — `requirePullRequest`, `statusContext`, and `docker.enabled`/`docker.network`. Each file is version-checked before merging (`gitTally.version.since`/`below`, `ConfigVersions.verdict`), so the message can name the file to fix: `since` is hard in both directions — too old a GitTally, or a file written before `ConfigVersions.FORMAT_BROKE_IN` and read after it — while `below` only warns. There is no format version (`apiVersion`) on purpose: only one configuration generation is supported, and the declared version exists to make the incompatibility nameable. -After merging, `resolveBuildSections` decides which section describes the builds: `builds` or the legacy `branches`, never both. With no real build definition (`builds.maxConcurrent` is not one, `dropNonDefinitionBuilds` already drops it) the legacy path runs and `branches.default` is merged into every other branch entry; otherwise `branches` is dropped with a warning and `mergeBuildDefaults` applies `builds.default` as the base of every other definition — its settings only, never its trigger keys (`SELECTOR_KEYS`). Deciding this on the merged map is deliberate: a build defined on a branch and unknown to the host still inherits the host's `builds.default`, sandbox policy included, which is what keeps the pinned keys effective for it. The result is bound to the `GitTallyConfig` data classes (`config/GitTallyConfig.kt`), which define the schema and all defaults; `GitTallyConfig.buildSettings(branch, build)` is the single answer to "what does this build run". +After merging, `resolveBuildSections` decides which section describes the builds: `builds` or the legacy `branches`, never both. With no real build definition (`builds.maxConcurrent` is not one, `dropNonDefinitionBuilds` already drops it) the legacy path runs and `branches.default` is merged into every other branch entry; otherwise `branches` is dropped with a warning and `mergeBuildDefaults` applies `builds.default` as the base of every other definition — its settings only, never its `trigger` block (`TRIGGER_KEYS`, a single key so that a selector added to `TriggerConfig` later is non-inheritable by construction). `checkTriggerBlocks` refuses a definition still writing `onPush`/`atTimes`/`branches`/`activeWithin` flat, per file and scoped like the version check. Deciding this on the merged map is deliberate: a build defined on a branch and unknown to the host still inherits the host's `builds.default`, sandbox policy included, which is what keeps the pinned keys effective for it. The result is bound to the `GitTallyConfig` data classes (`config/GitTallyConfig.kt`), which define the schema and all defaults; `GitTallyConfig.buildSettings(branch, build)` is the single answer to "what does this build run". Three places must stay in sync when config keys change: the `GitTallyConfig` data classes, the commented templates generated by `InitCommand`, and the reference in `docs/configuration.md`. diff --git a/AGENTS.md b/AGENTS.md index 8d01b03..527f1cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,8 @@ All production code lives under `de.hoennig.gittally`, with sub-packages `comman - When config keys change, three places must stay in sync: the `GitTallyConfig` data classes, the `InitCommand` templates, and `docs/configuration.md`. - Every config file may declare `gitTally.version.since`/`below` (the GitTally it is written for, never a format version — no API is involved). `since` is enforced in both directions, using `ConfigVersions.FORMAT_BROKE_IN` for "file predates a breaking change"; `below` only warns. A violation aborts the start for the machine and project config, but fails only that branch's builds for a branch config. - A branch describes its own CI: its committed `.gittally.yml` is the branch layer (`ConfigLoader.loadWithBranchLayer`, used by the watcher per origin branch and by `loadForWorktree` at build time) and takes precedence over `.git`/project — including the whole `builds` section, so a new configuration can be tried out on a branch without affecting other branches. Only the pinned set is stripped from that layer: secrets (`git`), host/repository sections (`server`, `gitea`, `executor`, `watcher`), the docker sandbox policy (`docker.enabled`, `docker.network`), and the trust gate (`requirePullRequest`). A branch must never reach credentials, disable its container, change its network, raise global concurrency, or bypass its own pull-request gate; a branch's definitions apply to that branch alone. -- A build definition carries the complete description of its build. `builds.default` is the base every other definition inherits its settings — never its trigger (`onPush`, `atTimes`, `branches`, `activeWithin`) — from, and the inheritance is applied after all layers are merged: that order is what makes a build a branch invents inherit the host's sandbox policy instead of the data-class default, so the pinning also holds for a build the host has never heard of. +- A build definition carries the complete description of its build, split in two: the `trigger` block (`onPush`, `atTimes`, `branches`, `activeWithin`) says when and for which branches it runs, everything else what it does. `builds.default` is the base every other definition inherits its settings — never its `trigger` — from. The split is structural so that a selector added to `TriggerConfig` later is non-inheritable by construction; writing a trigger key flat is refused, never ignored, because ignoring it leaves a build that silently stops running. A `!` prefix in `trigger.branches` excludes and always wins. +- The inheritance is applied after all layers are merged: that order is what makes a build a branch invents inherit the host's sandbox policy instead of the data-class default, so the pinning also holds for a build the host has never heard of. Pinned are `requirePullRequest`, `statusContext`, `docker.enabled`, and `docker.network`. - `builds` or the legacy `branches`, never both: `branches` is read only while the merged config defines no build at all (`builds.maxConcurrent` is not one), and ignored with a warning as soon as one exists. The section is deprecated and goes away once the repositories have migrated; then `ConfigVersions.FORMAT_BROKE_IN` gets set and a leftover `branches:` key must be rejected by name — the version check alone cannot catch a file that declares no version. - Web UI: server-rendered Thymeleaf plus one hand-written `static/gittally.js` — no SPA framework, no frontend build pipeline; every fetch has a timeout and an explicit error badge; `UiFormats` and `gittally.js` must produce identical display formats. - Git and Docker access shells out to the CLIs (`GitCommandRunner`, `docker`) — no JGit, no Docker SDK. diff --git a/build.gradle.kts b/build.gradle.kts index 725bedc..97a1e39 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -12,7 +12,7 @@ group = "de.hoennig" // bump at least the patch version for every deployment — and only then, not per commit — // so the UI footer (BuildProperties), --version and the release notes identify what is // actually running; a deployment bundles whatever was committed since the last one -version = "0.9.19" +version = "0.9.20" java { toolchain { diff --git a/docs/configuration.md b/docs/configuration.md index 202adf5..839cf8c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -162,11 +162,18 @@ executor: # Named build definitions (jobs, see notes below); every key names a build. builds: # "default" is the base every other definition inherits its settings from — never its - # trigger — and, with onPush, the build of every branch. Without this entry an implicit - # default build (onPush over all branches) applies; writing it replaces that implicit - # one, so a default without a trigger is a settings base and nothing else. + # trigger — and, with a trigger of its own, the build of every branch it selects. + # Without this entry an implicit default build (onPush over all branches) applies; + # writing it replaces that implicit one, so a default without a trigger is a settings + # base and nothing else. default: - onPush: true # trigger: build every new commit of the selected branches + # When this build runs and for which branches — the only part a definition does NOT + # inherit from builds.default; everything below the block does. + trigger: + onPush: true # build every new commit of the selected branches + # branches: ["*", "!master"] # names or globs; a "!" pattern excludes; default: all + # atTimes: ["01:00"] # daily UTC times HH:MM ("??:05" = every hour at :05) + # activeWithin: 24h # only branches with commits in the last 24h # run before each build cleanCommand: rm -rf build # shell command for each build @@ -181,6 +188,9 @@ builds: # origin (refs/pull/*/head — read via plain git, no API token needed; see notes below). # Pinned: a branch cannot set this in its own committed config. requirePullRequest: false + # Gitea check this build reports as; empty uses gitea.statusContext. Two builds of one + # commit under the same context overwrite each other. Pinned like requirePullRequest. + statusContext: "" # Optional Docker build runtime; when enabled, the clean and build commands # run inside a container instead of natively (see notes below). docker: @@ -197,16 +207,17 @@ builds: # additional environment variables set inside the build container env: {} - # Every further job inherits the settings above and adds its own trigger and selector. + # Every further job inherits the settings above and brings its own trigger. # The nightly rebuild runs the full check instead of the quick on-commit one and is # recorded separately as master@pitest: pitest: - onPush: false # trigger: build every new commit (default: false) - atTimes: ["01:00"] # trigger: daily UTC times HH:MM, "??:05" = hourly at :05 - branches: ["master", "release/*"] # selector: names or glob patterns (default: all) - activeWithin: 24h # selector: only branches with commits in the last 24h + trigger: + atTimes: ["01:00"] + branches: ["master", "release/*"] + activeWithin: 24h buildCommand: ./gradlew -PfullPitTest --console=plain --no-daemon piTestFull artifactDirs: [build/reports, build/libs] + statusContext: GitTally/pitest # Build artifact storage and retention. artifacts: @@ -279,16 +290,19 @@ To build pull-request branches only, gate the default build and give the permane ```yaml builds: default: - onPush: true + trigger: + onPush: true + branches: ["*", "!main"] requirePullRequest: true main: - onPush: true - branches: ["main"] + trigger: + onPush: true + branches: ["main"] requirePullRequest: false ``` Without that second definition, direct pushes and merges to `main` would never build — merge commits do not match any pull-request head. -Note that `main` is then selected by both definitions, so a push builds it twice; give the default build a `branches` selector that excludes it, or accept the second run. +The `!main` exclusion keeps the default build off it, so a push is built once instead of by both definitions. A plain git origin (no Gitea/GitHub) serves no `refs/pull/*/head` at all, so gated branches would never build there. For such origins, disable all gates globally with `watcher.pullRequestGate: false` — typically in the machine-specific `.git/gittally/.gittally.yml`, so the committed configuration keeps the gates for forge-backed environments. @@ -296,7 +310,9 @@ For such origins, disable all gates globally with `watcher.pullRequestGate: fals ### Notes on `builds` (build definitions) Every key of the `builds` section names a build definition (a job) over the branches — ADR 0007. -A build definition has triggers, a branch selector, and build-setting overrides. +A build definition has a `trigger` block — when it runs and for which branches — and the settings that say what it does. +The split is structural because `trigger` is the one part never inherited from `builds.default`. +Writing any of its keys outside the block is refused with a message naming the definition: ignoring them would leave the build without a trigger, and a job that silently stops running is worse than a configuration that refuses to load. Triggers: `onPush: true` builds every new commit of the selected branches; `atTimes: ["HH:MM", …]` rebuilds their heads once per day and slot (UTC). A slot may also be written as `??:MM` — that minute of every hour, expanded to its 24 slots, so the build runs hourly. @@ -304,14 +320,16 @@ Only the latest due slot of a day triggers, so slots missed while the server was A definition may have both; one with neither never triggers automatically — which is how `builds.default` is written when it is meant as a settings base only. GitTally logs a warning once when no definition has a trigger at all, because such an instance never builds anything on its own. -Selector: `branches` lists branch names or glob patterns (`*` matches any characters, also across `/`); empty selects all origin branches. +Selector: `trigger.branches` lists branch names or glob patterns (`*` matches any characters, also across `/`); empty selects all origin branches. +A pattern prefixed with `!` excludes instead, and an exclusion always wins regardless of order — `["*", "!master"]` is every branch but master. +That is how a branch gets a build of its own without being built by the default one as well. `activeWithin` (e.g. `24h`) additionally keeps only branches whose origin head commit is younger than the duration — useful to run a nightly deep check over all recently active branches. Both parts combine as an intersection. -Settings: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, `requirePullRequest`, and `docker` with all its keys. +Settings: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, `requirePullRequest`, `statusContext`, and `docker` with all its keys. A definition carries the complete description of its build; unset keys fall back to `builds.default` and then to GitTally's own defaults. -`requirePullRequest`, `docker.enabled`, and `docker.network` are pinned: they are read from the repo install/project config even when a branch sets them in its own committed config, see [the branch layer](#the-branch-layer-a-branch-describes-its-own-ci). -Inheritance from `builds.default` covers the settings only — a trigger and a selector say when and where *this* build runs, so `onPush`, `atTimes`, `branches`, and `activeWithin` are never inherited. +`requirePullRequest`, `statusContext`, `docker.enabled`, and `docker.network` are pinned: they are read from the repo install/project config even when a branch sets them in its own committed config, see [the branch layer](#the-branch-layer-a-branch-describes-its-own-ci). +Inheritance from `builds.default` covers the settings only — the `trigger` block says when and where *this* build runs and is never inherited. Definitions are part of the branch layer: a branch may add its own and override those from the project config, for its own builds only. Because the inheritance is applied after all layers are merged, a build a branch invents still inherits the host's `builds.default` — its sandbox policy included, which is what keeps the pinning effective for a build the host has never heard of. @@ -320,7 +338,8 @@ The `default` build records under the plain branch name; every other build recor The URL key is the sanitized pool name — `master@pitest` is served as `/branches/master_pitest/…`. The pools live as long as the underlying branch exists on origin. Restart, `gittally retry`, and the startup recovery re-run a build under its recorded definition, resolving the settings from the current configuration — the job definition is the source of truth, not the historical run. -The builds still run in their branch's worktree, one build per branch at a time, and the Gitea commit status is reported per commit in the shared status context (the last build of a commit wins there). +The builds still run in their branch's worktree, one build per branch at a time. +The Gitea commit status is reported per commit under `gitea.statusContext`, so two builds of the same commit overwrite each other's check — give the second one its own `statusContext` (`GitTally/quick`, say), or keep them apart with an exclusion pattern. The concurrency limit that used to live in this section moved to `executor.maxConcurrent` without an alias. A leftover `builds.maxConcurrent` key (or any other scalar where a definition belongs) is ignored with a warning, not a startup failure — a committed config cannot always be changed right away. diff --git a/docs/plan/18-remove-branches-section.md b/docs/plan/18-remove-branches-section.md index 6837e4f..84a7ab2 100644 --- a/docs/plan/18-remove-branches-section.md +++ b/docs/plan/18-remove-branches-section.md @@ -1,4 +1,4 @@ -# Step 18: Remove the legacy `branches` section, group the trigger +# 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. @@ -7,8 +7,7 @@ Scheduled for roughly **2026-09-05**, one week after v0.9.19 (2026-08-29), and o 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. -It carries a second, unrelated break in the same release, deliberately: the trigger and selector keys of a definition move into a `trigger` block. -Both changes force the same repositories to migrate the same files, so they cost one migration, one `FORMAT_BROKE_IN`, and one deploy together — and two of each apart. +The `trigger` block, originally planned here, shipped earlier — see the section below for what that leaves. ## Precondition Check (run first, do not skip) @@ -22,8 +21,6 @@ ssh tallyman@vm4006.hostsharing.net 'cd ~/hs.hsadmin.ng && grep -n "^branches:" Expected output: nothing at all. -The same holds for the second change: no configuration may still carry a definition with a flat `onPush`, `atTimes`, `branches`, or `activeWithin`. Since the two migrations touch the same files, do them in one commit per repository. - 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. @@ -42,31 +39,14 @@ Note that `branches:` also exists as the *selector* key **inside** a build defin - `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`: the generated template is `builds`-only since v0.9.19, but its `default` entry and the commented example job need the `trigger` block. Verify with `gittally init` in a scratch repo. + Note that it only bites files that declare a version — which is why the rejection by name above exists next to it, not instead of it. +- `commands/InitCommand.kt`: nothing — the generated template has been `builds`-only with a `trigger` block since v0.9.20. Verify with `gittally init` in a scratch repo. -## The `trigger` block +## Already done: the `trigger` block -`onPush`, `atTimes`, `branches`, and `activeWithin` move from the definition into a nested `trigger`: - -```yaml -builds: - default: - trigger: - onPush: true - branches: ["*", "!master"] - buildCommand: ./gradlew check -``` - -The point is that the inheritance rule becomes structural instead of a remembered list: everything except `trigger` is inherited from `builds.default`. -Today `ConfigLoader.SELECTOR_KEYS` enumerates the four keys, and whoever adds a fifth selector without touching that list makes it silently inheritable — a job firing on branches that are none of its business, noticed only much later. -After the change `mergeBuildDefaults` subtracts the single key `trigger`, and a new selector is automatically right. - -- `config/BuildDefinition.kt`: a nested `TriggerConfig(onPush, atTimes, branches, activeWithin)`; `selects`/`selectsByName`/`maxAge` read from it. -- `config/ConfigLoader.kt`: `SELECTOR_KEYS` becomes the single key `trigger`. -- Reject a definition that still carries any of the four keys flat, by name and with the same scoping as the `branches` rejection. `FORMAT_BROKE_IN` does not cover this: the hs.hsadmin.ng configs declare no version, and a flat `onPush` nobody reads any more means the branch stops building, wordlessly. -- `branches` inside `trigger` is the selector and unrelated to the removed top-level section — the two named the same thing, which is part of why the block is clearer. - -`trigger` also groups `branches`/`activeWithin`, which are selectors rather than triggers in the strict sense. That is the established shape (GitHub Actions writes `on: push: branches: [...]`) and was chosen over `when`. +`onPush`, `atTimes`, `branches`, and `activeWithin` moved into a nested `trigger` block in the release that made a definition self-contained, and writing them flat is refused since then. +Nothing is left to do for it here beyond not reintroducing the flat shape in examples. +`ConfigLoader.TRIGGER_KEYS` is already the single key the inheritance subtracts, so the removal below does not touch it. ## Tests @@ -85,8 +65,7 @@ Add a test that a build whose definition was removed from the config still resol - `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, and move the trigger keys of the mapping table into `trigger`. -- Every YAML example in `docs/configuration.md`, `docs/deployment.md`, and the ADRs that shows a build definition needs the `trigger` block. +- `docs/migration-from-legacy.md`: already maps to `builds.`; re-check it reads correctly without the legacy section existing. ## Production @@ -105,7 +84,7 @@ Deploy as usual (`docs/plan/15-runtime-bundle-distribution.md`), only while `/ap - `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 at one with a flat `onPush:`, and confirm both errors name the file and the way out. +- Deliberately: point the running instance at a scratch repository whose config still has `branches:` and confirm the error names the file and the way out. ## Rollback diff --git a/docs/plan/README.md b/docs/plan/README.md index ef5f613..61508d4 100644 --- a/docs/plan/README.md +++ b/docs/plan/README.md @@ -80,7 +80,7 @@ Added for the vm2176 → vm4006 migration (2026-08-10): 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, and group a definition's trigger and selector keys in a `trigger` block; run around 2026-09-05, after the precondition check in the step file +- [ ] `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): diff --git a/src/main/kotlin/de/hoennig/gittally/GitTallyApplication.kt b/src/main/kotlin/de/hoennig/gittally/GitTallyApplication.kt index 6f4937c..fe29e40 100644 --- a/src/main/kotlin/de/hoennig/gittally/GitTallyApplication.kt +++ b/src/main/kotlin/de/hoennig/gittally/GitTallyApplication.kt @@ -1,6 +1,6 @@ package de.hoennig.gittally -import de.hoennig.gittally.config.ConfigVersionException +import de.hoennig.gittally.config.ConfigException import org.springframework.boot.CommandLineRunner import org.springframework.boot.ExitCodeGenerator import org.springframework.boot.SpringApplication @@ -31,7 +31,7 @@ class CliRunner( .setExecutionExceptionHandler { exception, commandLine, _ -> // a config GitTally must not read is a stated fact, not a crash: the message // names the file, the versions, and the way out — a stack trace would bury it - if (exception is ConfigVersionException) { + if (exception is ConfigException) { commandLine.err.println("Error: ${exception.message}") CONFIG_ERROR_EXIT_CODE } else { diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt index de4588a..f95df36 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt @@ -396,12 +396,27 @@ class BuildExecutor( description = description(status, duration), targetUrl = null, workingDir = build.workingDir, + // from the primary config, not the worktree: statusContext is pinned, so a + // branch cannot report under a check name it was not given + context = statusContextOf(build), ) } catch (e: Exception) { log.warn("could not publish Gitea status {} for {}: {}", status, build.runningBuild.commit, e.message) } } + /** The build's own Gitea status context, empty when it uses the repository-wide one. */ + private fun statusContextOf(build: ActiveBuild): String = + try { + configLoader + .load(build.workingDir) + .buildSettings(build.runningBuild.branch, build.runningBuild.build) + .statusContext + } catch (e: Exception) { + log.warn("could not resolve the status context of {}: {}", build.runningBuild.branch, e.message) + "" + } + private fun description( status: BuildStatus, duration: Duration?, diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index b8fe56d..5a0afd8 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -182,7 +182,13 @@ class InitCommand( # they apply to that branch alone, so a new job can be tried out on one branch. builds: default: - onPush: true # build every new commit of the selected branches + # When this build runs and for which branches. The only part a build does + # NOT inherit from the default — everything below it does. + trigger: + onPush: true # build every new commit of the selected branches + # branches: ["*", "!master"] # names or globs; "!" excludes; default: all + # atTimes: ["01:00"] # daily UTC times HH:MM ("??:05" = every hour at :05) + # activeWithin: 24h # only branches with recent commits # run before each build cleanCommand: rm -rf build # shell command for each build @@ -203,12 +209,16 @@ class InitCommand( context: "." # Docker build context used with dockerfile network: "" # Docker network mode for the build container; empty = Docker default (pinned) env: {} # additional environment variables set inside the build container - # Further jobs inherit those settings and add their own trigger and selector: + # Gitea check this build reports as; empty uses gitea.statusContext. + # Two builds of one commit under the same context overwrite each other. + statusContext: "" + # Further jobs inherit those settings and only bring their own trigger: # pitest: - # atTimes: ["01:00"] # daily UTC times HH:MM ("??:05" = every hour at :05) - # branches: ["master"] # names or glob patterns; default: all branches - # activeWithin: 24h # only branches with recent commits + # trigger: + # atTimes: ["01:00"] + # branches: ["master"] # buildCommand: ./gradlew pitestFull + # statusContext: GitTally/pitest # Build artifact storage and retention. artifacts: diff --git a/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt b/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt index 0bc1180..784ea1c 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt @@ -8,16 +8,82 @@ import java.time.Instant * top-level `builds` section next to the reserved execution key `maxConcurrent` * (split apart by [ConfigLoader]). * - * A definition carries the complete description of one build. The `default` entry is - * additionally the base every other definition inherits its settings from — but never - * its trigger, see [SELECTOR_KEYS][ConfigLoader]. Unset values fall through to the - * [BranchConfig] defaults. - * - * The `default` build records its results under the plain branch name; every other - * build records under `@` with its own history, retention pool, and - * permanent latest-green link. + * A definition describes one build completely, in two halves: [trigger] says when it + * runs and for which branches, everything else says what it does. The `default` entry + * is additionally the base every other definition inherits from — its settings, never + * its [trigger]. That is why the halves are separated structurally instead of by a list + * of key names: a selector added to [TriggerConfig] later is non-inheritable by + * construction, not because someone remembered to extend a list. */ data class BuildDefinition( + /** When and for which branches this build runs; never inherited from `builds.default`. */ + val trigger: TriggerConfig = TriggerConfig(), + /** Overrides the build command; null inherits it. */ + val buildCommand: String? = null, + /** Overrides the clean command; null inherits it. */ + val cleanCommand: String? = null, + /** Overrides the artifact directories; null inherits them. */ + val artifactDirs: List? = null, + /** Overrides the stdout log file name; null inherits it. */ + val stdoutLog: String? = null, + /** Overrides the stderr log file name; null inherits it. */ + val stderrLog: String? = null, + /** + * The watcher builds a selected branch only while its head commit matches a + * pull-request head; null inherits. Pinned — a branch's own committed config can + * never set it, or it would bypass its own gate. + */ + val requirePullRequest: Boolean? = null, + /** + * Gitea commit status context of this build; null uses `gitea.statusContext`. + * Two builds of the same commit under the same context overwrite each other's + * result, so a second build over a branch needs its own context to be readable. + */ + val statusContext: String? = null, + /** Overrides of the docker settings; null inherits them. */ + val docker: DockerOverrides? = null, +) { + /** The settings this build runs with: [branchConfig] with this definition applied; unset values fall through. */ + fun applyTo(branchConfig: BranchConfig): BranchConfig = + branchConfig.copy( + buildCommand = buildCommand ?: branchConfig.buildCommand, + cleanCommand = cleanCommand ?: branchConfig.cleanCommand, + artifactDirs = artifactDirs ?: branchConfig.artifactDirs, + stdoutLog = stdoutLog ?: branchConfig.stdoutLog, + stderrLog = stderrLog ?: branchConfig.stderrLog, + requirePullRequest = requirePullRequest ?: branchConfig.requirePullRequest, + statusContext = statusContext ?: branchConfig.statusContext, + docker = + branchConfig.docker.copy( + enabled = docker?.enabled ?: branchConfig.docker.enabled, + image = docker?.image ?: branchConfig.docker.image, + dockerfile = docker?.dockerfile ?: branchConfig.docker.dockerfile, + context = docker?.context ?: branchConfig.docker.context, + network = docker?.network ?: branchConfig.docker.network, + env = docker?.env ?: branchConfig.docker.env, + ), + ) + + companion object { + /** Name of the implicit build that preserves the pre-ADR-0007 behavior: `onPush` over all branches. */ + const val DEFAULT = "default" + + /** The result-pool name of [build] on [branch]: the plain branch for the default build. */ + fun poolName( + branch: String, + build: String, + ): String = if (build == DEFAULT) branch else "$branch@$build" + } +} + +/** + * When a build runs and for which branches — the `trigger` block of a build definition, + * and the one part of it that is never inherited from `builds.default`. + * + * A definition with neither [onPush] nor [atTimes] never triggers automatically; that is + * how `builds.default` is written when it is meant as a settings base only. + */ +data class TriggerConfig( /** Build every new commit of the selected branches. */ val onPush: Boolean = false, /** @@ -28,7 +94,8 @@ data class BuildDefinition( val atTimes: List = emptyList(), /** * Branch names or glob patterns (`*` matches any characters, also across `/`); - * empty selects all origin branches. + * empty selects all origin branches. A pattern prefixed with `!` excludes instead, + * and an exclusion always wins — `["*", "!master"]` is every branch but master. */ val branches: List = emptyList(), /** @@ -36,27 +103,15 @@ data class BuildDefinition( * empty applies no age filter. Combines with [branches] as an intersection. */ val activeWithin: String = "", - /** Overrides the branch's build command; null inherits it. */ - val buildCommand: String? = null, - /** Overrides the branch's clean command; null inherits it. */ - val cleanCommand: String? = null, - /** Overrides the branch's artifact directories; null inherits them. */ - val artifactDirs: List? = null, - /** Overrides the branch's stdout log file name; null inherits it. */ - val stdoutLog: String? = null, - /** Overrides the branch's stderr log file name; null inherits it. */ - val stderrLog: String? = null, - /** - * The watcher builds a selected branch only while its head commit matches a - * pull-request head; null inherits. Pinned — a branch's own committed config can - * never set it, or it would bypass its own gate. - */ - val requirePullRequest: Boolean? = null, - /** Overrides of the branch's docker settings; null inherits them. */ - val docker: DockerOverrides? = null, ) { - /** True when [branch] matches the [branches] patterns (or none are configured). */ - fun selectsByName(branch: String): Boolean = branches.isEmpty() || branches.any { globToRegex(it).matches(branch) } + /** True when [branch] matches the [branches] patterns (or none are configured) and none excludes it. */ + fun selectsByName(branch: String): Boolean { + val (excluding, including) = branches.partition { it.startsWith(EXCLUDE_PREFIX) } + if (excluding.any { globToRegex(it.removePrefix(EXCLUDE_PREFIX)).matches(branch) }) { + return false + } + return including.isEmpty() || including.any { globToRegex(it).matches(branch) } + } /** * True when [branch] passes both selector parts; [headCommittedAt] is the branch @@ -80,35 +135,9 @@ data class BuildDefinition( fun maxAge(): Duration = DurationParser.parse(activeWithin) - /** The branch settings with this build's overrides applied; unset values fall through. */ - fun applyTo(branchConfig: BranchConfig): BranchConfig = - branchConfig.copy( - buildCommand = buildCommand ?: branchConfig.buildCommand, - cleanCommand = cleanCommand ?: branchConfig.cleanCommand, - artifactDirs = artifactDirs ?: branchConfig.artifactDirs, - stdoutLog = stdoutLog ?: branchConfig.stdoutLog, - stderrLog = stderrLog ?: branchConfig.stderrLog, - requirePullRequest = requirePullRequest ?: branchConfig.requirePullRequest, - docker = - branchConfig.docker.copy( - enabled = docker?.enabled ?: branchConfig.docker.enabled, - image = docker?.image ?: branchConfig.docker.image, - dockerfile = docker?.dockerfile ?: branchConfig.docker.dockerfile, - context = docker?.context ?: branchConfig.docker.context, - network = docker?.network ?: branchConfig.docker.network, - env = docker?.env ?: branchConfig.docker.env, - ), - ) - companion object { - /** Name of the implicit build that preserves the pre-ADR-0007 behavior: `onPush` over all branches. */ - const val DEFAULT = "default" - - /** The result-pool name of [build] on [branch]: the plain branch for the default build. */ - fun poolName( - branch: String, - build: String, - ): String = if (build == DEFAULT) branch else "$branch@$build" + /** Marks a [branches] pattern as excluding. */ + const val EXCLUDE_PREFIX = "!" private fun globToRegex(pattern: String): Regex = Regex( diff --git a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt index b891635..94c06a4 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt @@ -79,6 +79,7 @@ class ConfigLoader( // scoped to this branch: an incompatible branch config fails its own builds and // must never stop the server or hold up the branches that are fine checkVersion(branchLayer, "the committed .gittally.yml of this branch", BRANCH_HINT) + checkTriggerBlocks(branchLayer, "the committed .gittally.yml of this branch", BRANCH_HINT) return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer))) } @@ -200,8 +201,34 @@ class ConfigLoader( } private fun isTriggered(definition: Any?): Boolean { - val entry = definition as? Map<*, *> ?: return false - return entry["onPush"] == true || (entry["atTimes"] as? List<*>)?.isNotEmpty() == true + val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return false + return trigger["onPush"] == true || (trigger["atTimes"] as? List<*>)?.isNotEmpty() == true + } + + /** + * Refuses a definition that still writes its trigger and selector keys flat instead of + * inside `trigger`. Silently ignoring them would leave a build with no trigger at all — + * a branch that stops building without saying so, which is worse than not starting. + * Scoped like [checkVersion]: per file, so the message names the one to fix. + */ + private fun checkTriggerBlocks( + raw: Map, + source: String, + hint: String, + ) { + val builds = raw["builds"] as? Map<*, *> ?: return + val offenders = + builds.entries.mapNotNull { (name, value) -> + val flat = (value as? Map<*, *>)?.keys?.filter { it in FLAT_TRIGGER_KEYS } ?: return@mapNotNull null + flat.takeIf { it.isNotEmpty() }?.let { "builds.$name: ${it.joinToString(", ")}" } + } + if (offenders.isEmpty()) { + return + } + throw ConfigFormatException( + "$source declares a build trigger outside its trigger block (${offenders.joinToString("; ")}). " + + "Move these keys into a `trigger:` block inside the definition. $hint", + ) } /** @@ -213,7 +240,7 @@ class ConfigLoader( @Suppress("UNCHECKED_CAST") private fun mergeBuildDefaults(raw: Map): Map { val builds = raw["builds"] as? Map ?: return raw - val base = (builds[BuildDefinition.DEFAULT] as? Map)?.minus(SELECTOR_KEYS) ?: return raw + val base = (builds[BuildDefinition.DEFAULT] as? Map)?.minus(TRIGGER_KEYS) ?: return raw if (base.isEmpty()) { return raw } @@ -245,6 +272,8 @@ class ConfigLoader( // per file, so the message names the file to fix — the merged map has no provenance checkVersion(project, ".gittally.yml", ROLLBACK_HINT) checkVersion(repoInstall, ".git/gittally/.gittally.yml", ROLLBACK_HINT) + checkTriggerBlocks(project, ".gittally.yml", ROLLBACK_HINT) + checkTriggerBlocks(repoInstall, ".git/gittally/.gittally.yml", ROLLBACK_HINT) return deepMerge(project, repoInstall) } @@ -348,15 +377,25 @@ class ConfigLoader( /** * Settings keys a branch must never override, in a build definition as well as in * a legacy branch entry: the trust gate that decides whether the watcher builds - * this branch at all. + * this branch at all, and the Gitea check this build reports as — a branch that + * could choose its own context could take over the check a branch protection + * rule depends on. */ - private val PINNED_SETTING_KEYS = setOf("requirePullRequest") + private val PINNED_SETTING_KEYS = setOf("requirePullRequest", "statusContext") /** `docker` keys a branch must never override: the sandbox policy. */ private val PINNED_DOCKER_KEYS = setOf("enabled", "network") - /** Keys of a build definition that say *when* it runs; never inherited from `builds.default`. */ - private val SELECTOR_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin") + /** + * The one key of a build definition that says *when* and *for which branches* it + * runs; never inherited from `builds.default`. A single key on purpose: a selector + * added inside it is non-inheritable by construction, where a list of key names + * would have to be remembered. + */ + private val TRIGGER_KEYS = setOf("trigger") + + /** The keys that moved into [TRIGGER_KEYS]; still writing them flat is refused, not ignored. */ + private val FLAT_TRIGGER_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin") private const val LEGACY_BRANCHES_WARNING = "legacy-branches-ignored" diff --git a/src/main/kotlin/de/hoennig/gittally/config/ConfigVersion.kt b/src/main/kotlin/de/hoennig/gittally/config/ConfigVersion.kt index 8dc6592..0b87ff5 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/ConfigVersion.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/ConfigVersion.kt @@ -44,10 +44,23 @@ sealed interface VersionVerdict { } /** A configuration file this GitTally must not read; carries the file's name in its message. */ -class ConfigVersionException( +open class ConfigException( message: String, ) : RuntimeException(message) +/** The file declares a GitTally that cannot read it, see [ConfigVersions]. */ +class ConfigVersionException( + message: String, +) : ConfigException(message) + +/** + * The file is written in a shape this GitTally no longer reads. Refusing it is the point: + * a key that moved and is silently ignored means a build that quietly stops happening. + */ +class ConfigFormatException( + message: String, +) : ConfigException(message) + object ConfigVersions { /** * The version in which the configuration format last changed incompatibly — a file diff --git a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt index b186342..41cc867 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt @@ -22,7 +22,7 @@ data class GitTallyConfig( ) { /** The configured [buildDefinitions] plus the implicit `default` build unless overridden. */ fun effectiveBuildDefinitions(): Map = - mapOf(BuildDefinition.DEFAULT to BuildDefinition(onPush = true)) + buildDefinitions + mapOf(BuildDefinition.DEFAULT to BuildDefinition(trigger = TriggerConfig(onPush = true))) + buildDefinitions /** * The settings one build of [build] on [branch] runs with: the branch entry (falling @@ -153,6 +153,8 @@ data class BranchConfig( * head (`refs/pull//head` on origin); manual `build` commands are not affected. */ val requirePullRequest: Boolean = false, + /** Gitea commit status context of this build; empty uses `gitea.statusContext`. */ + val statusContext: String = "", val autoBuild: AutoBuildConfig = AutoBuildConfig(), val docker: DockerConfig = DockerConfig(), ) diff --git a/src/main/kotlin/de/hoennig/gittally/gitea/GiteaClient.kt b/src/main/kotlin/de/hoennig/gittally/gitea/GiteaClient.kt index 64ff6cd..e5fbb2c 100644 --- a/src/main/kotlin/de/hoennig/gittally/gitea/GiteaClient.kt +++ b/src/main/kotlin/de/hoennig/gittally/gitea/GiteaClient.kt @@ -56,13 +56,19 @@ class GiteaClient( fun isEnabled(workingDir: Path = Paths.get(".")): Boolean = isEnabled(configLoader.load(workingDir)) - /** Publishes a commit status for [sha]; returns false when disabled or the request failed. */ + /** + * Publishes a commit status for [sha]; returns false when disabled or the request failed. + * [context] is the build's own status context (`builds..statusContext`); blank or + * null uses the repository-wide `gitea.statusContext`. Two builds of one commit sharing + * a context overwrite each other's result, which is what a per-build context avoids. + */ fun publishStatus( sha: String, status: BuildStatus, description: String, targetUrl: String? = null, workingDir: Path = Paths.get("."), + context: String? = null, ): Boolean { val config = configLoader.load(workingDir) if (!isEnabled(config)) { @@ -71,7 +77,7 @@ class GiteaClient( } val body = mutableMapOf() body["state"] = status.toGiteaState() - body["context"] = config.gitea.statusContext + body["context"] = context?.takeIf { it.isNotBlank() } ?: config.gitea.statusContext body["description"] = description if (!targetUrl.isNullOrBlank()) { body["target_url"] = targetUrl diff --git a/src/main/kotlin/de/hoennig/gittally/server/BranchListing.kt b/src/main/kotlin/de/hoennig/gittally/server/BranchListing.kt index ac4d2e8..4c41fea 100644 --- a/src/main/kotlin/de/hoennig/gittally/server/BranchListing.kt +++ b/src/main/kotlin/de/hoennig/gittally/server/BranchListing.kt @@ -22,16 +22,25 @@ class BranchListing( ) { fun branches(workingDir: Path = Paths.get(".")): List { val heads = gitService.originBranchHeads(workingDir) + val namedResults = repository.latestPerName().filter { it.name != it.branch && it.branch in heads } + val branchesWithNamedPool = namedResults.map { it.branch }.toSet() val branchRows = - heads.map { (branch, headCommit) -> - // latestFor groups by build name, so a named slot's results never shadow the branch row - BranchDto.from(branch, headCommit, repository.latestFor(branch)) - } + heads + .mapNotNull { (branch, headCommit) -> + // latestFor groups by build name, so a named slot's results never shadow the branch row + val latest = repository.latestFor(branch) + // A branch whose builds all belong to named definitions has no default pool, + // and an empty row for it would read as "never built" next to its real ones. + // Without any build at all the row stays: that a branch is known and idle is + // exactly what it says then. + if (latest == null && branch in branchesWithNamedPool) { + null + } else { + BranchDto.from(branch, headCommit, latest) + } + } val namedRows = - repository - .latestPerName() - .filter { it.name != it.branch && it.branch in heads } - .map { latest -> BranchDto.from(latest.branch, latest.commit, latest, name = latest.name) } + namedResults.map { latest -> BranchDto.from(latest.branch, latest.commit, latest, name = latest.name) } return (branchRows + namedRows) .sortedWith(compareBy({ sortGroup(it.branch) }, { it.branch }, { it.name })) .map { row -> diff --git a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt index bb5ce21..aca65f1 100644 --- a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt @@ -208,7 +208,7 @@ class Watcher( gitService.newOriginBranches(DurationParser.parse(config.watcher.newBranchMaxAge), workingDir) val changed = (changedLocal + newOrigin).distinct() for (branch in changed) { - val onPush = definitionsFor(branch, heads[branch], workingDir, config).filterValues { it.onPush } + val onPush = definitionsFor(branch, heads[branch], workingDir, config).filterValues { it.trigger.onPush } for ((buildName, definition) in onPush) { if (selects(definition, branch, headCommitTimes)) { startBuildIfDue(branch, allowSameCommit = false, config, pullRequestHeads, workingDir, buildName) @@ -270,7 +270,7 @@ class Watcher( definition: BuildDefinition, branch: String, headCommitTimes: Lazy>, - ): Boolean = definition.selects(branch, { headCommitTimes.value[branch] }, clock.instant()) + ): Boolean = definition.trigger.selects(branch, { headCommitTimes.value[branch] }, clock.instant()) /** * Enqueues a build of the branch's origin head unless one is already pending or @@ -331,12 +331,14 @@ class Watcher( val timeOfDay = LocalTime.ofInstant(now, ZoneOffset.UTC) for (branch in originBranches) { val scheduled = - definitionsFor(branch, heads[branch], workingDir, config).filterValues { it.atTimes.isNotEmpty() } + definitionsFor(branch, heads[branch], workingDir, config).filterValues { + it.trigger.atTimes.isNotEmpty() + } for ((buildName, definition) in scheduled) { if (!selects(definition, branch, headCommitTimes)) { continue } - val slot = AutoBuildSlots.latestDueSlot(definition.atTimes, timeOfDay) ?: continue + val slot = AutoBuildSlots.latestDueSlot(definition.trigger.atTimes, timeOfDay) ?: continue val pool = BuildDefinition.poolName(branch, buildName) if (autoBuildState.value.isTriggered(pool, today, slot)) { continue diff --git a/src/main/resources/templates/releases.html b/src/main/resources/templates/releases.html index 35d42da..dc27171 100644 --- a/src/main/resources/templates/releases.html +++ b/src/main/resources/templates/releases.html @@ -7,6 +7,29 @@
+

v0.9.20 — 2026-08-29

+
    +
  • A build definition is now split in two: a trigger block says when the + build runs and for which branches (onPush, atTimes, + branches, activeWithin), everything beside it says what + the build does. Only the second half is inherited from builds.default, + and making that structural means a selector added later cannot become inheritable + by accident. A definition still writing those keys flat is refused by name — a + trigger nobody reads any more is a build that silently stops running.
    • +
  • A branch pattern prefixed with ! excludes instead of selecting, and an + exclusion wins whatever the order: branches: ["*", "!master"] is every + branch but master. That lets one branch have a build of its own without being built + by the default one as well — previously the only way to avoid the double build was + to give up the second definition's push trigger.
    • +
  • A build may report under its own Gitea check with statusContext; empty + keeps the repository-wide gitea.statusContext. Two builds of one commit + used to overwrite each other's result there, so a second build over the same branch + — a quick check next to a long one — was not readable in Gitea.
    • +
  • Fixed: a branch whose builds all belong to named definitions showed + an empty row in the branches view, reading as "never built" right next to its actual + builds.
    • +
+

v0.9.19 — 2026-08-29

  • A build definition now describes its build completely: requirePullRequest diff --git a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt index 3748dbf..8a191ba 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt @@ -132,9 +132,9 @@ class BuildExecutorTest : FunSpec() { liveLog shouldContain "out-main" liveLog shouldContain "err-main" - 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.giteaClient.publishStatus("abc123", BuildStatus.PENDING, any(), null, h.workingDir, any()) } + verify { h.giteaClient.publishStatus("abc123", BuildStatus.RUNNING, any(), null, h.workingDir, any()) } + verify { h.giteaClient.publishStatus("abc123", BuildStatus.SUCCESS, any(), null, h.workingDir, any()) } verify { h.artifactStore.persist(match { it.status == BuildStatus.SUCCESS }, build.stagingDir, h.workingDir) } } @@ -395,7 +395,7 @@ class BuildExecutorTest : FunSpec() { h.events.map { it.result.status } shouldContainExactly listOf(BuildStatus.PENDING, BuildStatus.RUNNING, BuildStatus.INTERRUPTED) Files.readString(build.liveLogFile) shouldContain "build interrupted by shutdown" - verify { h.giteaClient.publishStatus("abc123", BuildStatus.INTERRUPTED, any(), null, h.workingDir) } + verify { h.giteaClient.publishStatus("abc123", BuildStatus.INTERRUPTED, any(), null, h.workingDir, any()) } verify { h.artifactStore.persist(match { it.status == BuildStatus.INTERRUPTED }, build.stagingDir, any()) } } @@ -426,7 +426,7 @@ class BuildExecutorTest : FunSpec() { h.events .filter { it.result.artifactKey == second.artifactKey } .map { it.result.status } shouldContainExactly listOf(BuildStatus.PENDING) - verify(exactly = 0) { h.giteaClient.publishStatus("sha-2", BuildStatus.RUNNING, any(), any(), any()) } + verify(exactly = 0) { h.giteaClient.publishStatus("sha-2", BuildStatus.RUNNING, any(), any(), any(), any()) } } test("shutdown without any build in flight is a no-op") { @@ -464,7 +464,7 @@ class BuildExecutorTest : FunSpec() { test("a Gitea failure does not fail the build") { val h = harness("echo ok") every { - h.giteaClient.publishStatus(any(), any(), any(), any(), any()) + h.giteaClient.publishStatus(any(), any(), any(), any(), any(), any()) } throws RuntimeException("gitea down") h.executor.startBuild("main", "abc123", h.workingDir) diff --git a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt index aacb6d5..c07d5d9 100644 --- a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt @@ -2,6 +2,8 @@ package de.hoennig.gittally.config import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.booleans.shouldBeFalse +import io.kotest.matchers.booleans.shouldBeTrue import io.kotest.matchers.maps.shouldBeEmpty import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe @@ -69,9 +71,10 @@ class ConfigLoaderTest : FunSpec() { maxConcurrent: 2 builds: pitest: - atTimes: ["01:00"] - branches: ["master", "release/*"] - activeWithin: 24h + trigger: + atTimes: ["01:00"] + branches: ["master", "release/*"] + activeWithin: 24h buildCommand: ./gradlew piTestFull """.trimIndent(), ) @@ -83,14 +86,17 @@ class ConfigLoaderTest : FunSpec() { mapOf( "pitest" to BuildDefinition( - atTimes = listOf("01:00"), - branches = listOf("master", "release/*"), - activeWithin = "24h", + trigger = + TriggerConfig( + atTimes = listOf("01:00"), + branches = listOf("master", "release/*"), + activeWithin = "24h", + ), buildCommand = "./gradlew piTestFull", ), ) // the implicit default build (onPush over all branches) stays in place - config.effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(onPush = true) + config.effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(trigger = TriggerConfig(onPush = true)) } test("an explicit builds.default entry overrides the implicit default build") { @@ -99,11 +105,12 @@ class ConfigLoaderTest : FunSpec() { """ builds: default: - onPush: false + trigger: + onPush: false """.trimIndent(), ) - loader.load(dir).effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(onPush = false) + loader.load(dir).effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(trigger = TriggerConfig(onPush = false)) } test("a config that needs a newer GitTally is refused, naming the file and both versions") { @@ -209,7 +216,8 @@ class ConfigLoaderTest : FunSpec() { """ builds: pitest: - atTimes: ["01:00"] + trigger: + atTimes: ["01:00"] buildCommand: ./gradlew piTestPartial """.trimIndent(), ) @@ -220,7 +228,8 @@ class ConfigLoaderTest : FunSpec() { pitest: buildCommand: ./gradlew piTestFull experiment: - onPush: true + trigger: + onPush: true """.trimIndent(), ) @@ -228,8 +237,12 @@ class ConfigLoaderTest : FunSpec() { // the branch layer merges into the definition instead of replacing it config.buildDefinitions.getValue("pitest").buildCommand shouldBe "./gradlew piTestFull" - config.buildDefinitions.getValue("pitest").atTimes shouldBe listOf("01:00") - config.buildDefinitions.getValue("experiment").onPush shouldBe true + config.buildDefinitions + .getValue("pitest") + .trigger.atTimes shouldBe listOf("01:00") + config.buildDefinitions + .getValue("experiment") + .trigger.onPush shouldBe true } test("a branch cannot raise the concurrency limit or reach the sandbox policy through a build definition") { @@ -239,6 +252,7 @@ class ConfigLoaderTest : FunSpec() { builds: default: requirePullRequest: true + statusContext: GitTally docker: enabled: true network: none @@ -255,6 +269,7 @@ class ConfigLoaderTest : FunSpec() { builds: default: requirePullRequest: false + statusContext: GitTally/impersonated docker: enabled: false network: host @@ -270,6 +285,7 @@ class ConfigLoaderTest : FunSpec() { settings.requirePullRequest shouldBe true settings.docker.enabled shouldBe true settings.docker.network shouldBe "none" + settings.statusContext shouldBe "GitTally" // everything that describes the build itself stays the branch's own business settings.docker.image shouldBe "attacker-image" } @@ -291,7 +307,8 @@ class ConfigLoaderTest : FunSpec() { """ builds: invented: - atTimes: ["03:00"] + trigger: + atTimes: ["03:00"] buildCommand: ./gradlew whatever docker: enabled: false @@ -309,20 +326,119 @@ class ConfigLoaderTest : FunSpec() { settings.requirePullRequest shouldBe true } + test("an exclusion pattern takes a branch out of a build that would otherwise select it") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + builds: + default: + trigger: + onPush: true + branches: ["*", "!master"] + release: + trigger: + onPush: true + branches: ["master"] + """.trimIndent(), + ) + + val definitions = loader.load(dir).buildDefinitions + + definitions + .getValue("default") + .trigger + .selectsByName("mihoe/feature") + .shouldBeTrue() + definitions + .getValue("default") + .trigger + .selectsByName("master") + .shouldBeFalse() + definitions + .getValue("release") + .trigger + .selectsByName("master") + .shouldBeTrue() + definitions + .getValue("release") + .trigger + .selectsByName("mihoe/feature") + .shouldBeFalse() + } + + test("an exclusion wins over a matching pattern, whatever their order") { + val trigger = TriggerConfig(branches = listOf("!release/hotfix", "release/*")) + + trigger.selectsByName("release/1.0").shouldBeTrue() + trigger.selectsByName("release/hotfix").shouldBeFalse() + } + + test("a trigger key written outside the trigger block is refused, naming the definition") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + builds: + nightly: + atTimes: ["01:00"] + buildCommand: ./gradlew check + """.trimIndent(), + ) + + // ignoring it would leave the build without a trigger — a job that silently + // stops running is worse than a configuration that refuses to load + val thrown = shouldThrow { loader.load(dir) } + + thrown.message.shouldContain(".gittally.yml") + thrown.message.shouldContain("builds.nightly: atTimes") + thrown.message.shouldContain("trigger:") + } + + test("a branch writing its trigger flat fails only its own builds") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + builds: + default: + trigger: + onPush: true + """.trimIndent(), + ) + + shouldThrow { + loader.loadWithBranchLayer( + dir, + """ + builds: + experiment: + onPush: true + """.trimIndent(), + ) + }.message.shouldContain("this branch") + // the primary config is untouched, so every other branch keeps building + loader + .load(dir) + .buildDefinitions + .getValue("default") + .trigger.onPush + .shouldBeTrue() + } + test("builds.default is the base of every other build, but never its trigger") { val dir = Files.createTempDirectory("gittally-test") dir.resolve(".gittally.yml").toFile().writeText( """ builds: default: - onPush: true - branches: ["master"] + trigger: + onPush: true + branches: ["master"] buildCommand: ./gradlew check artifactDirs: [build/reports] docker: image: shared-image nightly: - atTimes: ["01:00"] + trigger: + atTimes: ["01:00"] artifactDirs: [build/reports, build/libs] """.trimIndent(), ) @@ -333,9 +449,9 @@ class ConfigLoaderTest : FunSpec() { nightly.docker?.image shouldBe "shared-image" nightly.artifactDirs shouldBe listOf("build/reports", "build/libs") // a trigger says when *this* build runs; inheriting it would fire every job at once - nightly.onPush shouldBe false - nightly.branches shouldBe emptyList() - nightly.atTimes shouldBe listOf("01:00") + nightly.trigger.onPush shouldBe false + nightly.trigger.branches shouldBe emptyList() + nightly.trigger.atTimes shouldBe listOf("01:00") } test("branches is honored while no build is defined and ignored as soon as one is") { @@ -587,14 +703,17 @@ class ConfigLoaderTest : FunSpec() { default: buildCommand: from-branch pitest: - atTimes: ["03:00"] + trigger: + atTimes: ["03:00"] buildCommand: ./gradlew piTestFull """.trimIndent(), ) config.git.token shouldBe "real-secret" config.buildSettings("main", "default").buildCommand shouldBe "from-branch" - config.buildDefinitions.getValue("pitest").atTimes shouldBe listOf("03:00") + config.buildDefinitions + .getValue("pitest") + .trigger.atTimes shouldBe listOf("03:00") } test("loadWithBranchLayer without a branch config equals load") { diff --git a/src/test/kotlin/de/hoennig/gittally/gitea/GiteaClientTest.kt b/src/test/kotlin/de/hoennig/gittally/gitea/GiteaClientTest.kt index a63c03d..73dcd35 100644 --- a/src/test/kotlin/de/hoennig/gittally/gitea/GiteaClientTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/gitea/GiteaClientTest.kt @@ -6,6 +6,7 @@ import com.github.tomakehurst.wiremock.client.WireMock.equalTo import com.github.tomakehurst.wiremock.client.WireMock.equalToJson import com.github.tomakehurst.wiremock.client.WireMock.get import com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor +import com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath import com.github.tomakehurst.wiremock.client.WireMock.okJson import com.github.tomakehurst.wiremock.client.WireMock.post import com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor @@ -102,6 +103,34 @@ class GiteaClientTest : ) } + test("a build's own status context replaces the repository-wide one") { + server.stubFor(post(publishUrl).willReturn(aResponse().withStatus(201))) + + // without this, two builds of one commit overwrite each other's check + client.publishStatus( + sha = "abc123", + status = BuildStatus.SUCCESS, + description = "d", + context = "GitTally/quick", + ) shouldBe true + + server.verify( + postRequestedFor(urlEqualTo(publishUrl)) + .withRequestBody(matchingJsonPath("${'$'}.context", equalTo("GitTally/quick"))), + ) + } + + test("a blank build status context falls back to the repository-wide one") { + server.stubFor(post(publishUrl).willReturn(aResponse().withStatus(201))) + + client.publishStatus("abc123", BuildStatus.SUCCESS, "d", context = "") shouldBe true + + server.verify( + postRequestedFor(urlEqualTo(publishUrl)) + .withRequestBody(matchingJsonPath("${'$'}.context", equalTo("GitTally"))), + ) + } + test("omits target_url when none is given") { server.stubFor(post(publishUrl).willReturn(aResponse().withStatus(201))) diff --git a/src/test/kotlin/de/hoennig/gittally/server/BranchListingTest.kt b/src/test/kotlin/de/hoennig/gittally/server/BranchListingTest.kt index 112cdba..0424189 100644 --- a/src/test/kotlin/de/hoennig/gittally/server/BranchListingTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/server/BranchListingTest.kt @@ -31,6 +31,19 @@ class BranchListingTest : FunSpec() { every { repository.latestPerName() } returns emptyList() } + test("a branch built only by named definitions loses its empty default row") { + val releaseResult = mainResult.copy(build = "release", name = "master@release") + every { gitService.originBranchHeads(any()) } returns mapOf("master" to "aaa", "idle" to "bbb") + every { repository.latestPerName() } returns listOf(releaseResult.copy(branch = "master")) + every { repository.latestFor(any()) } returns null + every { repository.latestGreenFor(any()) } returns null + + val rows = listing.branches() + + // no bare "master" row next to master@release — it would read as "never built" + rows.map { it.name } shouldBe listOf("master@release", "idle") + } + test("orders main/master first, then flat names, then hierarchical names") { every { gitService.originBranchHeads(any()) } returns mapOf( diff --git a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt index d31f939..0056b2f 100644 --- a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt @@ -14,6 +14,7 @@ import de.hoennig.gittally.config.BranchConfig import de.hoennig.gittally.config.BuildDefinition import de.hoennig.gittally.config.ConfigLoader import de.hoennig.gittally.config.GitTallyConfig +import de.hoennig.gittally.config.TriggerConfig import de.hoennig.gittally.config.WatcherConfig import de.hoennig.gittally.git.GitService import io.kotest.assertions.throwables.shouldThrow @@ -394,8 +395,7 @@ class WatcherTest : FunSpec() { mapOf( "pitest" to BuildDefinition( - atTimes = listOf("11:00"), - branches = listOf("main", "release/*"), + trigger = TriggerConfig(atTimes = listOf("11:00"), branches = listOf("main", "release/*")), ), ), ), @@ -421,7 +421,7 @@ class WatcherTest : FunSpec() { Harness( GitTallyConfig( buildDefinitions = - mapOf("pitest" to BuildDefinition(atTimes = listOf("11:00"), activeWithin = "24h")), + mapOf("pitest" to BuildDefinition(trigger = TriggerConfig(atTimes = listOf("11:00"), activeWithin = "24h"))), ), ) every { harness.gitService.originBranches(any()) } returns listOf("active", "dormant") @@ -443,7 +443,7 @@ class WatcherTest : FunSpec() { Harness( GitTallyConfig( buildDefinitions = - mapOf("lint" to BuildDefinition(onPush = true, branches = listOf("main"))), + mapOf("lint" to BuildDefinition(trigger = TriggerConfig(onPush = true, branches = listOf("main")))), ), ) every { harness.gitService.originBranches(any()) } returns listOf("main", "feature/x") @@ -463,7 +463,7 @@ class WatcherTest : FunSpec() { test("builds.default with onPush false disables the implicit on-push build") { val harness = - Harness(GitTallyConfig(buildDefinitions = mapOf("default" to BuildDefinition(onPush = false)))) + Harness(GitTallyConfig(buildDefinitions = mapOf("default" to BuildDefinition(trigger = TriggerConfig(onPush = false))))) every { harness.gitService.originBranches(any()) } returns listOf("main") every { harness.gitService.localBranches(any()) } returns listOf("main") every { harness.gitService.hasNewCommits("main", any()) } returns true @@ -490,7 +490,7 @@ class WatcherTest : FunSpec() { val harness = Harness() val branchLayer = GitTallyConfig( - buildDefinitions = mapOf("pitest" to BuildDefinition(atTimes = listOf("11:00"))), + buildDefinitions = mapOf("pitest" to BuildDefinition(trigger = TriggerConfig(atTimes = listOf("11:00")))), ) every { harness.gitService.originBranches(any()) } returns listOf("main", "experiment") every { harness.gitService.originBranchHeads(any()) } returns @@ -515,7 +515,7 @@ class WatcherTest : FunSpec() { val branchLayer = GitTallyConfig( buildDefinitions = - mapOf("pitest" to BuildDefinition(atTimes = listOf("11:00"), branches = listOf("main"))), + mapOf("pitest" to BuildDefinition(trigger = TriggerConfig(atTimes = listOf("11:00"), branches = listOf("main")))), ) every { harness.gitService.originBranches(any()) } returns listOf("main", "experiment") every { harness.gitService.originBranchHeads(any()) } returns @@ -558,7 +558,7 @@ class WatcherTest : FunSpec() { // caching the definitions by head commit alone would never notice val edited = GitTallyConfig( - buildDefinitions = mapOf("nightly" to BuildDefinition(atTimes = listOf("11:00"))), + buildDefinitions = mapOf("nightly" to BuildDefinition(trigger = TriggerConfig(atTimes = listOf("11:00")))), ) every { harness.configLoader.load(any()) } returns edited every { harness.configLoader.loadWithBranchLayer(any(), anyNullable()) } returns edited