STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Moved remote.pushDefault out of ReadBranchConfig and into finder

Browse source
This commit is contained in:
Tyler McGoffin 2025-01-24 11:05:15 -08:00
parent e4d8ed0e60
commit cdead50d57
5 changed files with 223 additions and 277 deletions
Download patch file Download diff file Expand all files Collapse all files

46
git/client.go
View file

@ -402,16 +402,10 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (BranchCon
return BranchConfig{}, nil
}
// Check to see if there is a pushDefault ref set for the repo
remotePushDefaultOut, err := c.Config(ctx, "remote.pushDefault")
if ok := errors.As(err, &gitError); ok && gitError.ExitCode != 1 {
return BranchConfig{}, err
}
return parseBranchConfig(outputLines(branchCfgOut), strings.TrimSuffix(remotePushDefaultOut, "\n")), nil
return parseBranchConfig(outputLines(branchCfgOut)), nil
}
func parseBranchConfig(branchConfigLines []string, remotePushDefault string) BranchConfig {
func parseBranchConfig(branchConfigLines []string) BranchConfig {
var cfg BranchConfig
// Read the config lines for the specific branch
@ -438,26 +432,6 @@ func parseBranchConfig(branchConfigLines []string, remotePushDefault string) Bra
}
}
// PushRemote{URL|Name} takes precedence over remotePushDefault, so we'll only
// use remotePushDefault if we don't have a push remote.
if cfg.PushRemoteURL == nil && cfg.PushRemoteName == "" {
// remotePushDefault usually indicates a "triangular" workflow
if remotePushDefault != "" {
pushRemoteURL, pushRemoteName := parseRemoteURLOrName(remotePushDefault)
cfg.PushRemoteURL = pushRemoteURL
cfg.PushRemoteName = pushRemoteName
} else {
// Without a PushRemote{URL|Name} or a remotePushDefault, we assume that the
// push remote ref is the same as the remote ref. This is likely
// a "centralized" workflow.
cfg.PushRemoteName = cfg.RemoteName
cfg.PushRemoteURL = cfg.RemoteURL
}
}
// Some `gh pr` workflows don't work if this is set to 'simple' or 'current'
cfg.RemotePushDefault = remotePushDefault
return cfg
}
@ -489,6 +463,22 @@ func (c *Client) PushDefault(ctx context.Context) (string, error) {
return "", err
}
// RemotePushDefault returns the value of remote.pushDefault in the config. If
// the value is not set, it returns an empty string.
func (c *Client) RemotePushDefault(ctx context.Context) (string, error) {
remotePushDefault, err := c.Config(ctx, "remote.pushDefault")
if err == nil {
return remotePushDefault, nil
}
var gitError *GitError
if ok := errors.As(err, &gitError); ok && gitError.ExitCode == 1 {
return "", nil
}
return "", err
}
// ParsePushRevision gets the value of the @{push} revision syntax
// An error here doesn't necessarily mean something are broke, but may mean that the @{push}
// revision syntax couldn't be resolved, such as in non-centralized workflows with

308
git/client_test.go
View file

