git-fetch while on "(no branch)"

git-fetch while on "(no branch)"

[Andy Parkins]

Hello,

Just ran this while on a detached HEAD.

$ git fetch
fatal: ref HEAD is not a symbolic ref
fatal: ref HEAD is not a symbolic ref
fatal: ref HEAD is not a symbolic ref
remote: Generating pack...
...etc...

The fetch appears to have succeeded anyway, but it's a scary message that 
tells a user something is "fatal".  Has this done any damage?


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

[PATCH] git-{parse-remote,pull}: throw away stderr of git-symbolic-ref

[Jeff King]

We look up the destination of the HEAD symref in order to get the
default remote and merge head for the current branch. However, if we
have a detached HEAD, there is no current branch. We handle this
situation by looking up branch..remote, which is empty.
Unfortunately, git-symbolic-ref complained to stderr, potentially
scaring users.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-parse-remote.sh |    4 ++--
 git-pull.sh         |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d2e4c2b..bc2485f 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -49,7 +49,7 @@ get_remote_url () {
 }
 
 get_default_remote () {
-	curr_branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
+	curr_branch=$(git-symbolic-ref HEAD 2>/dev/null | sed -e 's|^refs/heads/||')
 	origin=$(git-repo-config --get "branch.$curr_branch.remote")
 	echo ${origin:-origin}
 }
