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:
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
quoted
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.
quoted
Note that it *does* work if the remote branch name is explicitly specified:
quoted
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.
On Thu, May 27, 2021 at 11:39 PM Mathias Kunter [off-list ref] wrote:
Felipe,
thanks for your reply.
quoted
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`:
quoted
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.
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 [off-list ref] wrote:
quoted
Felipe,
thanks for your reply.
quoted
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`:
quoted
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'.
quoted
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.
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:
quoted
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
quoted
return (fetch_remote && fetch_remote != remote);
to
quoted
return (!fetch_remote || fetch_remote != remote);
I'd argue that if `simple` pushing is used, then the expected behavior
of the command
quoted
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:
quoted
On Thu, May 27, 2021 at 11:39 PM Mathias Kunter
[off-list ref] wrote:
quoted
Felipe,
thanks for your reply.
quoted
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`:
quoted
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'.
quoted
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.
From: Felipe Contreras <hidden> Date: 2021-05-28 21:12:55
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
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?
From: Felipe Contreras <hidden> Date: 2021-05-30 16:32:24
Mathias Kunter wrote:
Am 28.05.21 um 23:12 schrieb Felipe Contreras:
quoted
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
From: Felipe Contreras <hidden> Date: 2021-05-28 17:53:15
Mathias Kunter wrote:
quoted
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
quoted
return (fetch_remote && fetch_remote != remote);
to
quoted
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
quoted
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
From: Felipe Contreras <hidden> Date: 2021-05-28 17:22:54
Elijah Newren wrote:
quoted
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:
@@ -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.
On Fri, May 28, 2021 at 10:22 AM Felipe Contreras
[off-list ref] wrote:
Elijah Newren wrote:
quoted
quoted
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.
quoted hunk
So you don't get the "current" behavior when pushing to "origin".
Perhaps:
@@ -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.
@@ -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
From: Felipe Contreras <hidden> Date: 2021-05-28 17:10:28
Mathias Kunter wrote:
Felipe,
thanks for your reply.
quoted
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`:
quoted
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