git push default doesn't make sense

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 28, 2021, 21:12 UTC
Message-ID: <60b15cd2c4136_2183bc20893@natae.notmuch>
In-Reply-To: <287483ba-d682-6d04-23fa-22c33ab70376@gmail.com>

Mathias Kunter wrote:

Show 5 quoted lines

> However, the advised "git push <name>" command won't work on that branch 
> with the default settings of Git. To make it work, `simple` pushing 
> would have to use `current` behavior on a branch without upstream.
> 
> Please consider changing that. Thank you.

OK, after reorganizing the code to actually make it understandable [1], I ended up with this:

	if (centralized) {
		if (!branch->merge_nr || !branch->merge || !branch->remote_name)
			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 (branch->merge_nr != 1)
			die(_("The current branch %s has multiple upstream branches, "
			    "refusing to push."), branch->name);

		/* Additional safety */
		if (strcmp(branch->refname, branch->merge[0]->src))
			die_push_simple(branch, remote);
	}
	refspec_appendf(&rs, "%s:%s", branch->refname, branch->refname);

I agree this doesn't make sense.

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 therore it passes the safety check of `push.default=simple`, and that is much more dangerous than pushing any other branch.

Why do we barf with "fix-1", but not "master"? Doesn't make sense.

This is what we want:

	if (centralized &&
		(branch->merge_nr && branch->merge && branch->remote_name))
	{
		if (branch->merge_nr != 1)
			die(_("The current branch %s has multiple upstream branches, "
			    "refusing to push."), branch->name);

		/* Additional safety */
		if (strcmp(branch->refname, branch->merge[0]->src))
			die_push_simple(branch, remote);
	}
	refspec_appendf(&rs, "%s:%s", branch->refname, branch->refname);

In other words: `simple` should be the same as `current`, except when there's an upstream branch configured *and* the destination branch has a different name.

Cheers.

[1] https://lore.kernel.org/git/20210528201014.2175179-1-felipe.contreras@gmail.com/

-- 
Felipe Contreras