git push doesn't use local branch name as default

git push doesn't use local branch name as default

[Mathias Kunter]

Felipe,

thanks for your reply.

> Sounds like you want to change the default to `push.default=current`.

Yes, but shouldn't `simple` pushing also work? The documentation says 
about `push.default=simple`:

> When pushing to a remote that is different from the remote you normally
> pull from, work as `current`.

If there is no upstream, then there also is no "remote I normally pull 
from", and thus, according to the doc, `simple` should actually work 
like `current` in this case. Am I wrong here?

If `simple` pushing is used, it doesn't seem to make sense for me to 
fallback to `current` on branches which *do* have an upstream, but to 
error out on branches which do *not* have an upstream.

Cheers
Mathias Kunter


Am 27.05.21 um 21:51 schrieb Felipe Contreras:
> Mathias Kunter wrote:
>> Hi all,
>>
>> at https://git-scm.com/docs/git-push#_description it says:
>>
>>> When neither the command-line nor the configuration specify what to
>>> push, the default behavior is used, which corresponds to the simple
>>> value for push.default: the current branch is pushed to the
>>> corresponding upstream branch, but as a safety measure, the push is
>>> aborted if the upstream branch does not have the same name as the local
>>> one.
>>
>> However, on a branch which does *not* have an upstream branch
>> configured, the command
>>
>>> git push <remote_name>
>>
>> doesn't use the local branch name as default,
> 
> Yes it does, but only on the src side of the refspec. Something like:
> 
>    git push <remote_name> <branch_name>:
> 
> (invalid refspec)
> 
> Note the remote side is missing, so git doesn't know where to push to.
> 
>> Note that it *does* work if the remote branch name is explicitly specified:
>>
>>> git push <remote_name> <branch_name>
> 
> In that case git assumes you mean <branch_name>:<branch_name>.
> 
> Sounds like you want to change the default to `push.default=current`.
> 
> Cheers.
> 

Re: git push doesn't use local branch name as default

[Elijah Newren]

On Thu, May 27, 2021 at 11:39 PM Mathias Kunter <mathiaskunter@gmail.com> wrote:
>
> Felipe,
>
> thanks for your reply.
>
> > Sounds like you want to change the default to `push.default=current`.
>
> Yes, but shouldn't `simple` pushing also work? The documentation says
> about `push.default=simple`:
>
> > When pushing to a remote that is different from the remote you normally
> > pull from, work as `current`.

Perhaps this wording should be clarified to read

When you have a remote that you normally pull from but you are pushing
to a different remote then that one, then work as 'current'.

> If there is no upstream, then there also is no "remote I normally pull
> from", and thus, according to the doc, `simple` should actually work
> like `current` in this case. Am I wrong here?

The relevant code is

    return (fetch_remote && fetch_remote != remote);

so you only get the "current" behavior when fetch_remote is non-NULL.

Re: git push doesn't use local branch name as default

[Mathias Kunter]

> you only get the "current" behavior when fetch_remote is non-NULL.

Well, then my suggestion actually is to also use the `current` behavior 
when fetch_remote is NULL - i.e. change

> return (fetch_remote && fetch_remote != remote);

to

> return (!fetch_remote || fetch_remote != remote);

I'd argue that if `simple` pushing is used, then the expected behavior 
of the command

> git push <remote_name>

on a branch without upstream would actually be to use the `current` 
behavior instead of bailing out with an error.


Am 28.05.21 um 09:00 schrieb Elijah Newren:
> On Thu, May 27, 2021 at 11:39 PM Mathias Kunter <mathiaskunter@gmail.com> wrote:
>>
>> Felipe,
>>
>> thanks for your reply.
>>
>>> Sounds like you want to change the default to `push.default=current`.
>>
>> Yes, but shouldn't `simple` pushing also work? The documentation says
>> about `push.default=simple`:
>>
>>> When pushing to a remote that is different from the remote you normally
>>> pull from, work as `current`.
> 
> Perhaps this wording should be clarified to read
> 
> When you have a remote that you normally pull from but you are pushing
> to a different remote then that one, then work as 'current'.
> 
>> If there is no upstream, then there also is no "remote I normally pull
>> from", and thus, according to the doc, `simple` should actually work
>> like `current` in this case. Am I wrong here?
> 
> The relevant code is
> 
>      return (fetch_remote && fetch_remote != remote);
> 
> so you only get the "current" behavior when fetch_remote is non-NULL.
> 

