Skip to content

Commit caef1d1

Browse files
jtmcgjtmcg
committed
Create GHBranchConfig: a gh-domain specific branch config
It has been frustrating working with BranchConfig, as it lives halfway between being a git-specific config and a gh specific config. This commit attempts to remedy that, boiling the GHBranchConfig down to what we really care about in gh: the MergeRef and the PushRef. The MergeRef determines where the branch pulls from when `git pull` is invoked. The PushRef determines where the branch pushes to when `git push` is invoked. In practice, these two refs are a combination of repo and branch specific config options. Because gh is context aware, we can determine these successfully even when git cannot. Thus, parseGHBranchConfig builds the two refs from the available information from the various git config values and from the branch that gh is aware of. Why we care is that PRs for a branch are created from a branch's PushRef: i.e. where the changes are living upstream. This is typically an easy thing to resolve with `git rev-parse --abbrev-ref <branch>@{push}`. However, there are several use-cases solved for here in which this is not sufficient, revolving around triangular workflows. Additionally, the merge and push refs in GHBranchConfig are broken into two parts: the Remote and the Branch. This is because the Remote can either be given as an name (commonly "origin" or "upstream") or a URL string. Downstream consumers of this will need to handle the name or URL differently to resolve GitHub operations, so the struct is separated for ease of access to this Remote component.
1 parent a6cd5ee commit caef1d1

File tree

3 files changed

+286
-0
lines changed

3 files changed

+286
-0
lines changed

‎git/client.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,85 @@ func parseBranchConfig(branchConfigLines []string, remotePushDefault string, rev
469469
return cfg
470470
}
471471

472+
// ReadGHBranchConfig parses the git config to determine the merge and push remotes for the branch
473+
// as well as the merge base branch.
474+
func (c *Client) ReadGHBranchConfig(ctx context.Context, branch string) (GHBranchConfig, error) {
475+
476+
prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
477+
args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|pushremote|%s)$", prefix, MergeBaseConfig)}
478+
cmd, err := c.Command(ctx, args...)
479+
if err != nil {
480+
return GHBranchConfig{}, err
481+
}
482+
483+
// This is the error we expect if a git command does not run successfully.
484+
// If the ExitCode is 1, then we just didn't find any config for the branch.
485+
// We will use this error to check against the commands that are allowed to
486+
// return an empty result.
487+
var gitError *GitError
488+
branchCfgOut, err := cmd.Output()
489+
if err != nil {
490+
if ok := errors.As(err, &gitError); ok && gitError.ExitCode != 1 {
491+
return GHBranchConfig{}, err
492+
}
493+
return GHBranchConfig{}, nil
494+
}
495+
496+
// Check to see if there is a pushDefault ref set for the repo
497+
remotePushDefaultOut, err := c.Config(ctx, "remote.pushDefault")
498+
if ok := errors.As(err, &gitError); ok && gitError.ExitCode != 1 {
499+
return GHBranchConfig{}, err
500+
}
501+
502+
// Check to see if we can resolve the @{push} revision syntax. This is the easiest way to get
503+
// the name of the push remote.
504+
//We ignore errors resolving simple push.Default settings as these are handled downstream
505+
revParseOut, _ := c.revParse(ctx, "--verify", "--quiet", "--abbrev-ref", branch+"@{push}")
506+
507+
return parseGHBranchConfig(branch, outputLines(branchCfgOut), strings.TrimSuffix(remotePushDefaultOut, "\n"), firstLine(revParseOut)), nil
508+
}
509+
510+
func parseGHBranchConfig(branch string, branchConfigLines []string, remotePushDefault string, revParse string) GHBranchConfig {
511+
var ghBranchConfig GHBranchConfig
512+
ghBranchConfig.Push.Branch = branch
513+
514+
var pushRemote string
515+
for _, line := range branchConfigLines {
516+
parts := strings.SplitN(line, " ", 2)
517+
if len(parts) < 2 {
518+
continue
519+
}
520+
keys := strings.Split(parts[0], ".")
521+
switch keys[len(keys)-1] {
522+
case "remote":
523+
ghBranchConfig.Push.Remote = parts[1]
524+
ghBranchConfig.Merge.Remote = parts[1]
525+
case "merge":
526+
mergeBranch := strings.TrimPrefix(parts[1], "refs/heads/")
527+
ghBranchConfig.Merge.Branch = mergeBranch
528+
case "pushremote":
529+
pushRemote = strings.TrimPrefix(parts[1], "refs/remotes/")
530+
// TODO: handle gh-merge-base
531+
}
532+
}
533+
534+
if pushRemote != "" {
535+
ghBranchConfig.Push.Remote = pushRemote
536+
}
537+
538+
if revParse != "" {
539+
revParseParts := strings.Split(revParse, "/")
540+
ghBranchConfig.Push.Remote = revParseParts[0]
541+
ghBranchConfig.Push.Branch = revParseParts[1]
542+
}
543+
544+
if remotePushDefault != "" {
545+
ghBranchConfig.Push.Remote = remotePushDefault
546+
}
547+
548+
return ghBranchConfig
549+
}
550+
472551
// SetBranchConfig sets the named value on the given branch.
473552
func (c *Client) SetBranchConfig(ctx context.Context, branch, name, value string) error {
474553
name = fmt.Sprintf("branch.%s.%s", branch, name)

‎git/client_test.go‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,203 @@ func Test_parseBranchConfig(t *testing.T) {
11451145
}
11461146
}
11471147

