[PATCH v2 0/2] push: fix default action

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The default action "simple" doesn't make sense. It's supposed to be the
same as "current", except with an extra safety in case the name of the
upstream branch doesn't match the name of the current branch (and we are
pushing to the same remote). But if there's no upstream configured
there's no need for any check.

If this works:

  git clone $central .
  ...
  git push

Then this should too:

  git clone $central .
  git checkout -b topic
  ...
  git push

Cloning automatically sets up an upstream branch for "master", and
therefore it passes the safety check, but that is much more dangerous
than pushing any other branch.

Why do we barf with "topic", but not "master"?

Instead of behaving like "current" if the current branch doesn't have an
upstream configured, `git push` fails like "upstream", so it's a
Frankensteinian action.

If the upstream branch isn't configured, "simple" should not abort, just
like "current".

Since v1 I changed get_upstream_ref to always be non-fatal, and die on
the only caller instead.

Felipe Contreras (2):
  push: reorganize get_upstream_ref
  push: make default consistent

 Documentation/config/push.txt |  5 +++--
 Documentation/git-push.txt    |  6 +++---
 builtin/push.c                | 25 +++++++++++++++----------
 t/t5528-push-default.sh       |  2 +-
 4 files changed, 22 insertions(+), 16 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  7031b26f28 push: reorganize get_upstream_ref
1:  20bc9059f9 ! 2:  964b3c4289 push: make default consistent
    @@ Commit message
         pushing to the same remote). But if there's no upstream configured
         there's no need for any check.
     
    -    If this works:
    -
    -      git clone $central .
    -      ...
    -      git push
    -
    -    Then this should too:
    -
    -      git clone $central .
    -      git checkout -b fix-1
    -      ...
    -      git push
    -
    -    Cloning automatically sets up an upstream branch for "master", and
    -    therefore it passes the safety check, but that is much more dangerous
    -    than pushing any other branch.
    -
    -    Why do we barf with "fix-1", but not "master"?
    -
         Instead of behaving like "current" if the current branch doesn't have an
         upstream configured, `git push` fails like "upstream", so it's a
         Frankensteinian action.
    @@ Documentation/git-push.txt: what to push (See linkgit:git-config[1] for the mean
      
     
      ## builtin/push.c ##
    -@@ builtin/push.c: static const char message_detached_head_die[] =
    - 	   "\n"
    - 	   "    git push %s HEAD:<name-of-remote-branch>\n");
    - 
    --static const char *get_upstream_ref(struct branch *branch, const char *remote_name)
    -+static const char *get_upstream_ref(struct branch *branch, const char *remote_name, int fatal)
    - {
    --	if (!branch->merge_nr || !branch->merge || !branch->remote_name)
    -+	if (!branch->merge_nr || !branch->merge || !branch->remote_name) {
    -+		if (!fatal)
    -+			return NULL;
    - 		die(_("The current branch %s has no upstream branch.\n"
    - 		    "To push the current branch and set the remote as upstream, use\n"
    - 		    "\n"
    -@@ builtin/push.c: static const char *get_upstream_ref(struct branch *branch, const char *remote_na
    - 		    branch->name,
    - 		    remote_name,
    - 		    branch->name);
    -+	}
    - 	if (branch->merge_nr != 1)
    - 		die(_("The current branch %s has multiple upstream branches, "
    - 		    "refusing to push."), branch->name);
     @@ builtin/push.c: static void setup_default_push_refspecs(struct remote *remote)
    - 	switch (push_default) {
    - 	default:
    - 	case PUSH_DEFAULT_UNSPECIFIED:
    --	case PUSH_DEFAULT_SIMPLE:
    -+	case PUSH_DEFAULT_SIMPLE: {
    -+		const char *upstream;
    -+
      		if (!same_remote)
      			break;
    --		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
    -+		upstream = get_upstream_ref(branch, remote->name, 0);
    -+		if (upstream && strcmp(branch->refname, upstream))
    + 		dst = get_upstream_ref(branch);
    +-		if (!dst)
    +-			die(_("The current branch %s has no upstream branch.\n"
    +-			    "To push the current branch and set the remote as upstream, use\n"
    +-			    "\n"
    +-			    "    git push --set-upstream %s %s\n"),
    +-			    branch->name,
    +-			    remote->name,
    +-			    branch->name);
    +-		if (strcmp(branch->refname, dst))
    ++		if (dst && strcmp(branch->refname, dst))
      			die_push_simple(branch, remote);
    ++		if (!dst)
    ++			dst = branch->refname;
      		break;
    -+	}
      
      	case PUSH_DEFAULT_UPSTREAM:
    - 		if (!same_remote)
    -@@ builtin/push.c: static void setup_default_push_refspecs(struct remote *remote)
    - 			      "your current branch '%s', without telling me what to push\n"
    - 			      "to update which remote branch."),
    - 			    remote->name, branch->name);
    --		dst = get_upstream_ref(branch, remote->name);
    -+		dst = get_upstream_ref(branch, remote->name, 1);
    - 		break;
    - 
    - 	case PUSH_DEFAULT_CURRENT:
     
      ## t/t5528-push-default.sh ##
     @@ t/t5528-push-default.sh: test_expect_success 'push from/to new branch with current creates remote branch'
-- 
2.32.0.40.gb9b36f9b52




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux