diff --git a/.claude/skills/architecture/SKILL.md b/.claude/skills/architecture/SKILL.md index e6ff812..e46f1f0 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 — build settings and 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`, the per-branch `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` 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, `branches.default` is merged into every other named branch entry as its fallback, then the result is bound to the `GitTallyConfig` data classes (`config/GitTallyConfig.kt`), which define the schema and all defaults. +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". 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`. @@ -66,7 +66,7 @@ The runtime is selected per branch behind the `BuildRunner` interface: `Dispatch ## Watcher -`Watcher` replaces the legacy blocking main loop with a non-blocking fixed-delay poll cycle: fetch origin, enqueue due branches (changed local, recent new origin, due auto-build slots) via `BuildExecutor`, then prune results, artifacts, and stale worktrees. Branches with `branches..requirePullRequest` are enqueued only while their head commit matches a pull-request head, detected without an API token by listing `refs/pull/*/head` via `git ls-remote` (lazily, at most once per poll cycle); manual `build` commands bypass this gate, and `watcher.pullRequestGate: false` disables it globally for plain-git origins without pull-request refs. Which builds are due is decided per branch from that branch's own definitions (`definitionsFor`): the primary config with the branch's committed `.gittally.yml` merged on top, cached per branch by its head commit so the `git show` runs only when the branch moved, and falling back to the primary definitions when that config is unreadable. Nothing is scheduled until `Watcher.start()` is called explicitly (server/watch mode) — CLI commands and tests never start the loop. "Already built" is tracked via the result repository, not by moving local branch refs. +`Watcher` replaces the legacy blocking main loop with a non-blocking fixed-delay poll cycle: fetch origin, enqueue due branches (changed local, recent new origin, due auto-build slots) via `BuildExecutor`, then prune results, artifacts, and stale worktrees. Branches whose build has `requirePullRequest` are enqueued only while their head commit matches a pull-request head, detected without an API token by listing `refs/pull/*/head` via `git ls-remote` (lazily, at most once per poll cycle); manual `build` commands bypass this gate, and `watcher.pullRequestGate: false` disables it globally for plain-git origins without pull-request refs. Which builds are due is decided per branch from that branch's own definitions (`definitionsFor`): the primary config with the branch's committed `.gittally.yml` merged on top, cached per branch by its head commit *and* the primary config it was merged with, so the `git show` runs only when the branch moved while an edited machine or project config still takes effect on the next poll, and falling back to the primary definitions when that config is unreadable. Nothing is scheduled until `Watcher.start()` is called explicitly (server/watch mode) — CLI commands and tests never start the loop. "Already built" is tracked via the result repository, not by moving local branch refs. After the enqueue decision — and only after it, because a local ref lagging behind origin *is* the change signal — the cycle fast-forwards the primary checkout's local branch refs to their origin counterparts (`watcher.fastForwardLocalRefs`, `GitService.fastForwardLocalBranches`), so build tools reading the shared `.git` from a worktree see the refs they expect; diverged or ahead branches are never touched. Auto-build slot state lives in `.git/gittally/auto-builds.json`; watcher health is exposed via `Watcher.state()`. ## System Metrics diff --git a/AGENTS.md b/AGENTS.md index c4d56a8..8d01b03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,6 +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. +- `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/docs/configuration.md b/docs/configuration.md index 6772b21..202adf5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -60,10 +60,10 @@ branches that are fine. The `.gittally.yml` committed on a branch is applied as a third layer on top of the two above, giving the precedence **branch > repo install > project**. It takes precedence for -everything that describes how this branch is built: `buildCommand`, `cleanCommand`, -`artifactDirs`, log file names, `docker.image`/`dockerfile`/`context`/`env`, and the whole -`builds` section — its own definitions and its overrides of the definitions from the -project config. That is how a new configuration is tried out: change it on a branch, and +everything that describes how this branch is built: the whole `builds` section — its own +definitions and its overrides of the definitions from the project config, with +`buildCommand`, `cleanCommand`, `artifactDirs`, log file names, and +`docker.image`/`dockerfile`/`context`/`env` inside them. That is how a new configuration is tried out: change it on a branch, and no other branch's builds are affected. The branch layer is used in both places where it matters: the watcher reads the committed @@ -87,7 +87,9 @@ This keeps a branch from reaching credentials, reporting statuses to another rep raising the global concurrency, disabling its own build container, changing its network mode, or bypassing its own pull-request gate. Everything else is the branch's to decide — it can already run any command through `buildCommand`. -The deprecated `autoBuild` schedules are read from the repo install/project config only. +The pinned settings are stripped wherever they appear, in a build definition as well as in +a legacy `branches` entry. The deprecated `branches` section itself is read from the repo +install/project config only, and only while nothing defines a build at all. ## Inspect the Effective Config @@ -159,22 +161,52 @@ executor: # Named build definitions (jobs, see notes below); every key names a build. builds: - # Implicit unless overridden: the default build runs on push over all branches - # with the branch's regular settings — exactly the behavior without any - # build definitions. Set onPush: false here to disable on-push builds. - # default: - # onPush: true - # - # Example of a named build definition; all keys except its name are optional: - # 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 - # buildCommand: ./gradlew piTestFull # overrides; unset keys fall back to the - # cleanCommand: rm -rf build # merged branch settings (also available: - # artifactDirs: [build/reports] # stdoutLog, stderrLog, and docker - # # image/dockerfile/context/env) + # "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. + default: + onPush: true # trigger: build every new commit of the selected branches + # run before each build + cleanCommand: rm -rf build + # shell command for each build + buildCommand: ./gradlew --console=plain --no-daemon test + # directories copied as build artifacts + artifactDirs: + - build/reports + - build/doc + stdoutLog: build.stdout.log # filename for captured stdout + stderrLog: build.stderr.log # filename for captured stderr + # Build a selected branch only while its head commit matches a pull-request head on + # 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 + # Optional Docker build runtime; when enabled, the clean and build commands + # run inside a container instead of natively (see notes below). + docker: + # run clean/build commands in a Docker container (pinned) + enabled: false + # image for the build container; required when enabled + image: "" + # Dockerfile to (re)build the image from when it is missing or stale; empty pulls the image as-is + dockerfile: "" + # Docker build context used with dockerfile + context: "." + # Docker network mode for the build container; empty = Docker default (pinned) + network: "" + # additional environment variables set inside the build container + env: {} + + # Every further job inherits the settings above and adds its own trigger and selector. + # 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 + buildCommand: ./gradlew -PfullPitTest --console=plain --no-daemon piTestFull + artifactDirs: [build/reports, build/libs] # Build artifact storage and retention. artifacts: @@ -202,7 +234,7 @@ watcher: pollInterval: 10s # max commit age for new origin branches to be pulled automatically newBranchMaxAge: 5d - # Honor the branches..requirePullRequest gates (see notes below). + # Honor the builds..requirePullRequest gates (see notes below). # Set false for a plain git origin without pull-request refs (no Gitea/GitHub); # gated branches then build on new commits like any other branch. pullRequestGate: true @@ -211,58 +243,6 @@ watcher: # ahead local branch is never touched. Set false to leave refs/heads/* alone entirely. fastForwardLocalRefs: true -# Per-branch build configuration. -# Use "default" as the fallback for all branches not listed explicitly. -# Each entry merges build settings and auto-build scheduling. -branches: - default: - # run before each build - cleanCommand: rm -rf build - # shell command for each build - buildCommand: ./gradlew --console=plain --no-daemon test - # directories copied as build artifacts - artifactDirs: - - build/reports - - build/doc - stdoutLog: build.stdout.log # filename for captured stdout - stderrLog: build.stderr.log # filename for captured stderr - # Build this branch only while its head commit matches a pull-request head on origin - # (refs/pull/*/head — read via plain git, no API token needed; see notes below). - requirePullRequest: false - # DEPRECATED: define a build with atTimes in the builds section instead. - # Kept for compatibility: rebuilds this branch on schedule with its regular command. - autoBuild: - enabled: false # whether to rebuild on schedule - times: ["01:00"] # UTC times HH:MM for scheduled builds - # Optional Docker build runtime; when enabled, the clean and build commands - # run inside a container instead of natively (see notes below). - docker: - # run clean/build commands in a Docker container - enabled: false - # image for the build container; required when enabled - image: "" - # Dockerfile to (re)build the image from when it is missing or stale; empty pulls the image as-is - dockerfile: "" - # Docker build context used with dockerfile - context: "." - # Docker network mode for the build container; empty = Docker default - network: "" - # additional environment variables set inside the build container - env: {} - - master: - buildCommand: ./gradlew --console=plain --no-daemon quickCheck - - release: - buildCommand: ./gradlew --console=plain --no-daemon --no-build-cache test jacocoReport - -builds: - # the nightly rebuild runs the full check instead of the quick on-commit one, - # recorded separately as master@pitest - pitest: - atTimes: ["01:00"] - branches: ["master"] - buildCommand: ./gradlew -PfullPitTest --console=plain --no-daemon piTestFull ``` ### Notes on `server.bindAddress` @@ -282,7 +262,7 @@ All nginx/certificate failures are non-fatal warnings; the plain HTTP server kee The container is labelled `org.hoennig.gittally`; stale nginx containers of the repository are removed before each start, and the container is removed on shutdown. `server.port` must differ from `httpPort` and `httpsPort`. -### Notes on `branches..requirePullRequest` +### Notes on `builds..requirePullRequest` The gate applies to all watcher-triggered builds (push-triggered and scheduled auto builds). A manual `gittally build ` always builds, regardless of this setting. @@ -294,17 +274,21 @@ This ls-remote call is made at most once per poll cycle, and only when a branch Because matching is by commit id, a closed pull request whose head ref still equals the branch head also counts. Distinguishing open from closed pull requests would require the Gitea API. -To build pull-request branches only, set the key under `branches.default` and override it for permanent branches: +To build pull-request branches only, gate the default build and give the permanent branches a build of their own: ```yaml -branches: +builds: default: + onPush: true requirePullRequest: true main: + onPush: true + branches: ["main"] requirePullRequest: false ``` -Without the `main` override, direct pushes and merges to `main` would never build — merge commits do not match any pull-request head. +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. 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. @@ -317,18 +301,19 @@ A build definition has triggers, a branch selector, and build-setting overrides. 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. Only the latest due slot of a day triggers, so slots missed while the server was down are skipped instead of piling up, and a slot whose pool is still building is retried on the next poll cycle until it succeeds. -A definition may have both; one with neither never triggers automatically. +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. `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. -The `branches..requirePullRequest` gate stays a branch property and gates all watcher-triggered builds of that branch. -Overrides: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, and the docker image keys (`image`, `dockerfile`, `context`, `env`). -A definition has no `docker.enabled`/`docker.network` and no `requirePullRequest` — those are pinned branch properties, see [the branch layer](#the-branch-layer-a-branch-describes-its-own-ci). -The effective settings of one build on one branch merge in this order: defaults → `branches.default` → `branches.` → the branch's committed `.gittally.yml` → the build definition's overrides. -Unset keys fall back; the definition wins last because it is the job. -Definitions themselves are part of the branch layer: a branch may add its own and override those from the project config, for its own builds only. +Settings: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, `requirePullRequest`, 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. +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. The implicit `default` build (`onPush: true`, all branches) preserves the behavior without any definitions; defining other builds does not disable it, `builds.default.onPush: false` does. The `default` build records under the plain branch name; every other build records under `@` with its own row in the branches view (sorted after its branch), its own `retentionPerBranch` count, latest status, and permanent latest-green artifact link. @@ -337,11 +322,24 @@ 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). -`branches..autoBuild` (`enabled` + `times`) is the deprecated pre-ADR-0007 schedule, kept for compatibility: it rebuilds the branch's own pool with its regular command and logs a deprecation warning. -`autoBuild.times` entries carrying their own `buildCommand`/`name` (a short-lived v0.9.13 syntax) are no longer supported — use a build definition. 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. +### The legacy `branches` section + +Before build definitions existed, the settings lived in a per-branch `branches` section, with `branches.default` as the fallback for every branch not listed. +That section is deprecated and will be removed. +It is still read, but only while the merged configuration defines no build at all: as soon as one real definition exists, `branches` is ignored completely and a warning names it. +`builds.maxConcurrent` is not a definition — a configuration carrying only that leftover still uses `branches`. + +Either or, never both: a definition now carries the complete description of its build, and two half-answers would silently pull against each other. +The decision is made on the merged configuration, so a machine config that defines builds switches `branches` off for every branch, including the branches whose committed config still has one. + +`branches..autoBuild` (`enabled` + `times`) is the pre-ADR-0007 schedule that goes with it: it rebuilds the branch's own pool with its regular command and logs a deprecation warning. +`autoBuild.times` entries carrying their own `buildCommand`/`name` (a short-lived v0.9.13 syntax) are no longer supported — use a build definition. + +To migrate, move `branches.default` to `builds.default`, add `onPush: true`, and turn every other branch entry into a definition with a `branches` selector of its own. + ### Notes on `watcher.fastForwardLocalRefs` Builds run in worktrees that share the primary checkout's `.git`, so a build tool can read `refs/heads/*` there. @@ -355,7 +353,7 @@ Only fast-forwards are applied, as a compare-and-swap against the commit just re A local branch that diverged from origin or is ahead of it stays untouched, so local work in the primary checkout is never lost. The branch checked out in the primary checkout is advanced with `git merge --ff-only`, which refuses to overwrite conflicting uncommitted changes; a refusal is logged and the cycle continues. -### Notes on `branches..docker` +### Notes on `builds..docker` With `docker.enabled`, GitTally shells out to the `docker` CLI; the `docker` command must be on the `PATH`. When `dockerfile` is set, the image is (re)built whenever the Dockerfile content, its path, or the context path changed. diff --git a/docs/migration-from-legacy.md b/docs/migration-from-legacy.md index 7898c7f..ba6cb29 100644 --- a/docs/migration-from-legacy.md +++ b/docs/migration-from-legacy.md @@ -9,28 +9,29 @@ See [configuration.md](configuration.md) for the full configuration reference an Legacy configuration came from environment variables (`gitTally --env` template, sourced env files). The new configuration lives in two YAML files: `.gittally.yml` (committed) and `.git/gittally/.gittally.yml` (machine-specific, secrets). -Branch-level keys below live under `branches.`; use `branches.default` for what used to be the global value. +Build-level keys below live in a build definition under `builds.`; use `builds.default` for what used to be +the global value — it is the base every other definition inherits its settings from. | Legacy environment variable | New YAML key | |---|---| -| `GITTALLY_BUILD_COMMAND` | `branches..buildCommand` | -| `GITTALLY_BUILD_CLEAN_COMMAND` | `branches..cleanCommand` | -| `GITTALLY_BUILD_ARTEFACT_DIRS` | `branches..artifactDirs` — YAML list instead of `;`-separated | -| `GITTALLY_BUILD_STDOUT_LOG` | `branches..stdoutLog` | -| `GITTALLY_BUILD_STDERR_LOG` | `branches..stderrLog` | +| `GITTALLY_BUILD_COMMAND` | `builds..buildCommand` | +| `GITTALLY_BUILD_CLEAN_COMMAND` | `builds..cleanCommand` | +| `GITTALLY_BUILD_ARTEFACT_DIRS` | `builds..artifactDirs` — YAML list instead of `;`-separated | +| `GITTALLY_BUILD_STDOUT_LOG` | `builds..stdoutLog` | +| `GITTALLY_BUILD_STDERR_LOG` | `builds..stderrLog` | | `GITTALLY_NEW_BRANCH_COMMIT_MAX_AGE` | `watcher.newBranchMaxAge` | -| `GITTALLY_BUILD_DOCKER_IMAGE` | `branches..docker.image` — also set `docker.enabled: true` (replaces the `--docker` flag) | -| `GITTALLY_BUILD_DOCKERFILE` | `branches..docker.dockerfile` | -| `GITTALLY_BUILD_DOCKER_CONTEXT` | `branches..docker.context` | -| `GITTALLY_BUILD_DOCKER_NETWORK` | `branches..docker.network` — default is now Docker's default network, not `host` | -| `GITTALLY_BUILD_DOCKER_ENV` | `branches..docker.env` — YAML map instead of space-separated assignments | +| `GITTALLY_BUILD_DOCKER_IMAGE` | `builds..docker.image` — also set `docker.enabled: true` (replaces the `--docker` flag) | +| `GITTALLY_BUILD_DOCKERFILE` | `builds..docker.dockerfile` | +| `GITTALLY_BUILD_DOCKER_CONTEXT` | `builds..docker.context` | +| `GITTALLY_BUILD_DOCKER_NETWORK` | `builds..docker.network` — default is now Docker's default network, not `host` | +| `GITTALLY_BUILD_DOCKER_ENV` | `builds..docker.env` — YAML map instead of space-separated assignments | | `GITTALLY_ARTIFACT_SERVER_PORT` | `server.port` | | `GITTALLY_ARTIFACT_SERVER_BIND_ADDRESS` | `server.bindAddress` | | `GITTALLY_ARTIFACT_PUBLIC_BASE_URL` | `server.publicBaseUrl` | | `GITTALLY_ARTIFACT_BUILD_RETENTION_PER_BRANCH` | `artifacts.retentionPerBranch` for a count, `artifacts.retentionMaxAge` for a legacy age value (`h`/`d` suffix); unlike legacy, both limits can be combined | | `GITTALLY_IMPRESSUM_URL` | `server.impressumUrl` | -| `GITTALLY_AUTO_BUILD_BRANCHES` | `branches..autoBuild.enabled: true` per branch instead of a branch list | -| `GITTALLY_AUTO_BUILD_TIMES` | `branches..autoBuild.times` — YAML list, per branch | +| `GITTALLY_AUTO_BUILD_BRANCHES` | a build definition with `branches: [...]` selecting them | +| `GITTALLY_AUTO_BUILD_TIMES` | `builds..atTimes` — YAML list of UTC `HH:MM` slots | | `GITTALLY_GITEA_BASE_URL` | `gitea.baseUrl` | | `GITTALLY_GITEA_OWNER` | `gitea.owner` | | `GITTALLY_GITEA_REPO` | `gitea.repo` | @@ -50,7 +51,7 @@ New keys without a legacy counterpart: `builds.maxConcurrent`, `artifacts.rootDi ## Intentionally Not Ported - Self-install and self-update (`--install`, `--pull`, `GITTALLY_INSTALL_DIR`) — replaced by jar deployment plus `init --systemd`. -- `GITTALLY_BUILD_DOCKER_PREFLIGHT_COMMAND` and `GITTALLY_BUILD_DOCKER_JAVA_TOOL_OPTIONS` — hsadmin-ng-specific; use `branches..docker.env` if needed. +- `GITTALLY_BUILD_DOCKER_PREFLIGHT_COMMAND` and `GITTALLY_BUILD_DOCKER_JAVA_TOOL_OPTIONS` — hsadmin-ng-specific; use `builds..docker.env` if needed. - `HSADMIN_NG_*` environment-variable fallbacks. - Env-file configuration itself — the systemd `EnvironmentFile` now only tunes the JVM (`JAVA_OPTS`). - `GITTALLY_GITEA_DELETED_STATUS_DESCRIPTION`, `GITTALLY_BIN_FORWARD`, `GITTALLY_CONFIG_*` — internal legacy mechanics without a counterpart. diff --git a/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt index d9018eb..f48ab4a 100644 --- a/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt +++ b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt @@ -138,7 +138,7 @@ class FileArtifactStore( log.warn("build {} has no workspace; storing only its logs", build.artifactKey) return } - for (artifactDir in branchConfig(build.branch, workspace).artifactDirs) { + for (artifactDir in buildSettings(build, workspace).artifactDirs) { if (artifactDir.isBlank()) { continue } @@ -159,14 +159,16 @@ class FileArtifactStore( "reports/$artifactDir" } - /** The build config for [branch], with the build [workspace]'s `.gittally.yml` layered on top (see [ConfigLoader.loadForWorktree]). */ - private fun branchConfig( - branch: String, + /** + * The settings [build] ran with, from the build [workspace]'s `.gittally.yml` layered + * on top of the primary config (see [ConfigLoader.loadForWorktree]) — resolved through + * [GitTallyConfig.buildSettings], so a job's own `artifactDirs` are archived and not + * only the ones its branch would have used. + */ + private fun buildSettings( + build: BuildResult, workspace: Path, - ): BranchConfig { - val branches = configLoader.loadForWorktree(workingDir, workspace).branches - return branches[branch] ?: branches["default"] ?: BranchConfig() - } + ): BranchConfig = configLoader.loadForWorktree(workingDir, workspace).buildSettings(build.branch, build.build) private fun copyChildren( sourceDir: Path, diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index 546f018..b8fe56d 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -175,17 +175,40 @@ class InitCommand( # how many builds may run at the same time (at most one build per branch regardless) maxConcurrent: 1 - # Named build definitions (jobs) over the branches; every key names a build. - # A branch may add or override definitions in its own committed .gittally.yml — - # they then apply to that branch alone, so a new job can be tried out on a branch. + # Named build definitions (jobs); every key names a build. + # "default" is the base every other definition inherits its settings from — never + # its trigger — and is itself the build of every branch as long as it has one. + # A branch may add or override definitions in its own committed .gittally.yml; + # they apply to that branch alone, so a new job can be tried out on one branch. builds: - # Example definition — triggers (onPush/atTimes), branch selector - # (branches/activeWithin), and overrides of the branch settings: + default: + onPush: true # build every new commit of the selected branches + # run before each build + cleanCommand: rm -rf build + # shell command for each build + buildCommand: ./gradlew --console=plain --no-daemon test + # directories copied as build artifacts + artifactDirs: + - build/reports + stdoutLog: build.stdout.log # filename for captured stdout + stderrLog: build.stderr.log # filename for captured stderr + # build only while the branch head matches a pull-request head on origin + # (refs/pull/*/head — read via plain git, no API token needed); + # pinned: a branch cannot set this in its own committed config + requirePullRequest: false + docker: + enabled: false # run clean/build in a container instead of natively (pinned) + image: "" # image for the build container; required when enabled + dockerfile: "" # Dockerfile to (re)build the image from when missing or stale; empty pulls the image as-is + 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: # 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 - # buildCommand: ./gradlew piTestFull + # buildCommand: ./gradlew pitestFull # Build artifact storage and retention. artifacts: @@ -206,40 +229,12 @@ class InitCommand( pollInterval: 10s # max commit age for new origin branches to be pulled automatically newBranchMaxAge: 5d - # honor branches..requirePullRequest; set false for a plain git origin + # honor builds..requirePullRequest; set false for a plain git origin # without pull-request refs (refs/pull/*/head) — gated branches then build on new commits pullRequestGate: true # after enqueueing, fast-forward the primary checkout's local branch refs to origin, # so build tools reading the shared .git see the same refs (diverged branches stay untouched) fastForwardLocalRefs: true - - # Per-branch build configuration. - # Use "default" as the fallback for all branches not listed explicitly. - branches: - default: - # run before each build - cleanCommand: rm -rf build - # shell command for each build - buildCommand: ./gradlew --console=plain --no-daemon test - # directories copied as build artifacts - artifactDirs: - - build/reports - stdoutLog: build.stdout.log # filename for captured stdout - stderrLog: build.stderr.log # filename for captured stderr - # build only while the branch head matches a pull-request head on origin - # (refs/pull/*/head — read via plain git, no API token needed) - requirePullRequest: false - # DEPRECATED: define a build with atTimes in the builds section instead - autoBuild: - enabled: false # whether to rebuild on schedule - times: ["01:00"] # UTC times HH:MM for scheduled builds - docker: - enabled: false # run clean/build commands in a Docker container instead of natively - image: "" # image for the build container; required when enabled - dockerfile: "" # Dockerfile to (re)build the image from when missing or stale; empty pulls the image as-is - context: "." # Docker build context used with dockerfile - network: "" # Docker network mode for the build container; empty = Docker default - env: {} # additional environment variables set inside the build container """.trimIndent() file.toFile().writeText(content + "\n") println("created ${file.toFile().relativeTo(normalizedWorkingDir.toFile())}") diff --git a/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt b/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt index 78c243a..0bc1180 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/BuildDefinition.kt @@ -6,8 +6,12 @@ import java.time.Instant /** * A named build (job) over the branches — ADR 0007. In YAML these live in the * top-level `builds` section next to the reserved execution key `maxConcurrent` - * (split apart by [ConfigLoader]); a build definition always comes from the repo - * install/project config, never from a build worktree. + * (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 @@ -42,7 +46,13 @@ data class BuildDefinition( val stdoutLog: String? = null, /** Overrides the branch's stderr log file name; null inherits it. */ val stderrLog: String? = null, - /** Overrides of the branch's docker image settings; the sandbox policy (`enabled`, `network`) is not overridable. */ + /** + * 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). */ @@ -78,11 +88,14 @@ data class BuildDefinition( 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, ), ) @@ -106,8 +119,12 @@ data class BuildDefinition( } } -/** Nullable docker image overrides of a [BuildDefinition]; null values inherit the branch's setting. */ +/** Nullable docker overrides of a [BuildDefinition]; null values inherit the branch's setting. */ data class DockerOverrides( + /** Run the build in a container instead of natively. Pinned — a branch must not escape its sandbox. */ + val enabled: Boolean? = null, + /** Docker network mode. Pinned — a branch must not change the sandbox's reachability. */ + val network: String? = null, val image: String? = null, val dockerfile: String? = null, val context: String? = null, diff --git a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt index 973b035..b891635 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt @@ -34,6 +34,9 @@ class ConfigLoader( /** Version warnings already reported; the config is loaded on every poll cycle, per branch. */ private val warnedVersions = ConcurrentHashMap.newKeySet() + /** Section-level warnings already reported, keyed by a fixed slug; the config is loaded on every poll cycle. */ + private val warnedSections = ConcurrentHashMap.newKeySet() + fun load(workingDir: Path = Paths.get(".")): GitTallyConfig = toConfig(loadRaw(workingDir)) /** @@ -84,7 +87,7 @@ class ConfigLoader( if (raw.isEmpty()) { GitTallyConfig() } else { - yaml.convertValue(mergeBranchDefaults(dropNonDefinitionBuilds(raw)), GitTallyConfig::class.java) + yaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw)), GitTallyConfig::class.java) } return defaultPublicBaseUrl(config) } @@ -118,7 +121,8 @@ class ConfigLoader( /** * Removes the keys a branch must never override: the secret and host-side top-level - * sections, the per-branch trust gate, and the docker sandbox policy. + * sections, the trust gate, and the docker sandbox policy — the latter two wherever + * they may appear, in a `builds` definition as well as in a legacy `branches` entry. * See [loadWithBranchLayer]. */ @Suppress("UNCHECKED_CAST") @@ -128,19 +132,19 @@ class ConfigLoader( } val result = branchLayer.toMutableMap() PINNED_TOP_LEVEL_KEYS.forEach { result.remove(it) } - val branches = result["branches"] as? Map - if (branches != null) { - result["branches"] = branches.mapValues { (_, value) -> stripPinnedBranchKeys(value) } + for (section in listOf("builds", "branches")) { + val entries = result[section] as? Map ?: continue + result[section] = entries.mapValues { (_, value) -> stripPinnedSettings(value) } } return result } @Suppress("UNCHECKED_CAST") - private fun stripPinnedBranchKeys(value: Any?): Any? { - val branch = value as? Map ?: return value - val result = branch.toMutableMap() - PINNED_BRANCH_KEYS.forEach { result.remove(it) } - val docker = branch["docker"] as? Map + private fun stripPinnedSettings(value: Any?): Any? { + val entry = value as? Map ?: return value + val result = entry.toMutableMap() + PINNED_SETTING_KEYS.forEach { result.remove(it) } + val docker = entry["docker"] as? Map if (docker != null) { val strippedDocker = docker.toMutableMap().apply { PINNED_DOCKER_KEYS.forEach { remove(it) } } if (strippedDocker.isEmpty()) result.remove("docker") else result["docker"] = strippedDocker @@ -148,6 +152,82 @@ class ConfigLoader( return result } + /** + * Decides which of the two sections describes the builds: `builds` or the legacy + * `branches`, never both. As soon as the merged configuration carries one real build + * definition — `builds.maxConcurrent` alone is not one, it is already dropped by + * [dropNonDefinitionBuilds] — a `branches` section is ignored altogether, because a + * definition now carries the complete description of its build and two half-answers + * would silently pull against each other. + * + * Deliberately decided on the *merged* map, after all layers are in: a branch that + * brings its own `builds` therefore also switches the host's `branches` off for its + * own builds, and — the reason this order matters — a build the branch defines and + * the host has never heard of still inherits the host's `builds.default`, sandbox + * policy included. Were the sections resolved per layer, that build would start with + * an empty docker policy and run natively on the host, which is exactly the escape + * the pinned keys exist to prevent. + */ + private fun resolveBuildSections(raw: Map): Map { + @Suppress("UNCHECKED_CAST") + val definitions = raw["builds"] as? Map ?: emptyMap() + if (definitions.isEmpty()) { + return mergeBranchDefaults(raw) + } + if (raw.containsKey("branches") && warnedSections.add(LEGACY_BRANCHES_WARNING)) { + log.warn( + "ignoring the branches section: this configuration defines builds, and a build definition " + + "carries its own settings; move what is still needed into builds — branches is going away", + ) + } + warnWhenNothingIsTriggered(definitions) + return mergeBuildDefaults(raw - "branches") + } + + /** + * An explicit `builds.default` replaces the implicit on-push build, so a set of + * definitions can end up with no trigger at all — an instance that will never build + * anything. That is a plausible intention for a moment and a mistake for a week, so + * it is said out loud once instead of being enforced. + */ + private fun warnWhenNothingIsTriggered(definitions: Map) { + if (BuildDefinition.DEFAULT !in definitions || definitions.values.any { isTriggered(it) }) { + return + } + if (warnedSections.add(NO_TRIGGER_WARNING)) { + log.warn("no build defines onPush or atTimes; the watcher will never start a build on its own") + } + } + + private fun isTriggered(definition: Any?): Boolean { + val entry = definition as? Map<*, *> ?: return false + return entry["onPush"] == true || (entry["atTimes"] as? List<*>)?.isNotEmpty() == true + } + + /** + * Applies `builds.default` as the base of every other build definition — the settings + * only. A trigger is never inherited: `onPush` and `atTimes` say when *this* build + * runs, and the selectors say for which branches, so inheriting them would make every + * job fire whenever the default one does. + */ + @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 + if (base.isEmpty()) { + return raw + } + val merged = + builds.mapValues { (name, value) -> + if (name == BuildDefinition.DEFAULT) { + value + } else { + deepMerge(base, value as? Map ?: emptyMap()) + } + } + return raw + ("builds" to merged) + } + /** Legacy default: an empty `server.publicBaseUrl` becomes `https:///`. */ private fun defaultPublicBaseUrl(config: GitTallyConfig): GitTallyConfig { if (config.server.publicBaseUrl.isNotBlank() || @@ -260,19 +340,28 @@ class ConfigLoader( * the credentials, report statuses to another repository, raise the global * concurrency, or turn off the pull-request gate for the whole watcher. * The `builds` section is deliberately *not* pinned: it describes what the branch - * builds, and a branch can already run any command via `branches.*.buildCommand`. + * builds, which is the branch's own business — only the individual settings in + * [PINNED_SETTING_KEYS] and [PINNED_DOCKER_KEYS] are taken out of it. */ private val PINNED_TOP_LEVEL_KEYS = setOf("git", "gitea", "server", "executor", "watcher") /** - * Per-branch keys a branch must never override: the trust gate that decides - * whether the watcher builds this branch at all. + * 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. */ - private val PINNED_BRANCH_KEYS = setOf("requirePullRequest") + private val PINNED_SETTING_KEYS = setOf("requirePullRequest") - /** Per-branch `docker` keys a branch must never override: the sandbox policy. */ + /** `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") + + private const val LEGACY_BRANCHES_WARNING = "legacy-branches-ignored" + + private const val NO_TRIGGER_WARNING = "no-build-triggered" + private const val ROLLBACK_HINT = "Migrate the file, or roll back to the GitTally version it was written for." diff --git a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt index 0e10ccc..bb5ce21 100644 --- a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt @@ -6,7 +6,6 @@ import de.hoennig.gittally.build.BuildExecutor import de.hoennig.gittally.build.BuildResultRepository import de.hoennig.gittally.build.BuildStatus import de.hoennig.gittally.build.GitWorktreeWorkspaces -import de.hoennig.gittally.config.BranchConfig import de.hoennig.gittally.config.BuildDefinition import de.hoennig.gittally.config.ConfigLoader import de.hoennig.gittally.config.DurationParser @@ -209,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).filterValues { it.onPush } + val onPush = definitionsFor(branch, heads[branch], workingDir, config).filterValues { it.onPush } for ((buildName, definition) in onPush) { if (selects(definition, branch, headCommitTimes)) { startBuildIfDue(branch, allowSameCommit = false, config, pullRequestHeads, workingDir, buildName) @@ -228,17 +227,20 @@ class Watcher( * their selectors are evaluated for it alone, so a definition committed on one branch * can never schedule builds of another. * - * Cached per branch by its head commit, so the `git show` runs only when the branch - * moved. An unreadable branch config falls back to the primary definitions instead of - * failing the poll cycle. + * Cached per branch by its head commit *and* the primary configuration it was merged + * with, so the `git show` runs only when the branch moved — but an edited machine or + * project config takes effect on the next poll instead of waiting for a commit that + * may never come. An unreadable branch config falls back to the primary definitions + * instead of failing the poll cycle. */ private fun definitionsFor( branch: String, headCommit: String?, workingDir: Path, + primary: GitTallyConfig, ): Map { - val commit = headCommit ?: return configLoader.load(workingDir).effectiveBuildDefinitions() - branchDefinitions[branch]?.takeIf { it.commit == commit }?.let { return it.definitions } + val commit = headCommit ?: return primary.effectiveBuildDefinitions() + branchDefinitions[branch]?.takeIf { it.commit == commit && it.primary == primary }?.let { return it.definitions } val definitions = try { configLoader @@ -254,12 +256,13 @@ class Watcher( ) configLoader.load(workingDir).effectiveBuildDefinitions() } - branchDefinitions[branch] = CachedDefinitions(commit, definitions) + branchDefinitions[branch] = CachedDefinitions(commit, primary, definitions) return definitions } private class CachedDefinitions( val commit: String, + val primary: GitTallyConfig, val definitions: Map, ) @@ -298,7 +301,7 @@ class Watcher( return false } if (config.watcher.pullRequestGate && - branchConfig(config, branch).requirePullRequest && + config.buildSettings(branch, build).requirePullRequest && commit !in pullRequestHeads.value ) { log.info("not enqueueing branch {}: no pull request has head commit {}", branch, commit) @@ -309,11 +312,6 @@ class Watcher( return true } - private fun branchConfig( - config: GitTallyConfig, - branch: String, - ): BranchConfig = config.branches[branch] ?: config.branches["default"] ?: BranchConfig() - /** * Fires the due `atTimes` slot of every build definition for its selected branches, * once per day and slot per result pool. Rebuilding the already-built commit is @@ -332,7 +330,8 @@ class Watcher( val today = LocalDate.ofInstant(now, ZoneOffset.UTC) val timeOfDay = LocalTime.ofInstant(now, ZoneOffset.UTC) for (branch in originBranches) { - val scheduled = definitionsFor(branch, heads[branch], workingDir).filterValues { it.atTimes.isNotEmpty() } + val scheduled = + definitionsFor(branch, heads[branch], workingDir, config).filterValues { it.atTimes.isNotEmpty() } for ((buildName, definition) in scheduled) { if (!selects(definition, branch, headCommitTimes)) { continue diff --git a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt index b8d8b54..3748dbf 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt @@ -223,11 +223,10 @@ class BuildExecutorTest : FunSpec() { val h = Harness( """ - branches: + builds: default: buildCommand: "echo regular-${'$'}branch" cleanCommand: "" - builds: pitest: buildCommand: "echo nightly-${'$'}branch" """.trimIndent(), diff --git a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt index fa4064b..aacb6d5 100644 --- a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt @@ -236,7 +236,49 @@ class ConfigLoaderTest : FunSpec() { val dir = Files.createTempDirectory("gittally-test") dir.resolve(".gittally.yml").toFile().writeText( """ - branches: + builds: + default: + requirePullRequest: true + docker: + enabled: true + network: none + image: host-image + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("gittally-test-worktree") + worktree.resolve(".gittally.yml").toFile().writeText( + """ + executor: + maxConcurrent: 99 + watcher: + pullRequestGate: false + builds: + default: + requirePullRequest: false + docker: + enabled: false + network: host + image: attacker-image + """.trimIndent(), + ) + + val config = loader.loadForWorktree(dir, worktree) + val settings = config.buildSettings("any-branch", "default") + + config.executor.maxConcurrent shouldBe 1 + config.watcher.pullRequestGate shouldBe true + settings.requirePullRequest shouldBe true + settings.docker.enabled shouldBe true + settings.docker.network shouldBe "none" + // everything that describes the build itself stays the branch's own business + settings.docker.image shouldBe "attacker-image" + } + + test("a build the branch invents inherits the host's sandbox policy") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + builds: default: requirePullRequest: true docker: @@ -247,38 +289,85 @@ class ConfigLoaderTest : FunSpec() { val worktree = Files.createTempDirectory("gittally-test-worktree") worktree.resolve(".gittally.yml").toFile().writeText( """ - executor: - maxConcurrent: 99 - watcher: - pullRequestGate: false - branches: - default: - requirePullRequest: false builds: - default: + invented: + atTimes: ["03:00"] + buildCommand: ./gradlew whatever docker: enabled: false network: host """.trimIndent(), ) - val config = loader.loadForWorktree(dir, worktree) - val branchConfig = config.branches.getValue("default") + // the host has never heard of this build, so there is no lower layer to fall + // back to — it must inherit the policy from builds.default, not the data class + val settings = loader.loadForWorktree(dir, worktree).buildSettings("any-branch", "invented") - config.executor.maxConcurrent shouldBe 1 - config.watcher.pullRequestGate shouldBe true - branchConfig.requirePullRequest shouldBe true - // a build definition has no enabled/network at all, so it cannot reintroduce them - config.buildDefinitions - .getValue("default") - .applyTo(branchConfig) - .docker - .enabled shouldBe true - config.buildDefinitions - .getValue("default") - .applyTo(branchConfig) - .docker - .network shouldBe "none" + settings.buildCommand shouldBe "./gradlew whatever" + settings.docker.enabled shouldBe true + settings.docker.network shouldBe "none" + settings.requirePullRequest shouldBe true + } + + 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"] + buildCommand: ./gradlew check + artifactDirs: [build/reports] + docker: + image: shared-image + nightly: + atTimes: ["01:00"] + artifactDirs: [build/reports, build/libs] + """.trimIndent(), + ) + + val nightly = loader.load(dir).buildDefinitions.getValue("nightly") + + nightly.buildCommand shouldBe "./gradlew check" + 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") + } + + test("branches is honored while no build is defined and ignored as soon as one is") { + val dir = Files.createTempDirectory("gittally-test") + val legacy = + """ + branches: + default: + buildCommand: from-branches + docker: + enabled: true + """.trimIndent() + dir.resolve(".gittally.yml").toFile().writeText(legacy) + + // the leftover execution key is not a definition, so the legacy section still wins + loader.load(dir).buildSettings("main", "default").buildCommand shouldBe "from-branches" + dir.resolve(".gittally.yml").toFile().writeText("builds:\n maxConcurrent: 1\n" + legacy) + loader.load(dir).buildSettings("main", "default").buildCommand shouldBe "from-branches" + + dir.resolve(".gittally.yml").toFile().writeText( + legacy + + "\n" + + """ + builds: + default: + buildCommand: from-builds + """.trimIndent(), + ) + + val settings = loader.load(dir).buildSettings("main", "default") + settings.buildCommand shouldBe "from-builds" + settings.docker.enabled shouldBe false } test("repo install config overrides project config for same keys") { @@ -482,7 +571,7 @@ class ConfigLoaderTest : FunSpec() { """ git: token: real-secret - branches: + builds: default: buildCommand: from-git """.trimIndent(), @@ -495,17 +584,16 @@ class ConfigLoaderTest : FunSpec() { git: token: stolen builds: + default: + buildCommand: from-branch pitest: atTimes: ["03:00"] buildCommand: ./gradlew piTestFull - branches: - default: - buildCommand: from-branch """.trimIndent(), ) config.git.token shouldBe "real-secret" - config.branches.getValue("default").buildCommand shouldBe "from-branch" + config.buildSettings("main", "default").buildCommand shouldBe "from-branch" config.buildDefinitions.getValue("pitest").atTimes shouldBe listOf("03:00") } diff --git a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt index 1355c21..d31f939 100644 --- a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt @@ -545,6 +545,29 @@ class WatcherTest : FunSpec() { verify(exactly = 1) { harness.gitService.showFileAtCommit("commit-2", Watcher.CONFIG_FILE, any()) } } + test("an edited primary config takes effect without the branch moving") { + val harness = Harness() + every { harness.gitService.originBranches(any()) } returns listOf("main") + every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-1") + every { harness.gitService.originHeadCommit("main", any()) } returns "commit-1" + + harness.watcher.poll(harness.workingDir) + harness.startedBuilds.shouldBeEmpty() + + // the machine config gains a scheduled build while the branch stays where it is: + // caching the definitions by head commit alone would never notice + val edited = + GitTallyConfig( + buildDefinitions = mapOf("nightly" to BuildDefinition(atTimes = listOf("11:00"))), + ) + every { harness.configLoader.load(any()) } returns edited + every { harness.configLoader.loadWithBranchLayer(any(), anyNullable()) } returns edited + + harness.watcher.poll(harness.workingDir) + + verify { harness.buildExecutor.startBuild("main", "commit-1", any(), "nightly") } + } + test("an unreadable branch config falls back to the primary definitions instead of failing the poll") { val harness = Harness() every { harness.gitService.originBranches(any()) } returns listOf("main")