git describe fails without tags

git describe fails without tags

[Uwe Zeisberger]

Hello,

using git describe on Linus' sparse repo results in:

	uzeisberger@io:~/gsrc/sparse$ git describe
	fatal: cannot describe '62ac6c16058aba8693fd5827379debc5f57b60f5'

The reason is, that there exist no tags at all, so right, there is no
"most recent tag that is reachable from HEAD".

I wonder if it would be sane to assume an implicit tag for the empty
repository, s.t. git describe results in 

	<empty>-62ac6c16

(whatever name is choosen for <empty>).

Any opinions?

Best regards
Uwe

-- 
Uwe Zeisberger

http://www.google.com/search?q=1+degree+celsius+in+kelvin

Re: git describe fails without tags

[Junio C Hamano]

Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de> writes:

> I wonder if it would be sane to assume an implicit tag for the empty
> repository, s.t. git describe results in 
>
> 	<empty>-62ac6c16
>
> (whatever name is choosen for <empty>).
>
> Any opinions?

The value of 'describe' is not in the 62ac6c part, but in the
tag part, which lets us find out that the rev in question is at
least newer than that tag.  If the reason you are doing
"describe" is to find out an usable abbreviated name, you could
feed the first few hexdigits to "git rev-parse --verify",
lengthening the prefix longer by one until it says you have a
unique prefix [*1*].

In other words, not particularly interested, although it is
trivial to implement, like this:

diff --git a/describe.c b/describe.c
index 4866510..aeaf0fb 100644
--- a/describe.c
+++ b/describe.c
@@ -137,7 +137,7 @@ static void describe(char *arg, int last
 			return;
 		}
 	}
-	die("cannot describe '%s'", sha1_to_hex(cmit->object.sha1));
+	printf("%s\n", find_unique_abbrev(cmit->object.sha1, abbrev));
 }
 
 int main(int argc, char **argv)


[Footnote]

*1* If you do this often, we could introduce

	$ git rev-parse --abbrev=<n> HEAD

    that quacks like --verify (i.e. makes sure there is a single
    "extended SHA1 expression" that evaluates to a valid object
    name) but outputs the result abbreviated to at least <n>
    hexdigits.
    
    This has an added advantage that it would work on a
    non-commit object name.


-- >8 --
[PATCH] rev-parse: --abbrev option.

The new option behaves just like --verify, but outputs an
abbreviated object name that is unique within the repository.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 rev-parse.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

3c7c31a913c3c4060e85f2919e2136412928dcec
diff --git a/rev-parse.c b/rev-parse.c
index 0c951af..c1646e4 100644
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -20,6 +20,7 @@ static char *def = NULL;
 #define REVERSED 1
 static int show_type = NORMAL;
 static int symbolic = 0;
+static int abbrev = 0;
 static int output_sq = 0;
 
 static int revs_count = 0;
@@ -95,6 +96,8 @@ static void show_rev(int type, const uns
 		putchar('^');
 	if (symbolic && name)
 		show(name);
+	else if (abbrev)
+		show(find_unique_abbrev(sha1, abbrev));
 	else
 		show(sha1_to_hex(sha1));
 }
@@ -195,6 +198,17 @@ int main(int argc, char **argv)
 				verify = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--abbrev") ||
+			    !strncmp(arg, "--abbrev=", 9)) {
+				filter &= ~(DO_FLAGS|DO_NOREV);
+				verify = 1;
+				abbrev = DEFAULT_ABBREV;
+				if (arg[8] == '=')
+					abbrev = strtoul(arg + 9, NULL, 10);
+				if (abbrev < 0 || 40 <= abbrev)
+					abbrev = DEFAULT_ABBREV;
+				continue;
+			}
 			if (!strcmp(arg, "--sq")) {
 				output_sq = 1;
 				continue;
-- 
1.1.4.g869a

Re: git describe fails without tags

[Uwe Zeisberger]

Hello Junio,

Junio C Hamano wrote:
> Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de> writes:
> 
> > I wonder if it would be sane to assume an implicit tag for the empty
> > repository, s.t. git describe results in 
> >
> > 	<empty>-62ac6c16
> >
> > (whatever name is choosen for <empty>).
> >
> > Any opinions?
>
> [...] If the reason you are doing "describe" is to find out an usable
> abbreviated name, you could feed the first few hexdigits to "git
> rev-parse --verify", lengthening the prefix longer by one until it
> says you have a unique prefix [*1*].
Yes, I wrote a script that automatically build git and install it to
${HOME}/usr/stow/git-`git describe HEAD` and then stow(8)s it.  Writing
a similar script for sparse cannot use git describe because there are no
tags ...
 
> In other words, not particularly interested, although it is
> trivial to implement, like this:
> 
> diff --git a/describe.c b/describe.c
> index 4866510..aeaf0fb 100644
> --- a/describe.c
> +++ b/describe.c
> @@ -137,7 +137,7 @@ static void describe(char *arg, int last
>  			return;
>  		}
>  	}
> -	die("cannot describe '%s'", sha1_to_hex(cmit->object.sha1));
> +	printf("%s\n", find_unique_abbrev(cmit->object.sha1, abbrev));
>  }
>  
>  int main(int argc, char **argv)