Re: git push doesn't use local branch name as default

[Mathias Kunter]

Note that git itself also claims this should be working. If I do a "git 
push" on a branch without upstream, I will get:

> fatal: No configured push destination.
> Either specify the URL from the command-line or configure a remote repository using
> 
>     git remote add <name> <url>
> 
> and then push using the remote name
> 
>     git push <name>

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.

Cheers
Mathias Kunter


Am 28.05.21 um 09:44 schrieb Mathias Kunter:
>> you only get the "current" behavior when fetch_remote is non-NULL.
> 
> Well, then my suggestion actually is to also use the `current` behavior 
> when fetch_remote is NULL - i.e. change
> 
>> return (fetch_remote && fetch_remote != remote);
> 
> to
> 
>> return (!fetch_remote || fetch_remote != remote);
> 
> I'd argue that if `simple` pushing is used, then the expected behavior 
> of the command
> 
>> git push <remote_name>
> 
> on a branch without upstream would actually be to use the `current` 
> behavior instead of bailing out with an error.
> 
> 
> Am 28.05.21 um 09:00 schrieb Elijah Newren:
>> On Thu, May 27, 2021 at 11:39 PM Mathias Kunter 
>> <mathiaskunter@gmail.com> wrote:
>>>
>>> Felipe,
>>>
>>> thanks for your reply.
>>>
>>>> Sounds like you want to change the default to `push.default=current`.
>>>
>>> Yes, but shouldn't `simple` pushing also work? The documentation says
>>> about `push.default=simple`:
>>>
>>>> When pushing to a remote that is different from the remote you normally
>>>> pull from, work as `current`.
>>
>> Perhaps this wording should be clarified to read
>>
>> When you have a remote that you normally pull from but you are pushing
>> to a different remote then that one, then work as 'current'.
>>
>>> If there is no upstream, then there also is no "remote I normally pull
>>> from", and thus, according to the doc, `simple` should actually work
>>> like `current` in this case. Am I wrong here?
>>
>> The relevant code is
>>
>>      return (fetch_remote && fetch_remote != remote);
>>
>> so you only get the "current" behavior when fetch_remote is non-NULL.
>>

git push default doesn't make sense

[Felipe Contreras]

Mathias Kunter wrote:
> 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

Re: git push default doesn't make sense

[Mathias Kunter]

Am 28.05.21 um 23:12 schrieb Felipe Contreras:

> 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.

I guess so. In particular, as a simple git user, I'd expect the 
following to work out of the box, without having to manually adjust the 
configuration settings:

   git clone ssh://originUrl .
   git checkout -b myBranch
   git push           # expected push to origin/myBranch, but fails
   git push origin    # expected push to origin/myBranch, but fails
   git remote add myRemote ssh://myRemoteUrl
   git push myRemote  # expected push to myRemote/myBranch - works

Will your provided patch fix these failing push commands?

Re: git push default doesn't make sense

[Felipe Contreras]

Mathias Kunter wrote:
> Am 28.05.21 um 23:12 schrieb Felipe Contreras:
> 
> > 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.
> 
> I guess so. In particular, as a simple git user, I'd expect the 
> following to work out of the box, without having to manually adjust the 
> configuration settings:
> 
>    git clone ssh://originUrl .
>    git checkout -b myBranch
>    git push           # expected push to origin/myBranch, but fails
>    git push origin    # expected push to origin/myBranch, but fails
>    git remote add myRemote ssh://myRemoteUrl
>    git push myRemote  # expected push to myRemote/myBranch - works
> 
> Will your provided patch fix these failing push commands?

