Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not use set-upstream #88

Merged
merged 2 commits into from
Apr 20, 2023
Merged
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
21 changes: 5 additions & 16 deletions internal/actions/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,13 @@ func CreatePullRequest(ctx context.Context, repo *git.Repo, client *gh.Client, o
pushFlags = append(pushFlags, "--force-with-lease")
}

// Check if the upstream is set. If not, we set it during push.
// TODO: Should we store this somewhere? I think currently things will
// break if the upstream name is not the same name as the local
upstream, err := repo.RevParse(&git.RevParse{
SymbolicFullName: true,
Rev: fmt.Sprintf("%s@{u}", opts.BranchName),
})
if err != nil {
// Set the upstream branch
upstream = "origin/" + opts.BranchName
pushFlags = append(pushFlags, "--set-upstream", "origin", opts.BranchName)
} else {
upstream = strings.TrimPrefix(upstream, "refs/remotes/")
}
logrus.WithField("upstream", upstream).Debug("pushing latest changes")
// NOTE: This assumes that the user use the default push strategy (simple). It would
// be rare to use the upstream strategy.
pushFlags = append(pushFlags, "origin", opts.BranchName)
logrus.Debug("pushing latest changes")

_, _ = fmt.Fprint(os.Stderr,
" - pushing to ", color.CyanString("%s", upstream),
" - pushing to ", color.CyanString("origin/%s", opts.BranchName),
"\n",
)
if _, err := repo.Git(pushFlags...); err != nil {
Expand Down
38 changes: 16 additions & 22 deletions internal/actions/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ const (

type PushOpts struct {
Force ForceOpt
// If true, require the upstream tracking information to already be set
// (otherwise, don't push).
SkipIfUpstreamNotSet bool
// If true, skip pushing the branch if the upstream commit is the same as
// the local HEAD commit. The caller should probably call `git fetch` before
// If true, require the corresponding branch exist on the remote (otherwise, don't push).
SkipIfRemoteBranchNotExist bool
// If true, skip pushing the branch if the corresponding branch on the remote points to the
// same commit as the local HEAD commit. The caller should probably call `git fetch` before
// running this to make sure remote tracking information is up-to-date.
SkipIfUpstreamMatches bool
SkipIfRemoteBranchIsUpToDate bool
}

// Push pushes the current branch to the Git origin.
Expand All @@ -42,27 +41,21 @@ func Push(repo *git.Repo, opts PushOpts) error {
return errors.WrapIff(err, "failed to determine current branch")
}

if opts.SkipIfUpstreamNotSet || opts.SkipIfUpstreamMatches {
upstream, err := repo.RevParse(&git.RevParse{Rev: "@{upstream}"})
if opts.SkipIfUpstreamMatches && git.StderrMatches(err, "no upstream") {
_, _ = fmt.Fprint(os.Stderr,
" - not pushing branch ", colors.UserInput(currentBranch),
" (no upstream is set)\n",
colors.Troubleshooting(" - HINT: create a pr with "),
colors.CliCmd("av pr create"),
colors.Troubleshooting(" to automatically push the branch to GitHub and create a pull request\n"),
)
return nil
} else if err != nil {
// This usually indicates a detached HEAD state
return errors.WrapIff(err, "failed to determine upstream tracking information for branch %q", currentBranch)
if opts.SkipIfRemoteBranchNotExist || opts.SkipIfRemoteBranchIsUpToDate {
// NOTE: This remote branch pattern is configurable with the fetch spec. This code
// assumes that the user won't change the fetch spec from the default. Technically,
// this must be generated from the fetch spec.
remoteBranch := "refs/remotes/origin/" + currentBranch
remoteBranchCommit, err := repo.RevParse(&git.RevParse{Rev: remoteBranch})
if err != nil {
return errors.WrapIff(err, "corresponding remote branch %q doesn't exist", remoteBranch)
}

head, err := repo.RevParse(&git.RevParse{Rev: "HEAD"})
if err != nil {
return errors.WrapIff(err, "failed to determine branch HEAD for branch %q", currentBranch)
}
if opts.SkipIfUpstreamMatches && upstream == head {
if opts.SkipIfRemoteBranchIsUpToDate && remoteBranchCommit == head {
_, _ = fmt.Fprint(os.Stderr,
" - not pushing branch ", colors.UserInput(currentBranch),
" (upstream is already up-to-date)\n",
Expand All @@ -74,7 +67,7 @@ func Push(repo *git.Repo, opts PushOpts) error {
_, _ = fmt.Fprint(os.Stderr,
" - pushing ", colors.UserInput(currentBranch), "... ",
)
pushArgs := []string{"push", "--set-upstream"}
pushArgs := []string{"push"}
switch opts.Force {
case NoForce:
// pass
Expand All @@ -83,6 +76,7 @@ func Push(repo *git.Repo, opts PushOpts) error {
case ForcePush:
pushArgs = append(pushArgs, "--force")
}
pushArgs = append(pushArgs, "origin", currentBranch)
res, err := repo.Run(&git.RunOpts{
Args: pushArgs,
})
Expand Down
13 changes: 7 additions & 6 deletions internal/actions/sync_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package actions

import (
"context"
"emperror.dev/errors"
"fmt"
"os"
"strings"

"emperror.dev/errors"
"github.com/aviator-co/av/internal/config"
"github.com/aviator-co/av/internal/gh"
"github.com/aviator-co/av/internal/git"
Expand All @@ -13,8 +16,6 @@ import (
"github.com/aviator-co/av/internal/utils/sliceutils"
"github.com/shurcooL/githubv4"
"github.com/sirupsen/logrus"
"os"
"strings"
)

type SyncBranchOpts struct {
Expand Down Expand Up @@ -464,9 +465,9 @@ func syncBranchPushAndUpdatePullRequest(
}

if err := Push(repo, PushOpts{
Force: ForceWithLease,
SkipIfUpstreamNotSet: true,
SkipIfUpstreamMatches: true,
Force: ForceWithLease,
SkipIfRemoteBranchNotExist: true,
SkipIfRemoteBranchIsUpToDate: true,
}); err != nil {
return err
}
Expand Down