fix: Do not use set-upstream (#88)

Each local git branch can have an optional upstream branch. While the
name looks like the push destination of the branch, it's not. Unless
user specifically configure so, `git push` pushes a branch to the same
name branch on the remote (the simple strategy as described in `man 1
git-push`). This explains the behavior in the TODO comment in
internal/actions/pr.go "I think currently things will break if the
upstream name is not the same name as the local", because git's default
strategy is pushing to the same branch, not to the upstream branch.

The upstream branch is rather used when doing `git rebase`. Without an
argument, it rebases to the upstream branch. Likewise, when `git pull`
does the rebase, it'll rebase to the upstream branch. In many cases, if
a PR is rebased, the intention would be to rebase to the trunk branch
(master/main), not the same branch on the remote.

Based on this, it's not necessary for av-cli to modify the branch
upstream. It's not related to git-push (in most of the cases). Leaving
it intact should allow users to choose whichever branch that makes sense
to them (I personally use origin/main because it makes `git rebase`
easy).

This should not affect most of the users. Most of them won't notice
the change. The only case they are affected is (1) They do not use `av
pr create` but use `av stack sync` by some means creating a branch
metadata and (2) set the upstream branch to a different name manually
and (3) pushing to a different name branch manually or setting
"upstream" strategy explicitly in the git config. Again, it's very
unlikely this happens because `av pr create` did follow the "simple"
strategy rule, which is git's default, and in order for a user to be
affected by this change, they somehow need to get around it to use the
"upstream" strategy.

internal/actions/pr.go

@@ -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 {

internal/actions/push.go

@@ -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.
@@ -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",
@@ -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
@@ -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,
 	})

internal/actions/sync_branch.go

@@ -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"
@@ -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 {
@@ -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
 	}