@ -760,199 +760,20 @@ func TestClientReadBranchConfig(t *testing.T) {
},
},
{
name: "when the git reads the config, pushDefault isn't set, it should return the correct BranchConfig",
name: "when the config is read, it should return the correct BranchConfig",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\n",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 1,
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
PushRemoteName: "origin",
},
wantError: nil,
},
{
name: "when the git reads the config, pushDefault is set, it should return the correct BranchConfig",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\n",
},
`path/to/git config remote.pushDefault`: {
Stdout: "remotePushDefault",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
PushRemoteName: "remotePushDefault",
RemotePushDefault: "remotePushDefault",
},
wantError: nil,
},
{
name: "when git reads the config, pushDefault is set, an the branch hasn't been pushed, it should return the correct BranchConfig",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\n",
},
`path/to/git config remote.pushDefault`: {
Stdout: "remotePushDefault",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
PushRemoteName: "remotePushDefault",
RemotePushDefault: "remotePushDefault",
},
wantError: nil,
},
{
name: "when git reads the config but remotePushDefault fails, it should return and empty BranchConfig and the error",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\n",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 2,
Stderr: "remotePushDefault error",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{},
wantError: &GitError{
ExitCode: 2,
Stderr: "remotePushDefault error",
},
},
{
name: "read branch config, central",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base merge-base",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 1,
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
MergeRef: "refs/heads/trunk",
MergeBase: "merge-base",
PushRemoteName: "origin",
},
wantError: nil,
},
{
name: "read branch config, central, push.default = upstream",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk-remote",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 1,
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
MergeRef: "refs/heads/trunk-remote",
PushRemoteName: "origin",
},
},
{
name: "read branch config, central, push.default = upstream, no existing remote branch",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/main",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 1,
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "origin",
MergeRef: "refs/heads/main",
PushRemoteName: "origin",
},
},
{
name: "read branch config, triangular, push.default = current, has existing remote branch, branch.trunk.pushremote effective",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/main\nbranch.trunk.pushremote origin",
},
`path/to/git config remote.pushDefault`: {
ExitStatus: 1,
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.pushremote origin\nbranch.trunk.gh-merge-base gh-merge-base\n",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "upstream",
MergeRef: "refs/heads/main",
PushRemoteName: "origin",
MergeRef: "refs/heads/trunk",
MergeBase: "gh-merge-base",
},
},
{
name: "read branch config, triangular, push.default = current, has existing remote branch, remote.pushDefault effective",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/main",
},
`path/to/git config remote.pushDefault`: {
Stdout: "origin",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "upstream",
MergeRef: "refs/heads/main",
PushRemoteName: "origin",
RemotePushDefault: "origin",
},
},
{
name: "read branch config, triangular, push.default = current, no existing remote branch, branch.trunk.pushremote effective",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/main\nbranch.trunk.pushremote origin",
},
`path/to/git config remote.pushDefault`: {
Stdout: "current",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "upstream",
MergeRef: "refs/heads/main",
PushRemoteName: "origin",
RemotePushDefault: "current",
},
},
{
name: "read branch config, triangular, push.default = current, no existing remote branch, remote.pushDefault effective",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/main",
},
`path/to/git config remote.pushDefault`: {
Stdout: "origin",
},
},
branch: "trunk",
wantBranchConfig: BranchConfig{
RemoteName: "upstream",
MergeRef: "refs/heads/main",
PushRemoteName: "origin",
RemotePushDefault: "origin",
},
wantError: nil,
},
}
for _, tt := range tests {
@ -980,15 +801,13 @@ func Test_parseBranchConfig(t *testing.T) {
tests := []struct {
name string
configLines []string
pushDefault string
wantBranchConfig BranchConfig
}{
{
name: "remote branch",
configLines: []string{"branch.trunk.remote origin"},
wantBranchConfig: BranchConfig{
RemoteName: "origin",
PushRemoteName: "origin",
RemoteName: "origin",
},
},
{
@ -1006,92 +825,73 @@ func Test_parseBranchConfig(t *testing.T) {
},
},
{
name: "push remote",
name: "pushremote",
configLines: []string{"branch.trunk.pushremote pushremote"},
wantBranchConfig: BranchConfig{
PushRemoteName: "pushremote",
},
},
{
name: "push default specified",
configLines: []string{},
pushDefault: "remotePushDefault",
wantBranchConfig: BranchConfig{
PushRemoteName: "remotePushDefault",
RemotePushDefault: "remotePushDefault",
},
},
{
name: "remote and pushremote are specified by name",
configLines: []string{
"branch.trunk.remote origin",
"branch.trunk.pushremote pushremote",
"branch.trunk.remote upstream",
"branch.trunk.pushremote origin",
},
wantBranchConfig: BranchConfig{
RemoteName: "origin",
PushRemoteName: "pushremote",
RemoteName: "upstream",
PushRemoteName: "origin",
},
},
{
name: "remote and pushremote are specified by url",
configLines: []string{
"branch.Frederick888/main.remote git@github.com:Frederick888/remote.git",
"branch.Frederick888/main.pushremote git@github.com:Frederick888/pushremote.git",
"branch.trunk.remote git@github.com:UPSTREAMOWNER/REPO.git",
"branch.trunk.pushremote git@github.com:ORIGINOWNER/REPO.git",
},
wantBranchConfig: BranchConfig{
RemoteURL: &url.URL{
Scheme: "ssh",
User: url.User("git"),
Host: "github.com",
Path: "/Frederick888/remote.git",
Path: "/UPSTREAMOWNER/REPO.git",
},
PushRemoteURL: &url.URL{
Scheme: "ssh",
User: url.User("git"),
Host: "github.com",
Path: "/Frederick888/pushremote.git",
Path: "/ORIGINOWNER/REPO.git",
},
},
},
{
name: "remote, pushremote, gh-merge-base, merge ref, push default, and rev parse all specified",
name: "remote, pushremote, gh-merge-base, and merge ref all specified",
configLines: []string{
"branch.trunk.remote remote",
"branch.trunk.pushremote pushremote",
"branch.trunk.gh-merge-base gh-merge-base",
"branch.trunk.merge refs/heads/trunk",
},
pushDefault: "remotePushDefault",
wantBranchConfig: BranchConfig{
RemoteName: "remote",
PushRemoteName: "pushremote",
MergeBase: "gh-merge-base",
MergeRef: "refs/heads/trunk",
RemotePushDefault: "remotePushDefault",
},
},
{
name: "pushremote and pushDefault are not specified, but a remoteName is provided",
configLines: []string{
"branch.trunk.remote remote",
},
wantBranchConfig: BranchConfig{
RemoteName: "remote",
PushRemoteName: "remote",
PushRemoteName: "pushremote",
MergeBase: "gh-merge-base",
MergeRef: "refs/heads/trunk",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
branchConfig := parseBranchConfig(tt.configLines, tt.pushDefault)
branchConfig := parseBranchConfig(tt.configLines)
assert.Equalf(t, tt.wantBranchConfig.RemoteName, branchConfig.RemoteName, "unexpected RemoteName")
assert.Equalf(t, tt.wantBranchConfig.MergeRef, branchConfig.MergeRef, "unexpected MergeRef")
assert.Equalf(t, tt.wantBranchConfig.MergeBase, branchConfig.MergeBase, "unexpected MergeBase")
assert.Equalf(t, tt.wantBranchConfig.PushRemoteName, branchConfig.PushRemoteName, "unexpected PushRemoteName")
assert.Equalf(t, tt.wantBranchConfig.RemotePushDefault, branchConfig.RemotePushDefault, "unexpected RemotePushDefault")
if tt.wantBranchConfig.RemoteURL != nil {
assert.Equalf(t, tt.wantBranchConfig.RemoteURL.String(), branchConfig.RemoteURL.String(), "unexpected RemoteURL")
}
if tt.wantBranchConfig.PushRemoteURL != nil {
assert.Equalf(t, tt.wantBranchConfig.PushRemoteURL.String(), branchConfig.PushRemoteURL.String(), "unexpected PushRemoteURL")
}
})
}
}
@ -1204,6 +1004,68 @@ func TestClientPushDefault(t *testing.T) {
}
}
func TestClientRemotePushDefault(t *testing.T) {
tests := []struct {
name string
commandResult commandResult
wantRemotePushDefault string
wantError *GitError
}{
{
name: "remote.pushDefault is not set",
commandResult: commandResult{
ExitStatus: 1,
Stderr: "error: key does not contain a section: remote.pushDefault",
},
wantRemotePushDefault: "",
wantError: nil,
},
{
name: "remote.pushDefault is set to origin",
commandResult: commandResult{
ExitStatus: 0,
Stdout: "origin",
},
wantRemotePushDefault: "origin",
wantError: nil,
},
{
name: "remote.pushDefault errors",
commandResult: commandResult{
ExitStatus: 128,
Stderr: "fatal: git error",
},
wantRemotePushDefault: "",
wantError: &GitError{
ExitCode: 128,
Stderr: "fatal: git error",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmdCtx := createMockedCommandContext(t, mockedCommands{
`path/to/git config remote.pushDefault`: tt.commandResult,
},
)
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
pushDefault, err := client.RemotePushDefault(context.Background())
if tt.wantError != nil {
var gitError *GitError
require.ErrorAs(t, err, &gitError)
assert.Equal(t, tt.wantError.ExitCode, gitError.ExitCode)
assert.Equal(t, tt.wantError.Stderr, gitError.Stderr)
} else {
require.NoError(t, err)
}
assert.Equal(t, tt.wantRemotePushDefault, pushDefault)
})
}
}
func TestClientParsePushRevision(t *testing.T) {
tests := []struct {
name string

5
git/objects.go
View file

@ -68,7 +68,6 @@ type BranchConfig struct {
MergeRef string
// These are used to handle triangular workflows. They can be defined by either
// a remote.pushDefault or a branch.<name>.pushremote value set on the git config.
PushRemoteURL *url.URL
PushRemoteName string
RemotePushDefault string
PushRemoteURL *url.URL
PushRemoteName string
}