Skip to content

Commit f516d3c

Browse files
committed
feat(demo): read the git rung's change metadata from the repository
## Summary ### Why? The git rung merged for real and made up everything upstream of it. Because no change provider could read a plain git remote, `demo-queue` and `e2e-git-queue` both used the fake one, and `make demo-requests` compensated by writing the paths it had just committed onto the change URI (`sq-files=…`) for the fake to read back. That worked for changes the demo created and for nothing else. A branch pushed by hand carried no marker, so as far as the conflict analyzer could tell it touched nothing and conflicted with nothing — on the rung whose entire purpose is that the repository is real. It was also what pushed change URIs into the 255-byte storage limit and forced the paths to be budgeted down to one per directory. ### What? Both queues select the git change provider added in the previous two commits, and `gitSource` stops stating anything about what it touched. The orchestrator keeps its own copy of the same bare repository Runway merges into, points its own remote at it, and reads each change out of the commits. Each queue gets its own copy, because two copies at one path with different configuration would let whichever queue was built first decide what the other reads — which the config layer now rejects outright. **The orchestrator image gains git and a writable directory**, the same two things Runway's needed for the same reasons: git because the provider shells out to it, and `/var/submitqueue/changerepos` pre-created `0777` because Docker seeds a named volume from the image and the container's user is deployment-configurable, so a root-owned directory would leave a non-root service unable to provision. **The compose overlay mounts the sandbox into the orchestrator read-only** — it only ever fetches — plus a named volume for the copies. The volume rather than a bind mount for the reason Runway's checkout already is one: this is where git writes objects, and on macOS a freshly written loose object can read back as corrupt across the host filesystem bridge. The ladder table in the quickstart gains a "Read from" column. The rung's honesty was the point of the change and was not visible in the summary a reader skims. ## Test Plan Against a live `PROVIDER=git` stack, in order: - ✅ the container first, with the orchestrator still on `fake`: `git version 2.39.5`, `/srv/git` holding `sandbox.git`, and the repos directory `drwxrwxrwx` - ✅ **`FOLDERS=1`, six changes, no marker on any URI → a full dependency chain**, every batch depending on all the later ones. With `sq-files=` gone, the only way `pathoverlap` could see a shared directory is from paths the provider read out of the repository - ✅ **`FOLDERS=50`, five changes → no dependencies at all**, and they land in 3s rather than 10-14s. So it is genuinely keying on paths, not serializing everything - ✅ **the case that was impossible before**: two branches pushed by hand, carrying no marker, were described correctly — `{"author": {"name": "Hand", …}, "changed_files": [{"path": "shared/a.txt", "lines_added": 1, …}]}` read straight out of the `change` table - ✅ `make e2e-git-test` with both queues switched - ✅ `make test`, `make lint`, `make gazelle` **The risk flagged when planning this did not materialise.** `TestLand_ResubmittedAfterLanding_IsRejectedAsStale` asserts only that a resubmission errors, and the worry was that it would now error inside the provider — the head branch has moved, so the original SHA is no longer reachable from the ref — and pass while no longer testing staleness. Reproduced by hand: it still fails on `refs/heads/hand/one now points at 5b96c363…`, the staleness check. The provider's copy keeps the object it fetched the first time, so it resolves the commit locally and never refetches. Worth knowing about what the E2E does and does not buy: `e2e-git-queue` uses `analyzer: {type: none}`, so nothing there consumes the provider's output. Switching it proves the provider runs end to end without breaking a land; it does not check the metadata. That check is the `FOLDERS` runs above, which are hand-observed rather than asserted in CI.
1 parent 6f607c2 commit f516d3c

6 files changed

Lines changed: 71 additions & 22 deletions

File tree

doc/howto/QUICKSTART.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@ Start the stack, put traffic through it, and watch changes land — beginning wi
44

55
The stack always runs the same way. What changes is where the changes come from and what landing them does, chosen with `PROVIDER`:
66

7-
| `PROVIDER` | A change is | Building it | Landing it | Needs |
8-
|---|---|---|---|---|
9-
| **`fake`** (default) | a URI, and nothing else | instant fake pass | reports success without touching a repository | nothing |
10-
| **`git`** | a branch in a bare repository on disk | instant fake pass | a real fetch, cherry-pick and push | nothing |
11-
| **`github`** | a real pull request | a real GitHub Actions run per batch | a real push to a real repository | a repository, a token, and CI minutes |
7+
| `PROVIDER` | A change is | Read from | Building it | Landing it | Needs |
8+
|---|---|---|---|---|---|
9+
| **`fake`** (default) | a URI, and nothing else | the URI itself | instant fake pass | reports success without touching a repository | nothing |
10+
| **`git`** | a branch in a bare repository on disk | the repository | instant fake pass | a real fetch, cherry-pick and push | nothing |
11+
| **`github`** | a real pull request | GitHub's API | a real GitHub Actions run per batch | a real push to a real repository | a repository, a token, and CI minutes |
1212