1148+
// TODO: Implement these tests
1149+
// func TestClientReadGHBranchConfig(t *testing.T) {
1150+
// tests := []struct {
1151+
// name string
1152+
// cmds mockedCommands
1153+
// branch string
1154+
// wantGHBranchConfig GHBranchConfig
1155+
// wantError *GitError
1156+
// }{
1157+
// {
1158+
// name: "centralized workflow",
1159+
// cmds: mockedCommands{
1160+
// `path/to/git config --get-regexp ^branch\.feature-branch\.(remote|merge|pushremote|gh-merge-base)$`: {
1161+
// Stdout: "branch.feature-branch.remote origin\nbranch.feature-branch.merge refs/heads/feature-branch\n",
1162+
// },
1163+
// `path/to/git config remote.pushDefault`: {
1164+
// Stdout: "simple",
1165+
// },
1166+
// `path/to/git rev-parse --verify --quiet --abbrev-ref trunk@{push}`: {
1167+
// Stdout: "origin/feature-branch",
1168+
// },
1169+
// },
1170+
// branch: "trunk",
1171+
// wantGHBranchConfig: GHBranchConfig{
1172+
// Merge: RemoteRef{
1173+
// Remote: "origin",
1174+
// Branch: "feature-branch",
1175+
// },
1176+
// Push: RemoteRef{
1177+
// Remote: "origin",
1178+
// Branch: "feature-branch",
1179+
// },
1180+
// },
1181+
// wantError: nil,
1182+
// },
1183+
// }
1184+
// for _, tt := range tests {
1185+
// t.Run(tt.name, func(t *testing.T) {
1186+
// cmdCtx := createMockedCommandContext(t, tt.cmds)
1187+
// client := Client{
1188+
// GitPath: "path/to/git",
1189+
// commandContext: cmdCtx,
1190+
// }
1191+
// ghBranchConfig, err := client.ReadGHBranchConfig(context.Background(), tt.branch)
1192+
// if tt.wantError != nil {
1193+
// var gitError *GitError
1194+
// require.ErrorAs(t, err, &gitError)
1195+
// assert.Equal(t, tt.wantError.ExitCode, gitError.ExitCode)
1196+
// assert.Equal(t, tt.wantError.Stderr, gitError.Stderr)
1197+
// } else {
1198+
// require.NoError(t, err)
1199+
// }
1200+
// assert.Equal(t, tt.wantGHBranchConfig, ghBranchConfig)
1201+
// })
1202+
// }
1203+
// }
1204+
1205+
func Test_parseGHBranchConfig(t *testing.T) {
1206+
tests := []struct {
1207+
name string
1208+
branch string
1209+
branchConfigLines []string
1210+
remotePushDefault string
1211+
revParse string
1212+
wantGHBranchConfig GHBranchConfig
1213+
}{
1214+
{
1215+
name: "when the branch is named feature-branch and in a centralized workflow with a resolved rev-parse, it should return the correct GHBranchConfig",
1216+
branch: "feature-branch",
1217+
branchConfigLines: []string{
1218+
"branch.feature-branch.remote origin",
1219+
"branch.feature-branch.merge refs/heads/feature-branch",
1220+
},
1221+
remotePushDefault: "",
1222+
revParse: "origin/feature-branch",
1223+
wantGHBranchConfig: GHBranchConfig{
1224+
Merge: RemoteRef{
1225+
Remote: "origin",
1226+
Branch: "feature-branch",
1227+
},
1228+
Push: RemoteRef{
1229+
Remote: "origin",
1230+
Branch: "feature-branch",
1231+
},
1232+
},
1233+
},
1234+
{
1235+
name: "when the branch is named other-branch and in a centralized workflow with a resolved rev-parse, it should return the correct GHBranchConfig",
1236+
branch: "other-branch",
1237+
branchConfigLines: []string{
1238+
"branch.other-branch.remote origin",
1239+
"branch.other-branch.merge refs/heads/other-branch",
1240+
},
1241+
remotePushDefault: "",
1242+
revParse: "origin/other-branch",
1243+
wantGHBranchConfig: GHBranchConfig{
1244+
Merge: RemoteRef{
1245+
Remote: "origin",
1246+
Branch: "other-branch",
1247+
},
1248+
Push: RemoteRef{
1249+
Remote: "origin",
1250+
Branch: "other-branch",
1251+
},
1252+
},
1253+
},
1254+
{
1255+
name: "when the branch is named feature-branch and in a triangular workflow with the main, no remote.pushdefault set, and rev-parse is unresolved, it should return the correct GHBranchConfig",
1256+
branch: "feature-branch",
1257+
branchConfigLines: []string{
1258+
"branch.feature-branch.remote origin",
1259+
"branch.feature-branch.merge refs/heads/main",
1260+
},
1261+
remotePushDefault: "",
1262+
revParse: "",
1263+
wantGHBranchConfig: GHBranchConfig{
1264+
Merge: RemoteRef{
1265+
Remote: "origin",
1266+
Branch: "main",
1267+
},
1268+
Push: RemoteRef{
1269+
Remote: "origin",
1270+
Branch: "feature-branch",
1271+
},
1272+
},
1273+
},
1274+
{
1275+
name: "when the branch is named feature-branch from a fork called origin and in a triangular workflow with the Fork's parent repo called upstream, remote.pushdefault is set, and rev-parse is unresolved, it should return the correct GHBranchConfig",
1276+
branch: "feature-branch",
1277+
branchConfigLines: []string{
1278+
"branch.feature-branch.remote upstream",
1279+
"branch.feature-branch.merge refs/heads/main",
1280+
},
1281+
remotePushDefault: "origin",
1282+
revParse: "",
1283+
wantGHBranchConfig: GHBranchConfig{
1284+
Merge: RemoteRef{
1285+
Remote: "upstream",
1286+
Branch: "main",
1287+
},
1288+
Push: RemoteRef{
1289+
Remote: "origin",
1290+
Branch: "feature-branch",
1291+
},
1292+
},
1293+
},
1294+
{
1295+
name: "when the branch is named feature-branch from a fork called origin and in a triangular workflow with the Fork's parent repo called upstream, pushdefault is set on the branch, and rev-parse is unresolved, it should return the correct GHBranchConfig",
1296+
branch: "feature-branch",
1297+
branchConfigLines: []string{
1298+
"branch.feature-branch.remote upstream",
1299+
"branch.feature-branch.merge refs/heads/main",
1300+
"branch.feature-branch.pushremote origin",
1301+
},
1302+
remotePushDefault: "",
1303+
revParse: "",
1304+
wantGHBranchConfig: GHBranchConfig{
1305+
Merge: RemoteRef{
1306+
Remote: "upstream",
1307+
Branch: "main",
1308+
},
1309+
Push: RemoteRef{
1310+
Remote: "origin",
1311+
Branch: "feature-branch",
1312+
},
1313+
},
1314+
},
1315+
{
1316+
name: "when the branch is named feature-branch from a fork called origin and in a triangular workflow with the Fork's parent repo called upstream, pushdefault is set on the branch, rev-parse is unresolved, and the branch config is in a different order, it should return the correct GHBranchConfig",
1317+
branch: "feature-branch",
1318+
branchConfigLines: []string{
1319+
"branch.feature-branch.pushremote origin",
1320+
"branch.feature-branch.remote upstream",
1321+
"branch.feature-branch.merge refs/heads/main",
1322+
},
1323+
remotePushDefault: "",
1324+
revParse: "",
1325+
wantGHBranchConfig: GHBranchConfig{
1326+
Merge: RemoteRef{
1327+
Remote: "upstream",
1328+
Branch: "main",
1329+
},
1330+
Push: RemoteRef{
1331+
Remote: "origin",
1332+
Branch: "feature-branch",
1333+
},
1334+
},
1335+
},
1336+
}
1337+
for _, tt := range tests {
1338+
t.Run(tt.name, func(t *testing.T) {
1339+
ghBranchConfig := parseGHBranchConfig(tt.branch, tt.branchConfigLines, tt.remotePushDefault, tt.revParse)
1340+
assert.Equal(t, tt.wantGHBranchConfig, ghBranchConfig)
1341+
})
1342+
}
1343+
}
1344+
11481345
func Test_parseRemoteURLOrName(t *testing.T) {
11491346
tests := []struct {
11501347
name string

‎git/objects.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,13 @@ type BranchConfig struct {
7171
PushRemoteName string
7272
Push string
7373
}
74+
75+
type GHBranchConfig struct {
76+
Merge RemoteRef
77+
Push RemoteRef
78+
}
79+
80+
type RemoteRef struct {
81+
Remote string
82+
Branch string
83+
}

0 commit comments

Comments
 (0)