It's a pity your not particularly interested, I like that patch's idea.
git describe dies with an error here in a situation where there is the
possibility to do something sensible.

BTW, for the sake of consistency, I'd suggest

-	die("cannot describe '%s'", sha1_to_hex(cmit->object.sha1));
+	printf("g%s\n", find_unique_abbrev(cmit->object.sha1, abbrev));

Best regards
Uwe

-- 
Uwe Zeisberger

http://www.google.com/search?q=1+year+divided+by+3+in+seconds

Re: git describe fails without tags

[Junio C Hamano]

Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de> writes:

> Yes, I wrote a script that automatically build git and install it to
> ${HOME}/usr/stow/git-`git describe HEAD` and then stow(8)s it.  Writing
> a similar script for sparse cannot use git describe because there are no
> tags ...
> ...
> It's a pity your not particularly interested, I like that patch's idea.
> git describe dies with an error here in a situation where there is the
> possibility to do something sensible.

I think I understand the problem pretty well, and actually I am
sympathetic to the cause.

Having said that, I do not agree with the approach of your
patch.  It makes it inconvenient for scripts to tell describable
and indescribable revs apart by checking the exit status from
the command.

In other words, I think a sensible thing can be done more sanely
in your script.  Something like this?

	#!/bin/sh
	project=git ;# or 'sparse'
	version=`git describe HEAD` ||
		version=untagged-g`git rev-parse --abbrev HEAD`
	$(MAKE) prefix=$HOME/usr/stow/$project-$version

If you want to squelch the error message from indescribable rev,
you could do:

	version=`git describe HEAD 2>/dev/null` ||

of course.

Re: git describe fails without tags

[Uwe Zeisberger]

Hello Junio,

Junio C Hamano wrote:
> Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de> writes:
> 
> > Yes, I wrote a script that automatically build git and install it to
> > ${HOME}/usr/stow/git-`git describe HEAD` and then stow(8)s it.  Writing
> > a similar script for sparse cannot use git describe because there are no
> > tags ...
> > ...
> > It's a pity your not particularly interested, I like that patch's idea.
> > git describe dies with an error here in a situation where there is the
> > possibility to do something sensible.
> 
> I think I understand the problem pretty well, and actually I am
> sympathetic to the cause.
> 
> Having said that, I do not agree with the approach of your
> patch.  It makes it inconvenient for scripts to tell describable
> and indescribable revs apart by checking the exit status from
> the command.
OK, that's fine for me.  Then it would be sensible to add a note to the
docs describing the exit codes.

Thanks for your suggestion for my script.

Best regards
Uwe

-- 
Uwe Zeisberger

http://www.google.com/search?q=sin%28pi%2F2%29

Re: git describe fails without tags

[Uwe Zeisberger]

Hello Junio,

Junio C Hamano wrote:
> diff --git a/rev-parse.c b/rev-parse.c
> index 0c951af..c1646e4 100644
> --- a/rev-parse.c
> +++ b/rev-parse.c
> @@ -20,6 +20,7 @@ static char *def = NULL;
>  #define REVERSED 1
>  static int show_type = NORMAL;
>  static int symbolic = 0;
> +static int abbrev = 0;
>  static int output_sq = 0;
>  
>  static int revs_count = 0;
> @@ -95,6 +96,8 @@ static void show_rev(int type, const uns
>  		putchar('^');
>  	if (symbolic && name)
>  		show(name);
> +	else if (abbrev)
> +		show(find_unique_abbrev(sha1, abbrev));
>  	else
>  		show(sha1_to_hex(sha1));
>  }
> @@ -195,6 +198,17 @@ int main(int argc, char **argv)
>  				verify = 1;
>  				continue;
>  			}
> +			if (!strcmp(arg, "--abbrev") ||
> +			    !strncmp(arg, "--abbrev=", 9)) {
> +				filter &= ~(DO_FLAGS|DO_NOREV);
> +				verify = 1;
> +				abbrev = DEFAULT_ABBREV;
> +				if (arg[8] == '=')
> +					abbrev = strtoul(arg + 9, NULL, 10);
> +				if (abbrev < 0 || 40 <= abbrev)
> +					abbrev = DEFAULT_ABBREV;
> +				continue;
> +			}
>  			if (!strcmp(arg, "--sq")) {
>  				output_sq = 1;
>  				continue;
I see two things to fix in that patch:

 1) define DEFAULT_ABBREV (e.g. by moving it to cache.h, where
    find_unique_abbrev is defined.)

 2) describe.c allows only abbrev >= 4.  (Allowing values less than 2
    failes, because find_short_object_filename (and maybe others) assume
    len to be at least 2.)  I think 4 is sensible.

