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

Merge branch 'fetch-artifact-attestation-bundles-with-sas-url' of github.com:malancas/cli into fetch-artifact-attestation-bundles-with-sas-url

Browse source
This commit is contained in:
Meredith Lancaster 2025-01-06 12:58:13 -07:00
commit fb4fc7e7c2
10 changed files with 132 additions and 72 deletions
Download patch file Download diff file Expand all files Collapse all files

27
acceptance/testdata/pr/pr-create-without-upstream-config.txtar vendored Normal file
View file

@ -0,0 +1,27 @@
# This test is the same as pr-create-basic, except that the git push doesn't include the -u argument
# This causes a git config read to fail during gh pr create, but it should not be fatal
# Use gh as a credential helper
exec gh auth setup-git
# Create a repository with a file so it has a default branch
exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private
# Defer repo cleanup
defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING
# Clone the repo
exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING
# Prepare a branch to PR
cd $SCRIPT_NAME-$RANDOM_STRING
exec git checkout -b feature-branch
exec git commit --allow-empty -m 'Empty Commit'
exec git push origin feature-branch
# Create the PR
exec gh pr create --title 'Feature Title' --body 'Feature Body'
# Check the PR is indeed created
exec gh pr view
stdout 'Feature Title'

1
git/client.go
View file