1313
They are a ladder, not alternatives: the same commands work on each rung, so you can start with the one that needs nothing and only pay for what you want to see next. Each is a directory of configuration under [`service/submitqueue/demo/provider/`](../../service/submitqueue/demo/provider) — the difference between rungs is two YAML files, not a code path.
1414

15-
The queue's own logic is real on every rung; what changes is how much of the world around it is. Two things are worth knowing before reading a `landed` as more than it is. On `fake` and `git` **the build is faked**, so `landed` means the pipeline ran, not that anything was tested. And on `fake` and `git` the change provider is faked too: it cannot read a repository to see what a change touched, so `make demo-requests` states the paths on the change URI itself (`sq-files=`) for the conflict analyzer to key on. A change submitted by hand on those rungs touches nothing as far as the analyzer can tell, and conflicts with nothing.
15+
The queue's own logic is real on every rung; what changes is how much of the world around it is. The one thing to keep in mind before reading a `landed` as more than it is: on `fake` and `git` **the build is faked**, so it means the pipeline ran, not that anything was tested.
16+
17+
"Read from" is what the queue knows about a change — which files it touches, how large it is — and it is what conflict analysis and scoring are computed from. Only `fake` invents it: a change there is a URI pointing at nothing, so `make demo-requests` states the paths on the URI itself (`sq-files=`) and the fake reads them back, which means a change submitted by hand on that rung conflicts with nothing. On `git` the orchestrator keeps its own copy of the repository and reads the commits, so a change pushed by anyone is described correctly.
1618

1719
## Start the stack
1820

service/submitqueue/demo/provider/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ Pick one with `make local-submitqueue-start PROVIDER=<name>`, which bind-mounts
1414
| Directory | What it demonstrates | Needs |
1515
|---|---|---|
1616
| [`fake/`](fake) | the queue alone: a change is a URI, nothing merges anywhere — the default, and what the quickstart runs | nothing |
17-
| [`git/`](git) | a plain git remote with no provider at all: real fetch, cherry-pick and push against a bare repository | nothing |
17+
| [`git/`](git) | a plain git remote with no provider at all: change metadata read out of the repository, and a real fetch, cherry-pick and push against it | nothing |
1818
| [`github/`](github) | a live provider: GitHub change metadata, a real repository, pull requests marked merged | a repository and a token |
1919

20-
The three are a ladder, and the rung is the only thing that changes: the same commands land against all of them. `git/` is worth reading first of the two real ones. It is proof that the merge machinery has no provider in it — the same Runway code path lands changes against a bare repository addressed by path, with no credential and no API — and it is what the hermetic git E2E (`make e2e-git-test`) runs against.
20+
The three are a ladder, and the rung is the only thing that changes: the same commands land against all of them. `git/` is worth reading first of the two real ones. It is proof that neither half needs a provider — change metadata comes from reading the repository and the merge is the same Runway code path, both against a bare repository addressed by path, with no credential and no API — and it is what the hermetic git E2E (`make e2e-git-test`) runs against.
21+
22+
Note that the orchestrator and Runway each keep their **own** copy of a queue's repository and configure their own remote for it: the change provider's copy is in `profiles.yaml`, the merger's checkout in `merge.yaml`, and they are independent even when they name the same remote. That is the same shape a deployment has when both point at a remote host.
2123

2224
## Adding a provider
2325

service/submitqueue/demo/provider/git/profiles.yaml

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,32 @@ queues:
1717
# The queue `make demo-requests` and `make land` use by default. Serializes
1818
# batches that touch a shared directory, matching the github mode.
1919
#
20-
# The commits here are real, but the change provider above is not, and it
21-
# cannot read a repository to find out what they touched. `make demo-requests`
22-
# states the paths it committed on the change URI (`sq-files=`) for the fake
23-
# provider to report back. A change submitted by hand carries no such marker
24-
# and so conflicts with nothing.
20+
# Both halves are real here. The change provider keeps its own copy of the
21+
# same bare repository Runway merges into and reads each change out of it, so
22+
# the analyzer keys on paths that were actually committed — including for a
23+
# change submitted by hand, which nothing else could have described.
2524
- name: demo-queue
25+
changeProvider:
26+
type: git
27+
git:
28+
# The orchestrator's own copy, and its own remote pointing at the
29+
# sandbox — mounted read-only, since it only ever fetches. Runway
30+
# configures the same repository separately for itself in merge.yaml.
31+
remoteUrl: file:///srv/git/sandbox.git
32+
target: main
33+
repoPath: /var/submitqueue/changerepos/demo
2634
analyzer: {type: pathoverlap, by: directory}
2735