This results in the following patch:

--8<--
[PATCH] rev-parse: --abbrev option.

The new option behaves just like --verify, but outputs an abbreviated object
name that is unique within the repository.

This patch is a modification of a suggestion by Junio C Hamano.

Signed-off-by: Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de>

---

 cache.h     |    2 ++
 describe.c  |    1 -
 rev-parse.c |   14 ++++++++++++++
 3 files changed, 16 insertions(+), 1 deletions(-)

0d43ec7461b38d6a1d1563fd7dc2ebf399eabe9e
diff --git a/cache.h b/cache.h
index b493b65..139c670 100644
--- a/cache.h
+++ b/cache.h
@@ -177,6 +177,8 @@ extern int check_repository_format(void)
 #define DATA_CHANGED    0x0020
 #define TYPE_CHANGED    0x0040
 
+#define DEFAULT_ABBREV 8 /* maybe too many */
+
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/describe.c b/describe.c
index 4866510..6518f06 100644
--- a/describe.c
+++ b/describe.c
@@ -11,7 +11,6 @@ static const char describe_usage[] =
 static int all = 0;	/* Default to annotated tags only */
 static int tags = 0;	/* But allow any tags if --tags is specified */
 
-#define DEFAULT_ABBREV 8 /* maybe too many */
 static int abbrev = DEFAULT_ABBREV;
 
 static int names = 0, allocs = 0;
diff --git a/rev-parse.c b/rev-parse.c
index 0c951af..58cff6f 100644
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -20,6 +20,7 @@ static char *def = NULL;
 #define REVERSED 1
 static int show_type = NORMAL;
 static int symbolic = 0;
+static int abbrev = 0;
 static int output_sq = 0;
 
 static int revs_count = 0;
@@ -95,6 +96,8 @@ static void show_rev(int type, const uns
 		putchar('^');
 	if (symbolic && name)
 		show(name);
+	else if (abbrev)
+		show(find_unique_abbrev(sha1, abbrev));
 	else
 		show(sha1_to_hex(sha1));
 }
@@ -195,6 +198,17 @@ int main(int argc, char **argv)
 				verify = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--abbrev") ||
+					!strncmp(arg, "--abbrev=", 9)) {
+				filter &= ~(DO_FLAGS|DO_NOREV);
+				verify = 1;
+				abbrev = DEFAULT_ABBREV;
+				if (arg[8] == '=')
+					abbrev = strtoul(arg + 9, NULL, 10);
+				if (abbrev < 4 || 40 <= abbrev)
+					abbrev = DEFAULT_ABBREV;
+				continue;
+			}
 			if (!strcmp(arg, "--sq")) {
 				output_sq = 1;
 				continue;
-- 
1.1.4.g3e6c

Best regards
Uwe

-- 
Uwe Zeisberger

http://www.google.com/search?q=72+PS+point+in+inch

Re: git describe fails without tags

[Junio C Hamano]

Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de> writes:

> I see two things to fix in that patch:
>
>  1) define DEFAULT_ABBREV (e.g. by moving it to cache.h, where
>     find_unique_abbrev is defined.)

Sorry, the patch alone was not compilable since cleaning up the
definition of symbolic constants *_ABBREV comes before the patch
you quoted in the "pu" branch.

>  2) describe.c allows only abbrev >= 4.  (Allowing values less than 2
>     failes, because find_short_object_filename (and maybe others) assume
>     len to be at least 2.)  I think 4 is sensible.

Thanks.  "rev-parse --abbrev=2" would have segfaulted without
your fix.  I suspect substituting with MINIMUM instead of
DEFAULT in such a case would be more sensible, so...

-- >8 --
[PATCH] rev-parse --abbrev: do not try abbrev shorter than minimum.

We do not allow abbreviation shorter than 4 letters in other
parts of the system so do not attempt to generate such.

Noticed by Uwe Zeisberger.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 rev-parse.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

030d25d271adb2671f560b410d77585d8744acbf
diff --git a/rev-parse.c b/rev-parse.c
index 42969a6..8bf316e 100644
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -206,8 +206,10 @@ int main(int argc, char **argv)
 				abbrev = DEFAULT_ABBREV;
 				if (arg[8] == '=')
 					abbrev = strtoul(arg + 9, NULL, 10);
-				if (abbrev < 0 || 40 <= abbrev)
-					abbrev = DEFAULT_ABBREV;
+				if (abbrev < MINIMUM_ABBREV)
+					abbrev = MINIMUM_ABBREV;
+				else if (40 <= abbrev)
+					abbrev = 40;
 				continue;
 			}
 			if (!strcmp(arg, "--sq")) {
-- 
1.1.4.g00a4