@ -389,7 +389,6 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
return
}
cfg.LocalName = branch
for _, line := range outputLines(out) {
parts := strings.SplitN(line, " ", 2)
if len(parts) < 2 {

2
git/client_test.go
View file

@ -737,7 +737,7 @@ func TestClientReadBranchConfig(t *testing.T) {
name: "read branch config",
cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`,
wantBranchConfig: BranchConfig{LocalName: "trunk", RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
},
}
for _, tt := range tests {

12
git/objects.go
View file

@ -54,16 +54,6 @@ type Ref struct {
Name string
}
// TrackingRef represents a ref for a remote tracking branch.
type TrackingRef struct {
RemoteName string
BranchName string
}
func (r TrackingRef) String() string {
return "refs/remotes/" + r.RemoteName + "/" + r.BranchName
}
type Commit struct {
Sha string
Title string
@ -71,8 +61,6 @@ type Commit struct {
}
type BranchConfig struct {
// LocalName of the branch.
LocalName string
RemoteName string
RemoteURL *url.URL
// MergeBase is the optional base branch to target in a new PR if `--base` is not specified.

2
go.mod
View file

@ -11,7 +11,7 @@ require (
github.com/cenkalti/backoff/v4 v4.3.0
github.com/charmbracelet/glamour v0.7.0
github.com/charmbracelet/lipgloss v0.10.1-0.20240413172830-d0be07ea6b9c
github.com/cli/go-gh/v2 v2.11.1
github.com/cli/go-gh/v2 v2.11.2
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
github.com/cli/oauth v1.1.1
github.com/cli/safeexec v1.0.1

4
go.sum
View file

@ -95,8 +95,8 @@ github.com/charmbracelet/x/exp/term v0.0.0-20240425164147-ba2a9512b05f/go.mod h1
github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
github.com/cli/go-gh/v2 v2.11.1 h1:amAyfqMWQTBdue8iTmDUegGZK7c8kk6WCxD9l/wLtGI=
github.com/cli/go-gh/v2 v2.11.1/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE=
github.com/cli/go-gh/v2 v2.11.2 h1:oad1+sESTPNTiTvh3I3t8UmxuovNDxhwLzeMHk45Q9w=
github.com/cli/go-gh/v2 v2.11.2/go.mod h1:vVFhi3TfjseIW26ED9itAR8gQK0aVThTm8sYrsZ5QTI=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk=
github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs=

84
pkg/cmd/pr/create/create.go
View file

@ -518,44 +518,80 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
return nil
}
func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranchConfig *git.BranchConfig) *git.TrackingRef {
refsForLookup := []string{"HEAD"}
var trackingRefs []git.TrackingRef
// trackingRef represents a ref for a remote tracking branch.
type trackingRef struct {
remoteName string
branchName string
}
if headBranchConfig.RemoteName != "" {
tr := git.TrackingRef{
RemoteName: headBranchConfig.RemoteName,
BranchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
func (r trackingRef) String() string {
return "refs/remotes/" + r.remoteName + "/" + r.branchName
}
func mustParseTrackingRef(text string) trackingRef {
parts := strings.SplitN(string(text), "/", 4)
// The only place this is called is tryDetermineTrackingRef, where we are reconstructing
// the same tracking ref we passed in. If it doesn't match the expected format, this is a
// programmer error we want to know about, so it's ok to panic.
if len(parts) != 4 {
panic(fmt.Errorf("tracking ref should have four parts: %s", text))
}
if parts[0] != "refs" || parts[1] != "remotes" {
panic(fmt.Errorf("tracking ref should start with refs/remotes/: %s", text))
}
return trackingRef{
remoteName: parts[2],
branchName: parts[3],
}
}
// tryDetermineTrackingRef is intended to try and find a remote branch on the same commit as the currently checked out
// HEAD, i.e. the local branch. If there are multiple branches that might match, the first remote is chosen, which in
// practice is determined by the sorting algorithm applied much earlier in the process, roughly "upstream", "github", "origin",
// and then everything else unstably sorted.
func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (trackingRef, bool) {
// To try and determine the tracking ref for a local branch, we first construct a collection of refs
// that might be tracking, given the current branch's config, and the list of known remotes.
refsForLookup := []string{"HEAD"}
if headBranchConfig.RemoteName != "" && headBranchConfig.MergeRef != "" {
tr := trackingRef{
remoteName: headBranchConfig.RemoteName,
branchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
}
trackingRefs = append(trackingRefs, tr)
refsForLookup = append(refsForLookup, tr.String())
}
for _, remote := range remotes {
tr := git.TrackingRef{
RemoteName: remote.Name,
BranchName: headBranchConfig.LocalName,
tr := trackingRef{
remoteName: remote.Name,
branchName: localBranchName,
}
trackingRefs = append(trackingRefs, tr)
refsForLookup = append(refsForLookup, tr.String())
}
// Then we ask git for details about these refs, for example, refs/remotes/origin/trunk might return a hash
// for the remote tracking branch, trunk, for the remote, origin. If there is no ref, the git client returns
// no ref information.
//
// We also first check for the HEAD ref, so that we have the hash of the currently checked out commit.
resolvedRefs, _ := gitClient.ShowRefs(context.Background(), refsForLookup)
// If there is more than one resolved ref, that means that at least one ref was found in addition to the HEAD.
if len(resolvedRefs) > 1 {
headRef := resolvedRefs[0]
for _, r := range resolvedRefs[1:] {
if r.Hash != resolvedRefs[0].Hash {
// If the hash of the remote ref doesn't match the hash of HEAD then the remote branch is not in the same
// state, so it can't be used.
if r.Hash != headRef.Hash {
continue
}
for _, tr := range trackingRefs {
if tr.String() != r.Name {
continue
}
return &tr
}
// Otherwise we can parse the returned ref into a tracking ref and return that
return mustParseTrackingRef(r.Name), true
}
}
return nil
return trackingRef{}, false
}
func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
@ -647,14 +683,14 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
if isPushEnabled {
// determine whether the head branch is already pushed to a remote
if pushedTo := determineTrackingBranch(gitClient, remotes, &headBranchConfig); pushedTo != nil {
if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
isPushEnabled = false
if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil {
if r, err := remotes.FindByName(trackingRef.remoteName); err == nil {
headRepo = r
headRemote = r
headBranchLabel = pushedTo.BranchName
headBranchLabel = trackingRef.branchName
if !ghrepo.IsSame(baseRepo, headRepo) {
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), pushedTo.BranchName)
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), trackingRef.branchName)
}
}
}

63
pkg/cmd/pr/create/create_test.go
View file

@ -1622,12 +1622,13 @@ func Test_createRun(t *testing.T) {
}
}
func Test_determineTrackingBranch(t *testing.T) {
func Test_tryDetermineTrackingRef(t *testing.T) {
tests := []struct {
name string
cmdStubs func(*run.CommandStubber)
remotes context.Remotes
assert func(ref *git.TrackingRef, t *testing.T)
name string
cmdStubs func(*run.CommandStubber)
remotes context.Remotes
expectedTrackingRef trackingRef
expectedFound bool
}{
{
name: "empty",
@ -1635,54 +1636,53 @@ func Test_determineTrackingBranch(t *testing.T) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
},
assert: func(ref *git.TrackingRef, t *testing.T) {
assert.Nil(t, ref)
},
expectedTrackingRef: trackingRef{},
expectedFound: false,
},
{
name: "no match",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register("git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature", 0, "abc HEAD\nbca refs/remotes/origin/feature")
cs.Register("git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature", 0, "abc HEAD\nbca refs/remotes/upstream/feature")
},
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
},
assert: func(ref *git.TrackingRef, t *testing.T) {
assert.Nil(t, ref)
},
expectedTrackingRef: trackingRef{},
expectedFound: false,
},
{
name: "match",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature$`, 0, heredoc.Doc(`
cs.Register(`git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature$`, 0, heredoc.Doc(`
deadbeef HEAD
deadb00f refs/remotes/origin/feature
deadbeef refs/remotes/upstream/feature
deadb00f refs/remotes/upstream/feature
deadbeef refs/remotes/origin/feature
`))
},
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
},
assert: func(ref *git.TrackingRef, t *testing.T) {
assert.Equal(t, "upstream", ref.RemoteName)
assert.Equal(t, "feature", ref.BranchName)
expectedTrackingRef: trackingRef{
remoteName: "origin",
branchName: "feature",
},
expectedFound: true,
},
{
name: "respect tracking config",
@ -1702,9 +1702,8 @@ func Test_determineTrackingBranch(t *testing.T) {
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
},
assert: func(ref *git.TrackingRef, t *testing.T) {
assert.Nil(t, ref)
},
expectedTrackingRef: trackingRef{},
expectedFound: false,
},
}
for _, tt := range tests {
@ -1719,8 +1718,10 @@ func Test_determineTrackingBranch(t *testing.T) {
GitPath: "some/path/git",
}
headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
ref := determineTrackingBranch(gitClient, tt.remotes, &headBranchConfig)
tt.assert(ref, t)
ref, found := tryDetermineTrackingRef(gitClient, tt.remotes, "feature", headBranchConfig)
assert.Equal(t, tt.expectedTrackingRef, ref)
assert.Equal(t, tt.expectedFound, found)
})
}
}

2
pkg/cmd/repo/fork/fork.go
View file

@ -221,6 +221,8 @@ func forkRun(opts *ForkOptions) error {
} else {
if connectedToTerminal {
fmt.Fprintf(stderr, "%s Created fork %s\n", cs.SuccessIconWithColor(cs.Green), cs.Bold(ghrepo.FullName(forkedRepo)))
} else {
fmt.Fprintln(opts.IO.Out, ghrepo.GenerateRepoURL(forkedRepo, ""))
}
}

7
pkg/cmd/repo/fork/fork_test.go
View file

@ -390,6 +390,7 @@ func TestRepoFork(t *testing.T) {
},
},
httpStubs: forkPost,
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "implicit nontty remote exists",
@ -424,11 +425,13 @@ func TestRepoFork(t *testing.T) {
cs.Register("git remote rename origin upstream", 0, "")
cs.Register(`git remote add origin https://github.com/someone/REPO.git`, 0, "")
},
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "implicit nontty no args",
opts: &ForkOptions{},
httpStubs: forkPost,
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "passes git flags",
@ -561,6 +564,7 @@ func TestRepoFork(t *testing.T) {
Repository: "OWNER/REPO",
},
httpStubs: forkPost,
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "repo arg nontty repo already exists",
@ -604,6 +608,7 @@ func TestRepoFork(t *testing.T) {
cs.Register(`git -C REPO fetch upstream`, 0, "")
cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
},
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "non tty repo arg with fork-name",
@ -640,6 +645,7 @@ func TestRepoFork(t *testing.T) {
httpmock.StringResponse(renameResult))
},
wantErrOut: "",
wantOut: "https://github.com/OWNER/REPO\n",
},
{
name: "tty repo arg with fork-name",
@ -694,6 +700,7 @@ func TestRepoFork(t *testing.T) {
cs.Register(`git -C REPO fetch upstream`, 0, "")
cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
},
wantOut: "https://github.com/someone/REPO\n",
},
{
name: "does not retry clone if error occurs and exit code is not 128",