Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions runway/extension/merger/git/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ go_library(
"//api/runway/messagequeue/protopb:go_default_library",
"//platform/base/change/git:go_default_library",
"//platform/base/change/github:go_default_library",
"//platform/extension/gitworkspace/local:go_default_library",
"//platform/git/exec:go_default_library",
"//platform/extension/gitworkspace:go_default_library",
"//platform/metrics:go_default_library",
"//runway/extension/merger:go_default_library",
"@com_github_uber_go_tally//:go_default_library",
Expand Down Expand Up @@ -46,6 +45,7 @@ go_test(
"//api/base/mergestrategy/protopb:go_default_library",
"//api/runway/messagequeue:go_default_library",
"//api/runway/messagequeue/protopb:go_default_library",
"//platform/extension/gitworkspace:go_default_library",
"//platform/extension/gitworkspace/local:go_default_library",
"//platform/git/exec:go_default_library",
"//platform/git/exectest:go_default_library",
Expand Down
12 changes: 5 additions & 7 deletions runway/extension/merger/git/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# git merger

A `merger.Merger` backed by the `git` CLI operating on a local checkout. It applies a merge request's ordered steps onto a target branch, honoring each step's strategy, and — for a committing merge — pushes the result. It is constructed by the wiring layer (see [`service/runway`](../../../../service/runway)) with the checkout path, remote, target branch, pinned git runtime, committer identity, and the default strategy; the request itself carries only a queue name and the changes, so the merger reads change URIs straight from the payload (no store, no resolver).
A `merger.Merger` that drives the `git` CLI through a [`gitworkspace.Workspace`](../../../../platform/extension/gitworkspace). It applies a merge request's ordered steps onto a target branch, honoring each step's strategy, and — for a committing merge — pushes the result. It is constructed by the wiring layer (see [`service/runway`](../../../../service/runway)) with a workspace factory, remote, target branch, and the default strategy; the request itself carries only a queue name and the changes, so the merger reads change URIs straight from the payload (no store, no resolver).

## Model

Expand Down Expand Up @@ -57,13 +57,11 @@ Fetching by SHA guarantees the merger applies exactly the commit a URI names —

## Authorship

Git records an author and a committer separately, and the merger keeps them apart: the committer is always the merger's configured identity, because it is what applied the change, while the author is the person who wrote it.
Git records an author and a committer separately, and the merger keeps them apart: the committer is always the workspace's configured identity, because it is what applied the change, while the author is the person who wrote it.

`REBASE` gets this for free — cherry-pick carries each commit's author across, so every landed commit keeps whoever wrote it. The strategies that mint a fresh commit do not: the squash commit and the `--no-ff` merge commit are new objects, and without attribution both would be credited to the service. Each is therefore authored as the author recorded on the commit its change's URI pins. For a change spanning several commits by different people, that is the head commit's author — the one identity the request actually names.

The author is read out of the local object store, so this costs no network and needs nothing on the wire: every referenced commit is already fetched and verified before a step is applied. A commit that records no usable author (either half missing) falls back to the committer identity rather than failing the merge.

The author travels to git through `GIT_AUTHOR_NAME`/`GIT_AUTHOR_EMAIL` rather than `git commit --author`, because `git merge` has no `--author` flag and because the environment keeps the name and address as separate values — a single `Name <address>` string has to be parsed back apart, which a display name containing an angle bracket breaks.
The author is read out of the local object store, so this costs no network and needs nothing on the wire: every referenced commit is already fetched and verified before a step is applied. The author travels as `git commit --author`, since a workspace command carries arguments only. `git merge` has no `--author` flag, so a `MERGE` step merges with `--no-commit` and then commits the merge with the author. A commit that records no usable author falls back to the committer identity rather than failing the merge: either half missing, or a name or address containing an angle bracket or line break, which `--author`'s single `Name <address>` string cannot carry.

## Importing an unrelated history

Expand All @@ -87,9 +85,9 @@ A merge conflict surfaces as `merger.ErrConflict`. An unusable request surfaces

The distinction between the last two matters operationally: a commit that is missing while the remote answers is a property of the request, whereas a remote that will not answer is a property of the moment.

## Runtime and identity
## Workspace, runtime and identity

Every git invocation uses the pinned runtime (explicit executable, exec-path, and template dir) and a scrubbed environment: no ambient configuration, no system or global git config, no interactive prompts. Because that leaves no ambient identity, the committer name and email are injected per-invocation, which the commit-creating strategies (`REBASE`, `SQUASH_REBASE`, `MERGE`) require.
The merger opens one workspace per operation and runs every git command in it, one command per batch. Where git runs, which git it is, and who it commits as belong to the workspace, not the merger. The reference wiring uses the local workspace over the configured checkout, with the pinned runtime (explicit executable, exec-path, and template dir) and a scrubbed environment: no ambient configuration, no system or global git config, no interactive prompts. Because that leaves no ambient identity, the workspace injects the committer name and email into every git command, which the commit-creating strategies (`REBASE`, `SQUASH_REBASE`, `MERGE`) require.

Scrubbing denies git ambient *configuration* — anything that could change what a merge produces. It deliberately does not deny it the means to reach the remote, so the agent socket, `PATH`, ssh-command, TLS and proxy variables are inherited when set. Without them an SSH remote cannot authenticate and git cannot even exec `ssh`; none of them can influence merge semantics. A deployment needing more can name additional variables on the runtime.

Expand Down
48 changes: 23 additions & 25 deletions runway/extension/merger/git/author.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,34 +36,32 @@ type authorIdent struct {
Email string
}

// complete reports whether both halves are present. A half-known identity is
// unusable — git wants a name and an address, and substituting a placeholder
// for the missing half would attribute the commit to someone who does not
// exist, which is worse than not attributing it at all.
func (a authorIdent) complete() bool {
return a.Name != "" && a.Email != ""
// usable reports whether the identity can be passed to `git commit --author`.
//
// A half-known identity is unusable — git wants a name and an address, and
// substituting a placeholder for the missing half would attribute the commit
// to someone who does not exist, which is worse than not attributing it at
// all. So is one containing an angle bracket or a line break: --author takes a
// single "Name <address>" string that git parses back apart, and such a value
// splits in the wrong place or makes git search the history for a matching
// author and fail the commit outright.
func (a authorIdent) usable() bool {
if a.Name == "" || a.Email == "" {
return false
}
return !strings.ContainsAny(a.Name+a.Email, "<>\n")
}

// env renders the identity as the environment git reads an author from. It
// returns nil for an incomplete identity, which leaves the configured committer
// identity to stand in for both roles — the behavior from before attribution
// existed.
//
// The identity travels as GIT_AUTHOR_* rather than as `git commit --author`
// for two reasons. `git merge` has no --author flag at all, so the merge
// strategy could not use one. And the environment carries name and address as
// two values, where --author takes a single "Name <address>" string that git
// has to parse back apart — a name containing an angle bracket splits in the
// wrong place, and one that parses to nothing makes git search the history for
// a matching author and fail the commit outright.
func (a authorIdent) env() []string {
if !a.complete() {
// args renders the identity as the `git commit` arguments that credit it. It
// returns nil for an unusable identity, which leaves the configured committer
// identity to stand in for both roles. The identity travels as an argument
// rather than through the environment because a workspace command carries
// arguments only.
func (a authorIdent) args() []string {
if !a.usable() {
return nil
}
return []string{
"GIT_AUTHOR_NAME=" + a.Name,
"GIT_AUTHOR_EMAIL=" + a.Email,
}
return []string{"--author", a.Name + " <" + a.Email + ">"}
}

// commitAuthor reads the author recorded on a commit.
Expand All @@ -80,7 +78,7 @@ func (m *gitMerger) commitAuthor(ctx context.Context, sha string) (authorIdent,
// NUL-separated: a display name may contain spaces, angle brackets, quotes
// or anything else a person put in their git config, and none of those can
// be mistaken for the field separator.
out, err := m.run(ctx, nil, "show", "--no-patch", "--format=%an%x00%ae", sha)
out, err := m.run(ctx, "show", "--no-patch", "--format=%an%x00%ae", sha)
if err != nil {
return authorIdent{}, fmt.Errorf("git show --no-patch --format author %s: %w", sha, err)
}
Expand Down
Loading