Re: Bug in git rev-parse @{push}?
On Thu, Nov 03, 2016 at 03:59:13PM +, Chris Purcell wrote:
> >> Thanks, Jeff! If I remove the explicit configuration of remote.pushdefault
> >> = "origin", I get the same error message as you, so I suspect that's _not_
> >> the default.
> >
> > That's really bizarre, because I get the same behavior with or without
> > it set. Not only that, but it shouldn't even come into play, as
> > branch.foo.remote should take precedence anyway.
> >
> > So now I'm really puzzled.
>
> That's because I lied. Sorry! Too many branches configured. I've actually got
>
> [branch "foo"]
> remote = .
>
> If I change that to 'origin' like I claimed it was, it starts erroring.
OK, now I can reproduce. And changing remote.pushdefault _does_ matter
because builtin/push.c:is_workflow_triangular() uses it as a key to "we
are in a triangular workflow".
That was added by ed2b18292 (push: change `simple` to accommodate
triangular workflows, 2013-06-19). TBH, I think the right solution is
"stop using 'simple' in a triangular setup". But since that workflow
exists for git-push, we probably need to support it via @{push}, too.
I think the solution would be something like:
- move is_workflow_triangular() into remote.c so it is accessible in
both places
- when branch_get_push_1 sees that is_workflow_triangular() is true,
treat "simple" as "current"
- new tests should go into t/t1514-rev-parse-push.sh to cover this
case
Want to take a stab at it?
-Peff
Re: Bug in git rev-parse @{push}?
Will do, thanks! Meanwhile, I'll work around locally by changing off
'simple' in my config—if I can figure out how not to break everything
in the process...
On 3 November 2016 at 16:07, Jeff King wrote:
> On Thu, Nov 03, 2016 at 03:59:13PM +, Chris Purcell wrote:
>
>> >> Thanks, Jeff! If I remove the explicit configuration of remote.pushdefault
>> >> = "origin", I get the same error message as you, so I suspect that's _not_
>> >> the default.
>> >
>> > That's really bizarre, because I get the same behavior with or without
>> > it set. Not only that, but it shouldn't even come into play, as
>> > branch.foo.remote should take precedence anyway.
>> >
>> > So now I'm really puzzled.
>>
>> That's because I lied. Sorry! Too many branches configured. I've actually got
>>
>> [branch "foo"]
>> remote = .
>>
>> If I change that to 'origin' like I claimed it was, it starts erroring.
>
> OK, now I can reproduce. And changing remote.pushdefault _does_ matter
> because builtin/push.c:is_workflow_triangular() uses it as a key to "we
> are in a triangular workflow".
>
> That was added by ed2b18292 (push: change `simple` to accommodate
> triangular workflows, 2013-06-19). TBH, I think the right solution is
> "stop using 'simple' in a triangular setup". But since that workflow
> exists for git-push, we probably need to support it via @{push}, too.
>
> I think the solution would be something like:
>
> - move is_workflow_triangular() into remote.c so it is accessible in
> both places
>
> - when branch_get_push_1 sees that is_workflow_triangular() is true,
> treat "simple" as "current"
>
> - new tests should go into t/t1514-rev-parse-push.sh to cover this
> case
>
> Want to take a stab at it?
>
> -Peff
Re: Bug in git rev-parse @{push}?
>> Thanks, Jeff! If I remove the explicit configuration of remote.pushdefault >> = "origin", I get the same error message as you, so I suspect that's _not_ >> the default. > > That's really bizarre, because I get the same behavior with or without > it set. Not only that, but it shouldn't even come into play, as > branch.foo.remote should take precedence anyway. > > So now I'm really puzzled. That's because I lied. Sorry! Too many branches configured. I've actually got [branch "foo"] remote = . If I change that to 'origin' like I claimed it was, it starts erroring.
Re: Bug in git rev-parse @{push}?
On Thu, Nov 03, 2016 at 03:48:50PM +, Chris Purcell wrote: > Thanks, Jeff! If I remove the explicit configuration of remote.pushdefault > = "origin", I get the same error message as you, so I suspect that's _not_ > the default. That's really bizarre, because I get the same behavior with or without it set. Not only that, but it shouldn't even come into play, as branch.foo.remote should take precedence anyway. So now I'm really puzzled. -Peff
Re: Bug in git rev-parse @{push}?
Resending to mailing list because Inbox is fighting with vger...
On 3 November 2016 at 15:48, Chris Purcell wrote:
> Thanks, Jeff! If I remove the explicit configuration of remote.pushdefault =
> "origin", I get the same error message as you, so I suspect that's _not_ the
> default.
>
> On Thu, 3 Nov 2016 at 15:14 Jeff King wrote:
>>
>> On Thu, Nov 03, 2016 at 02:53:44PM +, Chris Purcell wrote:
>>
>> > I think I have discovered a bug in rev-parse's handling of @{push}:
>> >
>> > $ git push
>> > Everything up-to-date
>> > $ git rev-parse @{push}
>> > fatal: cannot resolve 'simple' push to a single destination
>> >
>> > The documentation for rev-parse says that "the suffix @{push} reports
>> > the branch 'where we would push to' if git push were run while
>> > branchname was checked out", so I would not expect this to error
>> > unless git push does.
>>
>> I'm not too surprised if there's a bug there. IIRC, the way the code is
>> structured, some of the logic had to be reimplemented for @{push} rather
>> than re-used, so there may be corner cases where they do not agree.
>>
>> > The relevant parts of my configuration are:
>> >
>> > [push]
>> > default = simple
>> > [remote]
>> > pushdefault = origin
>> > [branch "foo"]
>> > remote = origin
>> > merge = refs/heads/develop
>> >
>> > The code in branch_get_push_1 (remote.c) in the PUSH_DEFAULT_SIMPLE
>> > case is calling both branch_get_upstream and tracking_for_push_dest
>> > and erroring if they don't return the same result, which I assume is
>> > incorrect for a triangular workflow?
>>
>> I assume you have branch "foo" checked out?
>>
>> With this config I don't see how "git push" would work. Because you're
>> using "simple", it should complain that "develop" and "foo" are not the
>> same name.
>>
>> Can you give a more full reproduction recipe? If I try:
>>
>> git init tmp && cd tmp
>> git config push.default simple
>> git commit -m foo --allow-empty ;# just to have some commit to push
>>
>> git init --bare dst.git
>> git remote add origin dst.git
>> git push origin master:refs/heads/develop
>>
>> git checkout -b foo origin/develop
>>
>> # pushdefault of "origin" is already the default. checkout will have
>> # set up branch.foo.* as you specified. So let's try our push.
>> git push
>>
>> Then I get:
>>
>> fatal: The upstream branch of your current branch does not match
>> the name of your current branch. To push to the upstream branch
>> on the remote, use
>>
>> git push origin HEAD:develop
>>
>> To push to the branch of the same name on the remote, use
>>
>> git push origin foo
>>
>> which makes sense.
>>
>> If you _don't_ get that same message with "git push", then my next
>> question is: might you have any aliases or other systems like "hub" that
>> are munging the arguments to "git push"? Running with "GIT_TRACE=1" in
>> the environment might be enlightening there.
>>
>> > Please let me know if I've missed out important information by
>> > mistake. I'm happy to work on a patch if given guidance, but this is
>> > definitely outside my comfort zone for an unfamiliar codebase
>> > otherwise! e.g. I can't find the test suite.
>>
>> The tests are in the "t" directory; see t/README for details. You can
>> run them all with "make test" from the top-level directory.
>>
>> -Peff
Re: Bug in git rev-parse @{push}?
On Thu, Nov 03, 2016 at 02:53:44PM +, Chris Purcell wrote:
> I think I have discovered a bug in rev-parse's handling of @{push}:
>
> $ git push
> Everything up-to-date
> $ git rev-parse @{push}
> fatal: cannot resolve 'simple' push to a single destination
>
> The documentation for rev-parse says that "the suffix @{push} reports
> the branch 'where we would push to' if git push were run while
> branchname was checked out", so I would not expect this to error
> unless git push does.
I'm not too surprised if there's a bug there. IIRC, the way the code is
structured, some of the logic had to be reimplemented for @{push} rather
than re-used, so there may be corner cases where they do not agree.
> The relevant parts of my configuration are:
>
> [push]
> default = simple
> [remote]
> pushdefault = origin
> [branch "foo"]
> remote = origin
> merge = refs/heads/develop
>
> The code in branch_get_push_1 (remote.c) in the PUSH_DEFAULT_SIMPLE
> case is calling both branch_get_upstream and tracking_for_push_dest
> and erroring if they don't return the same result, which I assume is
> incorrect for a triangular workflow?
I assume you have branch "foo" checked out?
With this config I don't see how "git push" would work. Because you're
using "simple", it should complain that "develop" and "foo" are not the
same name.
Can you give a more full reproduction recipe? If I try:
git init tmp && cd tmp
git config push.default simple
git commit -m foo --allow-empty ;# just to have some commit to push
git init --bare dst.git
git remote add origin dst.git
git push origin master:refs/heads/develop
git checkout -b foo origin/develop
# pushdefault of "origin" is already the default. checkout will have
# set up branch.foo.* as you specified. So let's try our push.
git push
Then I get:
fatal: The upstream branch of your current branch does not match
the name of your current branch. To push to the upstream branch
on the remote, use
git push origin HEAD:develop
To push to the branch of the same name on the remote, use
git push origin foo
which makes sense.
If you _don't_ get that same message with "git push", then my next
question is: might you have any aliases or other systems like "hub" that
are munging the arguments to "git push"? Running with "GIT_TRACE=1" in
the environment might be enlightening there.
> Please let me know if I've missed out important information by
> mistake. I'm happy to work on a patch if given guidance, but this is
> definitely outside my comfort zone for an unfamiliar codebase
> otherwise! e.g. I can't find the test suite.
The tests are in the "t" directory; see t/README for details. You can
run them all with "make test" from the top-level directory.
-Peff
Bug in git rev-parse @{push}?
Hi folks,
I think I have discovered a bug in rev-parse's handling of @{push}:
$ git push
Everything up-to-date
$ git rev-parse @{push}
fatal: cannot resolve 'simple' push to a single destination
The documentation for rev-parse says that "the suffix @{push} reports
the branch 'where we would push to' if git push were run while
branchname was checked out", so I would not expect this to error
unless git push does.
The relevant parts of my configuration are:
[push]
default = simple
[remote]
pushdefault = origin
[branch "foo"]
remote = origin
merge = refs/heads/develop
The code in branch_get_push_1 (remote.c) in the PUSH_DEFAULT_SIMPLE
case is calling both branch_get_upstream and tracking_for_push_dest
and erroring if they don't return the same result, which I assume is
incorrect for a triangular workflow?
Please let me know if I've missed out important information by
mistake. I'm happy to work on a patch if given guidance, but this is
definitely outside my comfort zone for an unfamiliar codebase
otherwise! e.g. I can't find the test suite.
Cheers,
Chris