@@ -137,7 +137,7 @@ canon_refs_list_for_fetch () {
 		shift
 		if test "$remote" = "$(get_default_remote)"
 		then
-			curr_branch=$(git-symbolic-ref HEAD | \
+			curr_branch=$(git-symbolic-ref HEAD 2>/dev/null | \
 			    sed -e 's|^refs/heads/||')
 			merge_branches=$(git-repo-config \
 			    --get-all "branch.${curr_branch}.merge")
diff --git a/git-pull.sh b/git-pull.sh
index 9592617..f0cc023 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -83,7 +83,7 @@ merge_head=$(sed -e '/	not-for-merge	/d' \
 
 case "$merge_head" in
 '')
-	curr_branch=$(git-symbolic-ref HEAD | \
+	curr_branch=$(git-symbolic-ref HEAD 2>/dev/null | \
 		sed -e 's|^refs/heads/||')
 	echo >&2 "Warning: No merge candidate found because value of config option
          \"branch.${curr_branch}.merge\" does not match any remote branch fetched."
-- 
1.5.0.rc1.gd61e-dirty

Re: [PATCH] git-{parse-remote,pull}: throw away stderr of git-symbolic-ref

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We look up the destination of the HEAD symref in order to get the
> default remote and merge head for the current branch. However, if we
> have a detached HEAD, there is no current branch. We handle this
> situation by looking up branch..remote, which is empty.
> Unfortunately, git-symbolic-ref complained to stderr, potentially
> scaring users.

I think what I sent out was a moral equivalent, but generally
speaking I do not think we should discard stderr output
indiscriminatingly.

> diff --git a/git-pull.sh b/git-pull.sh
> index 9592617..f0cc023 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -83,7 +83,7 @@ merge_head=$(sed -e '/	not-for-merge	/d' \
>  
>  case "$merge_head" in
>  '')
> -	curr_branch=$(git-symbolic-ref HEAD | \
> +	curr_branch=$(git-symbolic-ref HEAD 2>/dev/null | \
>  		sed -e 's|^refs/heads/||')
>  	echo >&2 "Warning: No merge candidate found because value of config option
>           \"branch.${curr_branch}.merge\" does not match any remote branch fetched."

I missed this one, but I think this needs a bit more through
rework.  This is what happens when you say "git pull origin"
while your HEAD is detached, and specifying "branch..merge" in
the configuration should not be the way to define the default
remote branch to be merged while on a detached HEAD.  In fact, I
think there should not be any such default.

I think it makes more sense to say "your HEAD is detached and
you need to explicitly say which branch you would want to merge
in" in this case.

[PATCH] git-pull: disallow implicit merging to detached HEAD

[Jeff King]

Instead, we complain to the user and suggest that they explicitly
specify the remote and branch. We depend on the exit status of
git-symbolic-ref, so let's go ahead and document that.

Signed-off-by: Jeff King <peff@peff.net>
---

This is on top of your other patch.

> I think what I sent out was a moral equivalent, but generally
> speaking I do not think we should discard stderr output
> indiscriminatingly.

Agreed.

> I think it makes more sense to say "your HEAD is detached and
> you need to explicitly say which branch you would want to merge
> in" in this case.

Something like this?

 Documentation/git-symbolic-ref.txt |    4 ++++
 git-pull.sh                        |   13 +++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt
index 1e818bb..fec3b4f 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -44,6 +44,10 @@ cumbersome.  On some platforms, `ln -sf` does not even work as
 advertised (horrors).  Therefore symbolic links are now deprecated
 and symbolic refs are used by default.
 
+git-symbolic-ref will exit with status 0 if the contents of the
+symbolic ref were printed correctly, with status 1 if the requested
+name is not a symbolic ref, or 128 if another error occurs.
+
 Author
 ------
 Written by Junio C Hamano <junkio@cox.net>
diff --git a/git-pull.sh b/git-pull.sh
index 9592617..3e96569 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -83,8 +83,17 @@ merge_head=$(sed -e '/	not-for-merge	/d' \
 
 case "$merge_head" in
 '')
-	curr_branch=$(git-symbolic-ref HEAD | \
-		sed -e 's|^refs/heads/||')
+	curr_branch=$(git-symbolic-ref -q HEAD)
+	case $? in
+	  0) ;;
+	  1) echo >&2 "You are not currently on a branch; you must explicitly"
+	     echo >&2 "indicate which branch you wish to merge with"
+	     echo >&2 "  git pull <remote> <branch>"
+	     exit 1;;
+	  *) exit $?;;
+	esac
+	curr_branch=${curr_branch#refs/heads/}
+
 	echo >&2 "Warning: No merge candidate found because value of config option
          \"branch.${curr_branch}.merge\" does not match any remote branch fetched."
 	echo >&2 "No changes."
-- 
1.5.0.rc1.gb329-dirty

Re: [PATCH] git-pull: disallow implicit merging to detached HEAD

[Jeff King]

On Mon, Jan 15, 2007 at 05:25:33PM -0500, Jeff King wrote:

> +	  1) echo >&2 "You are not currently on a branch; you must explicitly"
> +	     echo >&2 "indicate which branch you wish to merge with"
> +	     echo >&2 "  git pull <remote> <branch>"

Urgh. Apparently using --amend is too challenging for me. I think it is
slightly clearer for the second line to read:
  "specify which branch you wish to merge:"

-Peff

Re: git-fetch while on "(no branch)"

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

> Hello,
>
> Just ran this while on a detached HEAD.
>
> $ git fetch
> fatal: ref HEAD is not a symbolic ref
> fatal: ref HEAD is not a symbolic ref
> fatal: ref HEAD is not a symbolic ref
> remote: Generating pack...
> ...etc...
>
> The fetch appears to have succeeded anyway, but it's a scary message that 
> tells a user something is "fatal".  Has this done any damage?

I do not think it has done any damage, but that is certainly alarming.
This might help (untested, of course).

By the way, with or without this patch, there currently is no
good way to tell failure modes between "git symbolic-ref HAED"
and "git symbolic-ref HEAD".  Both says "is not a symbolic ref".

We may want to do something about it.

---


diff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt
index 4bc35a1..1e818bb 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -7,7 +7,7 @@ git-symbolic-ref - read and modify symbolic refs
 
 SYNOPSIS
 --------
-'git-symbolic-ref' <name> [<ref>]
+'git-symbolic-ref' [-q] <name> [<ref>]
 
 DESCRIPTION
 -----------
@@ -23,6 +23,14 @@ A symbolic ref is a regular file that stores a string that
 begins with `ref: refs/`.  For example, your `.git/HEAD` is
 a regular file whose contents is `ref: refs/heads/master`.
 
+OPTIONS
+-------
+
+-q::
+	Do not issue an error message if the <name> is not a
+	symbolic ref but a detached HEAD; instead exit with
+	non-zero status silently.
+
 NOTES
 -----
 In the past, `.git/HEAD` was a symbolic link pointing at
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index d8be052..227c9d4 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -3,9 +3,9 @@
 #include "refs.h"
 
 static const char git_symbolic_ref_usage[] =
-"git-symbolic-ref name [ref]";
+"git-symbolic-ref [-q] name [ref]";
 
-static void check_symref(const char *HEAD)
+static void check_symref(const char *HEAD, int quiet)
 {
 	unsigned char sha1[20];
 	int flag;
@@ -13,17 +13,41 @@ static void check_symref(const char *HEAD)
 
 	if (!refs_heads_master)
 		die("No such ref: %s", HEAD);
-	else if (!(flag & REF_ISSYMREF))
-		die("ref %s is not a symbolic ref", HEAD);
+	else if (!(flag & REF_ISSYMREF)) {
+		if (!quiet)
+			die("ref %s is not a symbolic ref", HEAD);
+		else
+			exit(1);
+	}
 	puts(refs_heads_master);
 }
 
 int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 {
+	int quiet = 0;
+
 	git_config(git_default_config);
+
+	while (1 < argc) {
+		const char *arg = argv[1];
+		if (arg[0] != '-')
+			break;
+		else if (!strcmp("-q", arg))
+			quiet = 1;
+		else if (!strcmp("--", arg)) {
+			argc--;
+			argv++;
+			break;
+		}
+		else
+			die("unknown option %s", arg);
+		argc--;
+		argv++;
+	}
+
 	switch (argc) {
 	case 2:
-		check_symref(argv[1]);
+		check_symref(argv[1], quiet);
 		break;
 	case 3:
 		create_symref(argv[1], argv[2]);
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d2e4c2b..4fc6020 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -49,7 +49,7 @@ get_remote_url () {
 }
 
 get_default_remote () {
-	curr_branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
+	curr_branch=$(git-symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')
 	origin=$(git-repo-config --get "branch.$curr_branch.remote")
 	echo ${origin:-origin}
 }
@@ -137,7 +137,7 @@ canon_refs_list_for_fetch () {
 		shift
 		if test "$remote" = "$(get_default_remote)"
 		then
-			curr_branch=$(git-symbolic-ref HEAD | \
+			curr_branch=$(git-symbolic-ref -q HEAD | \
 			    sed -e 's|^refs/heads/||')
 			merge_branches=$(git-repo-config \
 			    --get-all "branch.${curr_branch}.merge")

Re: git-fetch while on "(no branch)"

[Jeff King]

On Mon, Jan 15, 2007 at 01:56:05PM -0800, Junio C Hamano wrote:

> -'git-symbolic-ref' <name> [<ref>]
> +'git-symbolic-ref' [-q] <name> [<ref>]

Please ignore my patch in this thread; yours is much more sensible.
However, if you do go this route, note that git-pull needs to use the
'-q' flag as well.

-Peff