From f0f996a53c89442ed948911bf47e5fa2a372c3e6 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Sat, 29 Aug 2026 12:05:10 +0200 Subject: [PATCH] A build definition says when it runs in a trigger block of its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `onPush`, `atTimes`, `branches`, and `activeWithin` move into a nested `trigger`. The split is structural on purpose: the inheritance from `builds.default` now subtracts one key instead of a list of four, so a selector added to `TriggerConfig` later is non-inheritable by construction rather than because someone remembered to extend the list. A definition still writing those keys flat is refused by name, per file and scoped like the version check — the machine and project config abort the start, a branch's committed config fails only that branch. Ignoring them would leave the build with no trigger at all, which is a job that quietly stops running: the failure this refusal exists to prevent. Two more things a definition can now say: - A `!` prefix in `trigger.branches` excludes, and an exclusion wins whatever the order. `["*", "!master"]` gives one branch a build of its own without the default build running over it as well — until now the only way out of that double build was to drop the second definition's push trigger. - `statusContext` overrides the Gitea check this build reports as, empty keeping the repository-wide one. Two builds of a commit shared a context and overwrote each other's result, so a quick check beside a long build was not readable in Gitea. Pinned like `requirePullRequest`: a branch that could pick its context could take over the check a branch protection rule depends on. Fixed on the way: a branch whose builds all belong to named definitions rendered an empty row in the branches view, reading as "never built" directly beside its real builds. That row was unreachable before the exclusion patterns made such a branch possible. --- .claude/skills/architecture/SKILL.md | 4 +- AGENTS.md | 3 +- build.gradle.kts | 2 +- docs/configuration.md | 57 ++++-- docs/plan/18-remove-branches-section.md | 41 ++--- docs/plan/README.md | 2 +- .../hoennig/gittally/GitTallyApplication.kt | 4 +- .../hoennig/gittally/build/BuildExecutor.kt | 15 ++ .../hoennig/gittally/commands/InitCommand.kt | 20 ++- .../gittally/config/BuildDefinition.kt | 143 +++++++++------ .../hoennig/gittally/config/ConfigLoader.kt | 53 +++++- .../hoennig/gittally/config/ConfigVersion.kt | 15 +- .../hoennig/gittally/config/GitTallyConfig.kt | 4 +- .../de/hoennig/gittally/gitea/GiteaClient.kt | 10 +- .../hoennig/gittally/server/BranchListing.kt | 25 ++- .../de/hoennig/gittally/watcher/Watcher.kt | 10 +- src/main/resources/templates/releases.html | 23 +++ .../gittally/build/BuildExecutorTest.kt | 12 +- .../gittally/config/ConfigLoaderTest.kt | 163 +++++++++++++++--- .../hoennig/gittally/gitea/GiteaClientTest.kt | 29 ++++ .../gittally/server/BranchListingTest.kt | 13 ++ .../hoennig/gittally/watcher/WatcherTest.kt | 16 +- 22 files changed, 486 insertions(+), 178 deletions(-) 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