git describe is failing

git describe is failing

[Eugene Sajine]

Hi,

I have a test repo which I'm playing with. It has about 15 commits and
one branch only (master) and couple of tags. Somehow it got to a state
when

$ git describe

gives:

fatal: cannot describe 'SHA-1'

The command is working ok with my other repo. It doesn't seem that the
test repo is corrupted. I can commit, push and pull, see the history
with gitk... How can I check or repair it?

Thanks,
Eugene

Re: git describe is failing

[Mikael Magnusson]

2009/10/22 Eugene Sajine <euguess@gmail.com>:
> Hi,
>
> I have a test repo which I'm playing with. It has about 15 commits and
> one branch only (master) and couple of tags. Somehow it got to a state
> when
>
> $ git describe
>
> gives:
>
> fatal: cannot describe 'SHA-1'
>
> The command is working ok with my other repo. It doesn't seem that the
> test repo is corrupted. I can commit, push and pull, see the history
> with gitk... How can I check or repair it?
>
> Thanks,
> Eugene

Your tags are probably not annotated (lightweight tags), use git
describe --tags.

-- 
Mikael Magnusson

Re: git describe is failing

[Thomas Rast]

Eugene Sajine wrote:
> $ git describe
> 
> gives:
> 
> fatal: cannot describe 'SHA-1'

man git-describe:

DESCRIPTION
       The command finds the most recent tag that is reachable from a
       commit.
[...]
       By default (without --all or --tags) git describe only shows
       annotated tags. For more information about creating annotated
       tags see the -a and -s options to git-tag(1).

If there is no such tag, it refuses to work.  Annotate your tags, or
try --always.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: git describe is failing

[Eugene Sajine]

> man git-describe:
>
> DESCRIPTION
>       The command finds the most recent tag that is reachable from a
>       commit.
> [...]
>       By default (without --all or --tags) git describe only shows
>       annotated tags. For more information about creating annotated
>       tags see the -a and -s options to git-tag(1).
>
> If there is no such tag, it refuses to work.  Annotate your tags, or
> try --always.
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
>

Thanks! It is working ok.
Although it is probably not the best error handling.
I believe git should fail with some meaningful message in this case...

Thanks,
Eugene

[PATCH] describe: when failing, tell the user about options that work

[Thomas Rast]

Users seem to call git-describe without reading the manpage, and then
wonder why it doesn't work with unannotated tags by default.

