Skip to content

Commit 8689cf9

Browse files
committed
refactor(merger): build the git command env from the shared composer
Replace the merger's inline command environment with gitexec.Env: the isolated HOME/XDG and the pinned runtime paths stay as literals, while the scrub set and the transport variables now come from platform/git/exec, the one source of truth every git caller shares. Delete the merger's own authEnvNames and passthroughEnv. No control-flow change — the merger still owns its working-tree flow and GitRuntime; only how each command's environment is assembled moves to the shared composer.
1 parent a3e2fd7 commit 8689cf9

2 files changed

Lines changed: 18 additions & 68 deletions

File tree

runway/extension/merger/git/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"//api/runway/messagequeue/protopb:go_default_library",
1818
"//platform/base/change/git:go_default_library",
1919
"//platform/base/change/github:go_default_library",
20+
"//platform/git/exec:go_default_library",
2021
"//platform/metrics:go_default_library",
2122
"//runway/extension/merger:go_default_library",
2223
"@com_github_uber_go_tally//:go_default_library",

runway/extension/merger/git/git_merger.go

Lines changed: 17 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ import (
7777
"context"
7878
"errors"
7979
"fmt"
80-
"os"
8180
"os/exec"
8281
"path/filepath"
8382
"strings"
@@ -89,6 +88,7 @@ import (
8988
mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
9089
runwaymq "github.com/uber/submitqueue/api/runway/messagequeue"
9190
runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb"
91+
gitexec "github.com/uber/submitqueue/platform/git/exec"
9292
coremetrics "github.com/uber/submitqueue/platform/metrics"
9393
"github.com/uber/submitqueue/runway/extension/merger"
9494
)
@@ -1094,76 +1094,25 @@ func newGitCommand(ctx context.Context, runtime GitRuntime, dir string, args ...
10941094

10951095
cmd := exec.CommandContext(ctx, runtime.Executable, gitArgs...)
10961096
cmd.Dir = dir
1097-
cmd.Env = []string{
1098-
"HOME=" + filepath.Join(dir, ".submitqueue-git-home"),
1099-
"XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"),
1100-
"GIT_CONFIG_NOSYSTEM=1",
1101-
"GIT_CONFIG_GLOBAL=" + os.DevNull,
1102-
"GIT_ATTR_NOSYSTEM=1",
1103-
"GIT_TERMINAL_PROMPT=0",
1104-
"GIT_PAGER=cat",
1105-
"GIT_EDITOR=:",
1106-
"GIT_EXEC_PATH=" + runtime.ExecPath,
1107-
"GIT_TEMPLATE_DIR=" + runtime.TemplateDir,
1108-
"LC_ALL=C",
1109-
"LANG=C",
1110-
}
1111-
cmd.Env = append(cmd.Env, passthroughEnv(runtime.PassthroughEnv)...)
1097+
// HOME and XDG_CONFIG_HOME are isolated to the checkout rather than inherited,
1098+
// so the runtime's literals — appended last — override any HOME a deployment
1099+
// passed through. The remaining literals pin git's runtime; the scrub set and
1100+
// transport variables come from the shared composer.
1101+
cmd.Env = gitexec.Env(gitexec.EnvOptions{
1102+
Transport: true,
1103+
Passthrough: runtime.PassthroughEnv,
1104+
Literal: []string{
1105+
"HOME=" + filepath.Join(dir, ".submitqueue-git-home"),
1106+
"XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"),
1107+
"GIT_EXEC_PATH=" + runtime.ExecPath,
1108+
"GIT_TEMPLATE_DIR=" + runtime.TemplateDir,
1109+
"LC_ALL=C",
1110+
"LANG=C",
1111+
},
1112+
})
11121113
return cmd
11131114
}
11141115

1115-
// authEnvNames are the variables inherited from the parent process when set.
1116-
//
1117-
// Scrubbing the environment is about denying git ambient *configuration* that
1118-
// could change what a merge produces. Reaching the remote is a separate
1119-
// concern, and these carry it: an SSH remote authenticates through the agent
1120-
// socket, git locates its ssh and credential helpers through PATH, and TLS and
1121-
// proxy settings decide whether an HTTPS remote is reachable at all. Dropping
1122-
// them does not make the merge more hermetic, it just makes fetch and push
1123-
// fail — and at Uber, where a uSSH certificate lives in the agent, it fails as
1124-
// an opaque authentication error rather than an obviously missing variable.
1125-
//
1126-
// None of these can influence merge semantics, which is what keeps them
1127-
// compatible with the scrubbing above.
1128-
var authEnvNames = []string{
1129-
"SSH_AUTH_SOCK",
1130-
"SSH_AGENT_PID",
1131-
"PATH",
1132-
"GIT_SSH",
1133-
"GIT_SSH_COMMAND",
1134-
"GIT_SSH_VARIANT",
1135-
"GIT_SSL_CAINFO",
1136-
"GIT_SSL_CAPATH",
1137-
"SSL_CERT_DIR",
1138-
"SSL_CERT_FILE",
1139-
"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY",
1140-
"http_proxy", "https_proxy", "no_proxy",
1141-
}
1142-
1143-
// passthroughEnv returns "NAME=value" entries for the auth and transport
1144-
// variables that are actually set, plus any extra names the deployment asked
1145-
// for. An unset variable is omitted rather than exported empty, which for
1146-
// SSH_AUTH_SOCK is the difference between "use the agent" and "there is no
1147-
// agent".
1148-
func passthroughEnv(extra []string) []string {
1149-
names := make([]string, 0, len(authEnvNames)+len(extra))
1150-
names = append(names, authEnvNames...)
1151-
names = append(names, extra...)
1152-
1153-
seen := make(map[string]bool, len(names))
1154-
env := make([]string, 0, len(names))
1155-
for _, name := range names {
1156-
if name == "" || seen[name] {
1157-
continue
1158-
}
1159-
seen[name] = true
1160-
if v, ok := os.LookupEnv(name); ok {
1161-
env = append(env, name+"="+v)
1162-
}
1163-
}
1164-
return env
1165-
}
1166-
11671116
// isConcreteStrategy reports whether s names a concrete integration strategy
11681117
// (i.e. not DEFAULT and not an unknown value).
11691118
func isConcreteStrategy(s mergestrategypb.Strategy) bool {

0 commit comments

Comments
 (0)