patchparse-remote: handle detached HEAD

In get_remote_merge_branch 'git for-each-ref' is used to know the upstream branch of the current branch ($curr_branch). But $curr_branch can be empty when in detached HEAD, so the call to for-each-ref is made without a pattern.

Quote the $curr_branch variable in the git for-each-ref call to always provide a pattern (the current branch or an empty string) Otherwise it would mean all refs.

This fixes a bug reported by Sverre Rabbelier. The overall results were correct but not the output text.

Signed-off-by: Santi Béjar <santi@agolina.net>
---
Hi *,

Sorry Sverre, but the patch you tested fixed your case but broke all the others :(

Hope you can also test it. Now it passes the test suite.

Thanks,
Santi
 git-parse-remote.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Heya,

On Mon, Dec 6, 2010 at 00:47, Santi Béjar <santi@agolina.net> wrote:

I think we usually use:

Reported-by: Sverre Rabbelier <srabbelier@gmail.com>

But since I already verified the fix, perhaps just:

Tested-by: Sverre Rabbelier <srabbelier@gmail.com>

is enough? Both are fine with me.

In get_remote_merge_branch 'git for-each-ref' is used to know the upstream branch of the current branch ($curr_branch). But $curr_branch can be empty when in detached HEAD, so the call to for-each-ref is made without a pattern.

Quote the $curr_branch variable in the git for-each-ref call to always provide a pattern (the current branch or an empty string) Otherwise it would mean all refs.

Reported-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Santi Béjar <santi@agolina.net>
Tested-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Changes since v1:
  Tags for Reported-by and Tested-by.

 git-parse-remote.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Santi Béjar <santi@agolina.net> writes:

What output do you want to see in this case? "Nothing needs to be reported because on detached head you are not tracking anything?"

If that is the case, shouldn't we be not calling "echo" at all to begin with? IOW, shouldn't the code read more like this?

	curr_branch=$(git symbolic-ref -q HEAD) &&
        test "$origin" = "$default" &&
	echo ...

get_remote_merge_branch with zero or one arguments returns the upstream branch. But a detached HEAD does no have an upstream branch, as it is not tracking anything. Handle this case testing the exit code of "git symbolic-ref -q HEAD".

Reported-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Santi Béjar <santi@agolina.net>
---

Or course, you are right. I didn't know/think about the exit code... Thanks.

Santi

 git-parse-remote.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Mon, Dec 6, 2010 at 11:20 AM, Santi Béjar <santi@agolina.net> wrote:

Now that I think of... the final form of the patch is yours (Junio). Feel free to add something like this to the commit message:

Final patch form by Junio C Hamano

Or alternatively, take ownership of the patch and add something like "Patch handled by Santi Béjar but final patch form by Junio C Hamano" and:

Acked-by: Santi Béjar <santi@agolina.net>

Santi

Santi Béjar <santi@agolina.net> writes:

The calling codepath in git-pull that wants to determine remoteref and oldremoteref seems to expect get-remote-merge-branch to succeed in order to find its $oldremoteref variable, and returning false in detached HEAD case here will change what happens there---it won't run "rev-list -g" anymore and quits the codepath early, leaving the variable empty.

But we do want to set the variable to an empty string in this case anyway, so there is no harm done (it probably is what we actually want to happen).

So this should be Ok. Sverre, do you want to do another round of testing just to be sure before I apply this?

Heya,

On Mon, Dec 6, 2010 at 17:03, Junio C Hamano <gitster@pobox.com> wrote:

Yup:

Tested-by: Sverre Rabbelier <srabbelier@gmail.com