Make a minimal effort towards seeing if there would have been
unannotated tags, and tell the user.  Specifically, we say that --tags
could work if we found any unannotated tags.  If not, we say that
--always would have given results.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Eugene Sajine wrote:
> [git-describe fails if you don't have annotated tags]
>  
> Thanks! It is working ok.
> Although it is probably not the best error handling.
> I believe git should fail with some meaningful message in this case...

We already had most of the information available, so hey, why not.


 builtin-describe.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/builtin-describe.c b/builtin-describe.c
index 2dcfd3d..1ffa0d8 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -96,8 +96,6 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void 
 	if (!all) {
 		if (!prio)
 			return 0;
-		if (!tags && prio < 2)
-			return 0;
 	}
 	add_to_known_names(all ? path + 5 : path + 10, commit, prio, sha1);
 	return 0;
@@ -184,6 +182,7 @@ static void describe(const char *arg, int last_one)
 	struct possible_tag all_matches[MAX_TAGS];
 	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
 	unsigned long seen_commits = 0;
+	unsigned int unannotated_cnt = 0;
 
 	if (get_sha1(arg, sha1))
 		die("Not a valid object name %s", arg);
@@ -217,7 +216,9 @@ static void describe(const char *arg, int last_one)
 		seen_commits++;
 		n = c->util;
 		if (n) {
-			if (match_cnt < max_candidates) {
+			if (!tags && !all && n->prio < 2) {
+				unannotated_cnt++;
+			} else if (match_cnt < max_candidates) {
 				struct possible_tag *t = &all_matches[match_cnt++];
 				t->name = n;
 				t->depth = seen_commits - 1;
@@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
 			printf("%s\n", find_unique_abbrev(sha1, abbrev));
 			return;
 		}
-		die("cannot describe '%s'", sha1_to_hex(sha1));
+		if (unannotated_cnt)
+			die("cannot describe '%s'"
+			    " with only\nannotated tags. Try --tags.",
+			    sha1_to_hex(sha1));
+		else
+			die("cannot describe '%s'"
+			    " with tags\nTry --always, or create some tags.",
+			    sha1_to_hex(sha1));
 	}
 
 	qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
-- 
1.6.5.1.70.g1383ae

Re: [PATCH] describe: when failing, tell the user about options that work

[Alex Riesen]

On Thu, Oct 22, 2009 at 17:44, Thomas Rast <trast@student.ethz.ch> wrote:
> @@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
>                        printf("%s\n", find_unique_abbrev(sha1, abbrev));
>                        return;
>                }
> -               die("cannot describe '%s'", sha1_to_hex(sha1));
> +               if (unannotated_cnt)
> +                       die("cannot describe '%s'"
> +                           " with only\nannotated tags. Try --tags.",
> +                           sha1_to_hex(sha1));
> +               else
> +                       die("cannot describe '%s'"
> +                           " with tags\nTry --always, or create some tags.",
> +                           sha1_to_hex(sha1));

These are quite verbose. Could they be conditional on something like
advice.describeHints?

Re: [PATCH] describe: when failing, tell the user about options that work

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> These are quite verbose. Could they be conditional on something like
> advice.describeHints?

Concurred.  Please make it so.

Re: [PATCH] describe: when failing, tell the user about options that work

[Nicolas Pitre]

On Thu, 22 Oct 2009, Junio C Hamano wrote:

> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > These are quite verbose. Could they be conditional on something like
> > advice.describeHints?
> 
> Concurred.  Please make it so.

I don't have a strong opinion either ways, but is having a less verbose 
message in the _error_ path really a big issue?


Nicolas

Re: [PATCH] describe: when failing, tell the user about options that work

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> @@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
>  			printf("%s\n", find_unique_abbrev(sha1, abbrev));
>  			return;
>  		}
> -		die("cannot describe '%s'", sha1_to_hex(sha1));
> +		if (unannotated_cnt)
> +			die("cannot describe '%s'"
> +			    " with only\nannotated tags. Try --tags.",

Did you mean UNannotated tags here?

> +			    sha1_to_hex(sha1));
> +		else
> +			die("cannot describe '%s'"
> +			    " with tags\nTry --always, or create some tags.",
> +			    sha1_to_hex(sha1));
>  	}
>  
>  	qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);

Re: [PATCH] describe: when failing, tell the user about options that work

[Junio C Hamano]

Nicolas Pitre <nico@fluxnic.net> writes:

> On Thu, 22 Oct 2009, Junio C Hamano wrote:
>
>> Alex Riesen <raa.lkml@gmail.com> writes:
>> 
>> > These are quite verbose. Could they be conditional on something like
>> > advice.describeHints?
>> 
>> Concurred.  Please make it so.
>
> I don't have a strong opinion either ways, but is having a less verbose 
> message in the _error_ path really a big issue?

Thanks for catching a thinko---I somehow thought the patch was about
warning when we give an annotated tag that is farther than an unannotated
one.

It should be good enough to unconditionally give hints upon an error, as
long as they are correct.

Re: [PATCH] describe: when failing, tell the user about options that work