It's not really a patch (yet), but yeah: it will.

-- 
Felipe Contreras

Re: git push doesn't use local branch name as default

[Felipe Contreras]

Mathias Kunter wrote:
> > you only get the "current" behavior when fetch_remote is non-NULL.
> 
> Well, then my suggestion actually is to also use the `current` behavior 
> when fetch_remote is NULL - i.e. change
> 
> > return (fetch_remote && fetch_remote != remote);
> 
> to
> 
> > return (!fetch_remote || fetch_remote != remote);

It's not quite that easy. You need to see the context of that code:

  static int is_workflow_triangular(struct remote *remote)
  {
    struct remote *fetch_remote = remote_get(NULL);
    return (fetch_remote && fetch_remote != remote);
  }

That would affect many pathways.

> I'd argue that if `simple` pushing is used, then the expected behavior 
> of the command
> 
> > git push <remote_name>
> 
> on a branch without upstream would actually be to use the `current` 
> behavior instead of bailing out with an error.

I agree, but this mis-mash of modes makes the logic very hard to see.

I'll send patches to cleanup the logic, it makes no sense to have a
frankenstein of two modes and that is in fact the default mode. The
logic of the default mode should be crystal clear.

Cheers.

-- 
Felipe Contreras

Re: git push doesn't use local branch name as default

[Felipe Contreras]

Elijah Newren wrote:
> > If there is no upstream, then there also is no "remote I normally pull
> > from", and thus, according to the doc, `simple` should actually work
> > like `current` in this case. Am I wrong here?
> 
> The relevant code is
> 
>     return (fetch_remote && fetch_remote != remote);
> 
> so you only get the "current" behavior when fetch_remote is non-NULL.

fetch_remote is practically never non-NULL.

fetch_remote is remote_get(NULL), which is basically the equivalent of:

remote_get(remote_for_branch(current_branch, ...));

Typically when an upstream branch is not configured, this is the same
as:

remote_get("origin");

The only time fetch_remote is NULL is when the configured remote is
invalid.

So you don't get the "current" behavior when pushing to "origin".

Perhaps:

--- a/Documentation/config/push.txt
+++ b/Documentation/config/push.txt
@@ -29,8 +29,8 @@ push.default::
   different from the local one.
 +
 When pushing to a remote that is different from the remote you normally
-pull from, work as `current`.  This is the safest option and is suited
-for beginners.
+pull from (typically "origin"), work as `current`.  This is the safest option
+and is suited for beginners.
 +
 This mode has become the default in Git 2.0.

-- 
Felipe Contreras

Re: git push doesn't use local branch name as default

[Elijah Newren]

On Fri, May 28, 2021 at 10:22 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Elijah Newren wrote:
> > > If there is no upstream, then there also is no "remote I normally pull
> > > from", and thus, according to the doc, `simple` should actually work
> > > like `current` in this case. Am I wrong here?
> >
> > The relevant code is
> >
> >     return (fetch_remote && fetch_remote != remote);
> >
> > so you only get the "current" behavior when fetch_remote is non-NULL.
>
> fetch_remote is practically never non-NULL.
>
> fetch_remote is remote_get(NULL), which is basically the equivalent of:
>
> remote_get(remote_for_branch(current_branch, ...));
>
> Typically when an upstream branch is not configured, this is the same
> as:
>
> remote_get("origin");
>
> The only time fetch_remote is NULL is when the configured remote is
> invalid.

Ah, thanks for correcting and clarifying here.

>
> So you don't get the "current" behavior when pushing to "origin".
>
> Perhaps:
>
> --- a/Documentation/config/push.txt
> +++ b/Documentation/config/push.txt
> @@ -29,8 +29,8 @@ push.default::
>    different from the local one.
>  +
>  When pushing to a remote that is different from the remote you normally
> -pull from, work as `current`.  This is the safest option and is suited
> -for beginners.
> +pull from (typically "origin"), work as `current`.  This is the safest option
> +and is suited for beginners.