2836
- name: e2e-git-queue
37+
# Its own copy, separate from demo-queue's: the two agree about the
38+
# repository but not about everything else, and a shared path would make
39+
# whichever was built first decide for both.
40+
changeProvider:
41+
type: git
42+
git:
43+
remoteUrl: file:///srv/git/sandbox.git
44+
target: main
45+
repoPath: /var/submitqueue/changerepos/sandbox
2946
# Maximum parallelism: batches never conflict, so the test controls
3047
# ordering through what it lands rather than through the analyzer.
3148
analyzer: {type: none}

service/submitqueue/demo/requests/git.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,15 @@ func (s *gitSource) open(ctx context.Context, spec changeSpec) (openedChange, er
136136

137137
return openedChange{
138138
headSHA: headSHA,
139-
// The commits are real, but the fake change provider is what the
140-
// orchestrator asks about them, and it cannot read a repository — so the
141-
// paths just committed are stated on the URI for it to report back.
142-
uri: withFiles(gitchange.ChangeID{
139+
// Nothing is stated about what this change touches. The orchestrator
140+
// keeps its own copy of this repository and reads that out of the
141+
// commits, which is the whole difference between this rung and the fake
142+
// one — and what makes a change pushed by hand behave the same as one
143+
// from here.
144+
uri: gitchange.ChangeID{
143145
Scheme: "git", Remote: gitRemote, Repo: s.repo,
144146
Ref: "refs/heads/" + spec.branch, CommitSHA: headSHA,
145-
}.String(), spec.files),
147+
}.String(),
146148
// No pull request to number, so the branch names the change. Empty URL:
147149
// a branch in a bare repository has nothing to open.
148150
cell: client.Cell{Text: spec.branch},

service/submitqueue/docker-compose.git.yml

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,30 @@
1919
# extensions (service/submitqueue/demo/provider/git)
2020
# SQ_GIT_SANDBOX_DIR the bare repository the merger fetches and pushes
2121
# SQ_RUNWAY_CHECKOUT_DIR storage for the working trees the merger owns
22+
#
23+
# Optional:
24+
# SQ_ORCHESTRATOR_REPO_DIR storage for the orchestrator's own repository
25+
# copies; a named volume when unset
2226

2327
services:
2428
orchestrator-service:
2529
environment:
2630
# Which change provider, build runner, and conflict analyzer each queue
27-
# resolves to. Everything at the edges stays fake here; only the merge is
28-
# real.
31+
# resolves to. The build runner stays fake here; the change provider and
32+
# the merge are real.
2933
- PROFILES_CONFIG_PATH=/etc/submitqueue/profiles.yaml
3034
volumes:
3135
- ${SQ_PROVIDER_CONFIG_DIR}:/etc/submitqueue:ro
36+
# The same bare repository Runway merges into, read-only: the orchestrator
37+
# only ever fetches from it. Each service keeps its own copy and points
38+
# its own remote at this one, which is the arrangement a real deployment
39+
# has with both of them pointing at a remote host.
40+
- ${SQ_GIT_SANDBOX_DIR}:/srv/git:ro
41+
# Where the orchestrator keeps its copies. A named volume rather than a
42+
# bind mount for the same reason Runway's checkout is one: this is where
43+
# git writes objects, and on macOS a freshly written loose object can read
44+
# back as corrupt over the host filesystem bridge.
45+
- ${SQ_ORCHESTRATOR_REPO_DIR:-orchestrator-changerepos}:/var/submitqueue/changerepos
3246

3347
runway-service:
3448
environment:
@@ -55,3 +69,4 @@ services:
5569

5670
volumes:
5771
runway-checkouts:
72+
orchestrator-changerepos:

service/submitqueue/orchestrator/server/Dockerfile

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
FROM debian:bookworm-slim
22

3-
RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/lists/* \
4-
&& mkdir -p /app && chmod 0755 /app
3+
# git is a runtime dependency, not a build one: the git change provider shells
4+
# out to it to read what a change touched. Without it the service still starts,
5+
# but a queue configured with that provider fails at startup instead.
6+
RUN apt-get update && apt-get install -y ca-certificates git && rm -rf /var/lib/apt/lists/* \
7+
&& mkdir -p /app && chmod 0755 /app \
8+
# The repository copies the git change provider owns. Created here so that a
9+
# named volume mounted over it starts with a mode the service can write:
10+
# Docker seeds an empty volume from the image, and the container's user is
11+
# deployment-configurable (SQ_CONTAINER_USER), so a root-owned directory
12+
# would leave a non-root service unable to provision. Group- and
13+
# world-writable for that reason, which costs nothing on a path that exists
14+
# to be mounted over.
15+
&& mkdir -p /var/submitqueue/changerepos && chmod 0777 /var/submitqueue/changerepos
516
WORKDIR /app
617

718
# Copy pre-built Linux binary

0 commit comments

Comments
 (0)