[Thomas Rast]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > @@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
> >  			printf("%s\n", find_unique_abbrev(sha1, abbrev));
> >  			return;
> >  		}
> > -		die("cannot describe '%s'", sha1_to_hex(sha1));
> > +		if (unannotated_cnt)
> > +			die("cannot describe '%s'"
> > +			    " with only\nannotated tags. Try --tags.",
> 
> Did you mean UNannotated tags here?

No, but I think I see where the misunderstanding came from.

This code path means that we did not find a tag to describe with, but
we counted some unannotated tags (and because of how the counting
logic is wrapped, this only triggers when neither --all nor --tags are
in effect).

So I wanted it to say "it is impossible to describe this with the tags
you told me to use", which in this case are the annotated ones.

I tried to keep the general structure of the message ("cannot describe
..."), and with this restriction I can't seem to find a clearer
wording.  However, it could be written e.g.

  No annotated tags can describe '%s'.  However, there were
  unannotated tags: try --tags.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] describe: when failing, tell the user about options that work

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano wrote:
>> Thomas Rast <trast@student.ethz.ch> writes:
>> 
>> > @@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
>> >  			printf("%s\n", find_unique_abbrev(sha1, abbrev));
>> >  			return;
>> >  		}
>> > -		die("cannot describe '%s'", sha1_to_hex(sha1));
>> > +		if (unannotated_cnt)
>> > +			die("cannot describe '%s'"
>> > +			    " with only\nannotated tags. Try --tags.",
>> 
>> Did you mean UNannotated tags here?
>
> No, but I think I see where the misunderstanding came from.
>
> This code path means that we did not find a tag to describe with, but
> we counted some unannotated tags (and because of how the counting
> logic is wrapped, this only triggers when neither --all nor --tags are
> in effect).

I think I read the code right ;-).

> So I wanted it to say "it is impossible to describe this with the tags
> you told me to use", which in this case are the annotated ones.

The way I read it was "it is impossible to describe it in the way you told
me to, when the tags you have are only unannotated kind."

> However, it could be written e.g.
>
>   No annotated tags can describe '%s'.  However, there were
>   unannotated tags: try --tags.

Sounds better.

[PATCH v2] describe: when failing, tell the user about options that work

[Thomas Rast]

Users seem to call git-describe without reading the manpage, and then
wonder why it doesn't work with unannotated tags by default.

Make a minimal effort towards seeing if there would have been
unannotated tags, and tell the user.  Specifically, we say that --tags
could work if we found any unannotated tags.  If not, we say that
--always would have given results.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> > However, it could be written e.g.
> >
> >   No annotated tags can describe '%s'.  However, there were
> >   unannotated tags: try --tags.
> 
> Sounds better.

Then let's make it so.  Sorry for taking so long.


 builtin-describe.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/builtin-describe.c b/builtin-describe.c
index 2dcfd3d..4ea6f88 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -96,8 +96,6 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void 
 	if (!all) {
 		if (!prio)
 			return 0;
-		if (!tags && prio < 2)
-			return 0;
 	}
 	add_to_known_names(all ? path + 5 : path + 10, commit, prio, sha1);
 	return 0;
@@ -184,6 +182,7 @@ static void describe(const char *arg, int last_one)
 	struct possible_tag all_matches[MAX_TAGS];
 	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
 	unsigned long seen_commits = 0;
+	unsigned int unannotated_cnt = 0;
 
 	if (get_sha1(arg, sha1))
 		die("Not a valid object name %s", arg);
@@ -217,7 +216,9 @@ static void describe(const char *arg, int last_one)
 		seen_commits++;
 		n = c->util;
 		if (n) {
-			if (match_cnt < max_candidates) {
+			if (!tags && !all && n->prio < 2) {
+				unannotated_cnt++;
+			} else if (match_cnt < max_candidates) {
 				struct possible_tag *t = &all_matches[match_cnt++];
 				t->name = n;
 				t->depth = seen_commits - 1;
@@ -259,7 +260,14 @@ static void describe(const char *arg, int last_one)
 			printf("%s\n", find_unique_abbrev(sha1, abbrev));
 			return;
 		}
-		die("cannot describe '%s'", sha1_to_hex(sha1));
+		if (unannotated_cnt)
+			die("No annotated tags can describe '%s'.\n"
+			    "However, there were unannotated tags: try --tags.",
+			    sha1_to_hex(sha1));
+		else
+			die("No tags can describe '%s'.\n"
+			    "Try --always, or create some tags.",
+			    sha1_to_hex(sha1));
 	}
 
 	qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
-- 
1.6.5.1.161.g3b9c0