This is certainly an improvement.  I wonder if it might still be
considered ambiguous or hard to parse, though.  If so, maybe something
like:

If you have a default remote configured for the current branch and are
pushing to a remote other than that one (or if you have no default
remote configured and are pushing to a remote other than 'origin'),
then work as 'current'.

It may also be helpful to move the "This is the safest option and is
suited for beginners" out of its current paragraph and combine it with
the "This mode has become the default" in the next paragraph.

Re: git push doesn't use local branch name as default

[Felipe Contreras]

Elijah Newren wrote:
> On Fri, May 28, 2021 at 10:22 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:

> > Perhaps:
> >
> > --- a/Documentation/config/push.txt
> > +++ b/Documentation/config/push.txt
> > @@ -29,8 +29,8 @@ push.default::
> >    different from the local one.
> >  +
> >  When pushing to a remote that is different from the remote you normally
> > -pull from, work as `current`.  This is the safest option and is suited
> > -for beginners.
> > +pull from (typically "origin"), work as `current`.  This is the safest option
> > +and is suited for beginners.
> 
> This is certainly an improvement.  I wonder if it might still be
> considered ambiguous or hard to parse, though.

I am sure it is, just like plenty of the official documentation.

> If so, maybe something like:
> 
> If you have a default remote configured for the current branch

This is still very hard to parse, especially since there's no command to
"configure the remote of a branch" (AFAIK).

> and are pushing to a remote other than that one

And you've lost me.

I have to do a mental model of what's trying to be said:

  x && pushed != x

If x is nil, then I can't push to it, so this is the same as:

  pushed != x

> (or if you have no default remote configured and are pushing to a
> remote other than 'origin'),

So:

  !x && pushed != 'origin'

Altogether:

  (pushed != x) || (!x && pushed != 'origin')

So:

  pushed != (x || 'origin')

And we can use an x that is more friendly to users (and there are
commands to set it):

  If you are pushing to a remote that is not the same as the upstream
  branch, or 'origin'...

> then work as 'current'.

And now I have to read what `current` is.

The current documentation is trying to replicate what the convoluted
code is doing, your version is a little better, but not by much.


This is much more straightforward:

  pushes the current branch with the same name on the remote.

  If you are working on a centralized workflow--pushing to the same
  repository you pull from (typically `origin`)--then you need to
  configure an upstream branch with the same name.

When you describe the situation clearly (and not simply transcribe the
code), it becomes clear why users like Mathias don't see the current
behavior as sane.

I'm working on some patches so the issue becomes clear for those who
don't speak user.

Cheers.

-- 
Felipe Contreras

RE: git push doesn't use local branch name as default

[Felipe Contreras]

Mathias Kunter wrote:
> Felipe,
> 
> thanks for your reply.
> 
> > Sounds like you want to change the default to `push.default=current`.
> 
> Yes, but shouldn't `simple` pushing also work? The documentation says 
> about `push.default=simple`:
> 
> > When pushing to a remote that is different from the remote you normally
> > pull from, work as `current`.
> 
> If there is no upstream, then there also is no "remote I normally pull 
> from",

If there's no upstream the remote is "origin".

> and thus, according to the doc, `simple` should actually work 
> like `current` in this case. Am I wrong here?

Only if you are not pushing to "origin".

> If `simple` pushing is used, it doesn't seem to make sense for me to 
> fallback to `current` on branches which *do* have an upstream, but to 
> error out on branches which do *not* have an upstream.

That is not the criteria. It depends whether or not you are in a
triangular workflow [1].

If you pull from kernel.org, but push to github.com, then you are in a
triangular workflow and the name of the branch is not checked.

The oposite is a centralized workflow, where you pull and push to the
same repository, then git adds an extra check.

If you don't want to set `push.default`, you can alternatively rename
your remote to something other than "origin", then your branches with no
upstram truly would have nowhere to fetch from.

But then `git fetch` without arguments will not do anything.

Cheers.

[1] https://felipec.wordpress.com/2014/05/11/git-triangular-workflows/

-- 
Felipe Contreras