git describe bug?

git describe bug?

[Dustin Sallings]

	describe does not choose the most recent tag when multiple tags point to a single commit (for example, when an RC release becomes a proper release).

	There's a bit of conflict in the documentation between the following:

		``tags with newer dates will always be preferred over tags with older dates''

	and the next sentence:

		``If an exact match is found, its name will be output and searching will stop.''

	The code does not allow for multiple exact matches, leading to what I would consider incorrect behavior as shown below:


dhcp-103:/tmp/x 1212% git init
Initialized empty Git repository in /private/tmp/x/.git/
dhcp-103:/tmp/x 1213% git config user.email me@example.com
dhcp-103:/tmp/x 1214% git commit --allow-empty -m initial
[master (root-commit) 7eb97db] initial
dhcp-103:/tmp/x 1215% git commit --allow-empty -m second
[master 86544e0] second
dhcp-103:/tmp/x 1216% git tag -m first 1.0
dhcp-103:/tmp/x 1217% git describe                      # expect 1.0
1.0
dhcp-103:/tmp/x 1218% git tag -m second 2.0
dhcp-103:/tmp/x 1219% git describe                      # expect 2.0
1.0
dhcp-103:/tmp/x 1220% git show-ref 
86544e0414d9d4bc0767eed1ffe36b655638fe81 refs/heads/master
2594a45768685b7fbf69cf2e2fca087cdf3cbfec refs/tags/1.0
175a0473faa32cacff0b42f165d39a9547025432 refs/tags/2.0
dhcp-103:/tmp/x 1221% git cat-file tag refs/tags/1.0
object 86544e0414d9d4bc0767eed1ffe36b655638fe81
type commit
tag 1.0
tagger Dustin Sallings <me@example.com> 1270227996 -0700

first
0.000u 0.001s 0:00.00 0.0%	0+0k 0+0io 0pf+0w
dhcp-103:/tmp/x 1222% git cat-file tag refs/tags/2.0
object 86544e0414d9d4bc0767eed1ffe36b655638fe81
type commit
tag 2.0
tagger Dustin Sallings <me@example.com> 1270228008 -0700

second


-- 
Dustin Sallings

Re: git describe bug?

[Shawn O. Pearce]

Dustin Sallings <dustin@spy.net> wrote:
> 
> 	describe does not choose the most recent tag when multiple tags point to a single commit (for example, when an RC release becomes a proper release).
> 
> 	There's a bit of conflict in the documentation between the following:
> 
> 		``tags with newer dates will always be preferred over tags with older dates''
> 
> 	and the next sentence:
> 
> 		``If an exact match is found, its name will be output and searching will stop.''
> 
> 	The code does not allow for multiple exact matches, leading to what I would consider incorrect behavior as shown below:

Yes, I've seen this too.  IIRC we've actually discussed this in
the past.  I can't find the thread (my search skills are sub-par
despite who I work for...).  But the general idea if I remember
it right was we wanted to use the older tag, because that tag
came first.

So its probably more a documentation bug than a software bug.

-- 
Shawn.

Re: git describe bug?

[Andreas Ericsson]

On 04/02/2010 08:31 PM, Shawn O. Pearce wrote:
> Dustin Sallings<dustin@spy.net>  wrote:
>>
>> 	describe does not choose the most recent tag when multiple tags point to a single commit (for example, when an RC release becomes a proper release).
>>
>> 	There's a bit of conflict in the documentation between the following:
>>
>> 		``tags with newer dates will always be preferred over tags with older dates''
>>
>> 	and the next sentence:
>>
>> 		``If an exact match is found, its name will be output and searching will stop.''
>>
>> 	The code does not allow for multiple exact matches, leading to what I would consider incorrect behavior as shown below:
> 
> Yes, I've seen this too.  IIRC we've actually discussed this in
> the past.  I can't find the thread (my search skills are sub-par
> despite who I work for...).  But the general idea if I remember
> it right was we wanted to use the older tag, because that tag
> came first.
> 

Right now the behaviour is inconsistent. In the latest git repo
I tried the following:
  $ git tag -a v99 -msdf
  $ git tag -a v100 -m100
  $ git describe
v100
  $ git describe v99
v100
  $ git describe v100
v100
  $ git show v99
tag v99
Tagger: Andreas Ericsson <ae@op5.se>
Date:   Tue Apr 6 11:27:28 2010 +0200

sdf

commit 8b5fe8c9ec3f961ec9b09844eca6225f06af5b0b
...
  $ git show v100
tag v100
Tagger: Andreas Ericsson <ae@op5.se>
Date:   Tue Apr 6 11:27:37 2010 +0200

100

commit 8b5fe8c9ec3f961ec9b09844eca6225f06af5b0b
...

  $ ls .git/refs/tags/v{100,99}
.git/refs/tags/v100  .git/refs/tags/v99

So as you can see, something else is going on. Both tags here are
stashed as loose objects and both refs are unpacked.

Using either strverscmp(3) or allowing one to sort by tagging date
with the option to use first or latest would be preferrable. I'll
see what I can cook up during my lunch-break.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: git describe bug?

[Shawn O. Pearce]

Andreas Ericsson <ae@op5.se> wrote:
> Right now the behaviour is inconsistent.
...
> So as you can see, something else is going on. Both tags here are
> stashed as loose objects and both refs are unpacked.

Its the order the names were iterated out of the directory by
readdir().  Or the order they were sorted by lexographical ordering
during for_each_ref().  I can't remember if for_each_ref() sorts
the items before processing.

But either describe uses the first item it found for a given commit
if they have the same priority.  There's no notion of date being
factored into the decision of which tag to keep.

I'll work up a patch right now.

-- 
Shawn.

[PATCH] describe: Break annotated tag ties by tagger date

[Shawn O. Pearce]

If more than one annotated tag points at the same commit, use the
tag whose tagger field has a more recent date stamp.  This resolves
non-deterministic cases where the maintainer has done:

  $ git tag -a -m "2.1-rc1" v2.1-rc1  deadbeef
  $ git tag -a -m "2.1"     v2.1      deadbeef

If the tag is an older-style annotated tag with no tagger date,
we assume a date stamp of 1 second after the UNIX epoch. This will
cause us to prefer an annotated tag that has a valid date, or to
simply avoid scanning the tag object again if a 3rd tag was found
for the same commit.

We could also try to consider the tag object chain, favoring a tag
that "includes" another one:

  $ git tag -a -m "2.1-rc0" v2.1-rc1  deadbeef
  $ git tag -a -m "2.1"     v2.1      v2.1-rc1

However traversing the tag's object chain looking for inclusion
is much more complicated.  Its already very likely that even in
these cases the v2.1 tag will have a more recent tagger date than
v2.1-rc1, so with this change describe should still resolve this
by selecting the more recent v2.1.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin/describe.c  |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t6120-describe.sh |    8 ++++--
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 71be2a9..cf98664 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -36,6 +36,7 @@ static const char *diff_index_args[] = {
 struct commit_name {
 	struct tag *tag;
 	int prio; /* annotated tag = 2, tag = 1, head = 0 */
+	unsigned long date;
 	unsigned char sha1[20];
 	char path[FLEX_ARRAY]; /* more */
 };
@@ -43,18 +44,74 @@ static const char *prio_names[] = {
 	"head", "lightweight", "annotated",
 };
 
+static unsigned long tag_date(const unsigned char *sha1)
+{
+	enum object_type type;
+	unsigned long size;
+	char *buf;
+	int offset = 0;
+	unsigned long time = 1;
+
+	buf = read_sha1_file(sha1, &type, &size);
+	if (buf && type == OBJ_TAG) {
+		while (offset < size && buf[offset] != '\n') {
+			int new_offset = offset + 1;
+			while (new_offset < size && buf[new_offset++] != '\n')
+				; /* do nothing */
+			if (!prefixcmp(buf + offset, "tagger ")) {
+				char *line = buf + offset + 7;
+				char *date;
+
+				buf[new_offset] = '\0';
+				date = strchr(line, '>');
+				if (date)
+					time = strtoul(date + 1, NULL, 10);
+				break;
+			}
+			offset = new_offset;
+		}
+	} else
+		error("missing tag %s", sha1_to_hex(sha1));
+	free(buf);
+	return time;
+}
+
+static int replace_name(struct commit_name *e,
+			       int prio,
+			       const unsigned char *sha1,
+			       unsigned long *date)
+{
+	if (!e || e->prio < prio)
+		return 1;
+
+	if (e->prio == 2 && prio == 2) {
+		/* Multiple annotated tags point to the same commit.
+		 * Select one to keep based upon their tagger date.
+		 */
+		if (!e->date)
+			e->date = tag_date(e->sha1);
+		*date = tag_date(sha1);
+		if (e->date < *date)
+			return 1;
+	}
+
+	return 0;
+}
+
 static void add_to_known_names(const char *path,
 			       struct commit *commit,
 			       int prio,
 			       const unsigned char *sha1)
 {
 	struct commit_name *e = commit->util;
-	if (!e || e->prio < prio) {
+	unsigned long date = 0;
+	if (replace_name(e, prio, sha1, &date)) {
 		size_t len = strlen(path)+1;
 		free(e);
 		e = xmalloc(sizeof(struct commit_name) + len);
 		e->tag = NULL;
 		e->prio = prio;
+		e->date = date;
 		hashcpy(e->sha1, sha1);
 		memcpy(e->path, path, len);
 		commit->util = e;
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 065dead..876d1ab 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -8,7 +8,7 @@ test_description='test describe
  o----o----o----o----o----.    /
        \        A    c        /
         .------------o---o---o
-                     D   e
+                   D,R   e
 '
 . ./test-lib.sh
 
@@ -68,6 +68,8 @@ test_expect_success setup '
 	echo D >another && git add another && git commit -m D &&
 	test_tick &&
 	git tag -a -m D D &&
+	test_tick &&
+	git tag -a -m R R &&
 
 	test_tick &&
 	echo DD >another && git commit -a -m another &&
@@ -89,10 +91,10 @@ test_expect_success setup '
 
 check_describe A-* HEAD
 check_describe A-* HEAD^
-check_describe D-* HEAD^^
+check_describe R-* HEAD^^
 check_describe A-* HEAD^^2
 check_describe B HEAD^^2^
-check_describe D-* HEAD^^^
+check_describe R-* HEAD^^^
 
 check_describe c-* --tags HEAD
 check_describe c-* --tags HEAD^
-- 
1.7.1.rc1.232.gc76b8

-- 
Shawn.

Re: [PATCH] describe: Break annotated tag ties by tagger date

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If more than one annotated tag points at the same commit, use the
> tag whose tagger field has a more recent date stamp.  This resolves
> non-deterministic cases where the maintainer has done:

I think it is a good idea to introduce this tiebreaker to give the listing
some degree of stability.  And I also notice that you prepared a patch
that can easily apply to an older codebase like 1.6.6 maintenance track.

I have anything against this as an incremental and low impact improvement,
but at the same time I think we might want to consider adding the tagger
date to "struct tag".

Re: [PATCH] describe: Break annotated tag ties by tagger date

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > If more than one annotated tag points at the same commit, use the
> > tag whose tagger field has a more recent date stamp.  This resolves
> > non-deterministic cases where the maintainer has done:
> 
> I think it is a good idea to introduce this tiebreaker to give the listing
> some degree of stability.  And I also notice that you prepared a patch
> that can easily apply to an older codebase like 1.6.6 maintenance track.
> 
> I have anything against this as an incremental and low impact improvement,
> but at the same time I think we might want to consider adding the tagger
> date to "struct tag".

Yea, I thought about that.  It would have simplified the code
in describe.  But I was also trying to avoid a larger impact.  :-)

I'm more than happy to respin with a new date field in struct tag.
I just won't be able to get to it before Monday.

-- 
Shawn.

[PATCH v2 1/5] tag.c: Correct indentation

[Shawn O. Pearce]

These lines were incorrectly indented with spaces, violating our
coding style.  Its annoying to read with 4 position tab stops, so
fix the indentation to be correct.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Respin of the stable describe stuff, this time pushing the date
 parsing into tag.c and the struct tag member.

 tag.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tag.c b/tag.c
index 4470d2b..52d71bb 100644
--- a/tag.c
+++ b/tag.c
@@ -44,9 +44,9 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 	char type[20];
 	const char *start = data;
 
-        if (item->object.parsed)
-                return 0;
-        item->object.parsed = 1;
+	if (item->object.parsed)
+		return 0;
+	item->object.parsed = 1;
 
 	if (size < 64)
 		return -1;
-- 
1.7.1.rc1.246.g978a8

[PATCH v2 2/5] tag.h: Remove unused signature field

[Shawn O. Pearce]

Its documented as unused.  So lets just drop it from the structure
since we haven't ever used it.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 tag.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/tag.h b/tag.h
index 7a0cb00..c437890 100644
--- a/tag.h
+++ b/tag.h
@@ -9,7 +9,6 @@ struct tag {
 	struct object object;
 	struct object *tagged;
 	char *tag;
-	char *signature; /* not actually implemented */
 };
 
 extern struct tag *lookup_tag(const unsigned char *sha1);
-- 
1.7.1.rc1.246.g978a8

[PATCH v2 3/5] tag.c: Refactor parse_tag_buffer to be saner to program

[Shawn O. Pearce]

This code was horribly ugly to follow.  The structure of the headers
in an annotated tag object must follow a prescribed order, and most
of these are required.  Simplify the entire parsing logic by going
through the headers in the order they are supposed to appear in,
acting on each header as its identified in the buffer.

This change has the same behavior as the older version, its just
easier to read and maintain.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 tag.c |   43 +++++++++++++++++++++----------------------
 1 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/tag.c b/tag.c
index 52d71bb..ceb8655 100644
--- a/tag.c
+++ b/tag.c
@@ -38,11 +38,11 @@ struct tag *lookup_tag(const unsigned char *sha1)
 
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
-	int typelen, taglen;
 	unsigned char sha1[20];
-	const char *type_line, *tag_line, *sig_line;
 	char type[20];
-	const char *start = data;
+	const char *bufptr = data;
+	const char *tail = bufptr + size;
+	const char *nl;
 
 	if (item->object.parsed)
 		return 0;
@@ -50,29 +50,19 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 
 	if (size < 64)
 		return -1;
-	if (memcmp("object ", data, 7) || get_sha1_hex((char *) data + 7, sha1))
+	if (memcmp("object ", bufptr, 7) || get_sha1_hex(bufptr + 7, sha1) || bufptr[47] != '\n')
 		return -1;
+	bufptr += 48; /* "object " + sha1 + "\n" */
 
-	type_line = (char *) data + 48;
-	if (memcmp("\ntype ", type_line-1, 6))
+	if (prefixcmp(bufptr, "type "))
 		return -1;
-
-	tag_line = memchr(type_line, '\n', size - (type_line - start));
-	if (!tag_line || memcmp("tag ", ++tag_line, 4))
-		return -1;
-
-	sig_line = memchr(tag_line, '\n', size - (tag_line - start));
-	if (!sig_line)
-		return -1;
-	sig_line++;
-
-	typelen = tag_line - type_line - strlen("type \n");
-	if (typelen >= 20)
+	bufptr += 5;
+	nl = memchr(bufptr, '\n', tail - bufptr);
+	if (!nl || sizeof(type) <= (nl - bufptr))
 		return -1;
-	memcpy(type, type_line + 5, typelen);
-	type[typelen] = '\0';
-	taglen = sig_line - tag_line - strlen("tag \n");
-	item->tag = xmemdupz(tag_line + 4, taglen);
+	strncpy(type, bufptr, nl - bufptr);
+	type[nl - bufptr] = '\0';
+	bufptr = nl + 1;
 
 	if (!strcmp(type, blob_type)) {
 		item->tagged = &lookup_blob(sha1)->object;
@@ -87,6 +77,15 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 		item->tagged = NULL;
 	}
 
+	if (prefixcmp(bufptr, "tag "))
+		return -1;
+	bufptr += 4;
+	nl = memchr(bufptr, '\n', tail - bufptr);
+	if (!nl)
+		return -1;
+	item->tag = xmemdupz(bufptr, nl - bufptr);
+	bufptr = nl + 1;
+
 	return 0;
 }
 
-- 
1.7.1.rc1.246.g978a8

[PATCH v2 4/5] tag.c: Parse tagger date (if present)

[Shawn O. Pearce]

Just like with committer dates, we parse the tagger date into the
struct tag so its available for further downstream processing.
However since the tagger header was not introduced until Git 0.99.1
we must consider it optional.  For tags missing this header we use
the default date of 0.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 tag.c |   22 ++++++++++++++++++++++
 tag.h |    1 +
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/tag.c b/tag.c
index ceb8655..85607c2 100644
--- a/tag.c
+++ b/tag.c
@@ -36,6 +36,23 @@ struct tag *lookup_tag(const unsigned char *sha1)
         return (struct tag *) obj;
 }
 
+static unsigned long parse_tag_date(const char *buf, const char *tail)
+{
+	const char *dateptr;
+
+	while (buf < tail && *buf++ != '>')
+		/* nada */;
+	if (buf >= tail)
+		return 0;
+	dateptr = buf;
+	while (buf < tail && *buf++ != '\n')
+		/* nada */;
+	if (buf >= tail)
+		return 0;
+	/* dateptr < buf && buf[-1] == '\n', so strtoul will stop at buf-1 */
+	return strtoul(dateptr, NULL, 10);
+}
+
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
 	unsigned char sha1[20];
@@ -86,6 +103,11 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 	item->tag = xmemdupz(bufptr, nl - bufptr);
 	bufptr = nl + 1;
 
+	if (!prefixcmp(bufptr, "tagger "))
+		item->date = parse_tag_date(bufptr, tail);
+	else
+		item->date = 0;
+
 	return 0;
 }
 
diff --git a/tag.h b/tag.h
index c437890..4766272 100644
--- a/tag.h
+++ b/tag.h
@@ -9,6 +9,7 @@ struct tag {
 	struct object object;
 	struct object *tagged;
 	char *tag;
+	unsigned long date;
 };
 
 extern struct tag *lookup_tag(const unsigned char *sha1);
-- 
1.7.1.rc1.246.g978a8

[PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Shawn O. Pearce]

If more than one annotated tag points at the same commit, use the
tag whose tagger field has a more recent date stamp.  This resolves
non-deterministic cases where the maintainer has done:

  $ git tag -a -m "2.1-rc1" v2.1-rc1  deadbeef
  $ git tag -a -m "2.1"     v2.1      deadbeef

If the tag is an older-style annotated tag with no tagger date,
we assume a date stamp of 1 second after the UNIX epoch. This will
cause us to prefer an annotated tag that has a valid date, or to
simply avoid scanning the tag object again if a 3rd tag was found
for the same commit.

We could also try to consider the tag object chain, favoring a tag
that "includes" another one:

  $ git tag -a -m "2.1-rc0" v2.1-rc1  deadbeef
  $ git tag -a -m "2.1"     v2.1      v2.1-rc1

However traversing the tag's object chain looking for inclusion
is much more complicated.  Its already very likely that even in
these cases the v2.1 tag will have a more recent tagger date than
v2.1-rc1, so with this change describe should still resolve this
by selecting the more recent v2.1.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin/describe.c  |   49 +++++++++++++++++++++++++++++++++++++++++++++----
 t/t6120-describe.sh |    8 +++++---
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 71be2a9..43caff2 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -35,7 +35,8 @@ static const char *diff_index_args[] = {
 
 struct commit_name {
 	struct tag *tag;
-	int prio; /* annotated tag = 2, tag = 1, head = 0 */
+	unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
+	unsigned name_checked:1;
 	unsigned char sha1[20];
 	char path[FLEX_ARRAY]; /* more */
 };
@@ -43,18 +44,53 @@ static const char *prio_names[] = {
 	"head", "lightweight", "annotated",
 };
 
+static int replace_name(struct commit_name *e,
+			       int prio,
+			       const unsigned char *sha1,
+			       struct tag **tag)
+{
+	if (!e || e->prio < prio)
+		return 1;
+
+	if (e->prio == 2 && prio == 2) {
+		/* Multiple annotated tags point to the same commit.
+		 * Select one to keep based upon their tagger date.
+		 */
+		struct tag *t;
+
+		if (!e->tag) {
+			t = lookup_tag(e->sha1);
+			if (!t || parse_tag(t))
+				return 1;
+			e->tag = t;
+		}
+
+		t = lookup_tag(sha1);
+		if (!t || parse_tag(t))
+			return 0;
+		*tag = t;
+
+		if (e->tag->date < t->date)
+			return 1;
+	}
+
+	return 0;
+}
+
 static void add_to_known_names(const char *path,
 			       struct commit *commit,
 			       int prio,
 			       const unsigned char *sha1)
 {
 	struct commit_name *e = commit->util;
-	if (!e || e->prio < prio) {
+	struct tag *tag = NULL;
+	if (replace_name(e, prio, sha1, &tag)) {
 		size_t len = strlen(path)+1;
 		free(e);
 		e = xmalloc(sizeof(struct commit_name) + len);
-		e->tag = NULL;
+		e->tag = tag;
 		e->prio = prio;
+		e->name_checked = 0;
 		hashcpy(e->sha1, sha1);
 		memcpy(e->path, path, len);
 		commit->util = e;
@@ -165,10 +201,15 @@ static void display_name(struct commit_name *n)
 {
 	if (n->prio == 2 && !n->tag) {
 		n->tag = lookup_tag(n->sha1);
-		if (!n->tag || parse_tag(n->tag) || !n->tag->tag)
+		if (!n->tag || parse_tag(n->tag))
 			die("annotated tag %s not available", n->path);
+	}
+	if (n->tag && !n->name_checked) {
+		if (!n->tag->tag)
+			die("annotated tag %s has no embedded name", n->path);
 		if (strcmp(n->tag->tag, all ? n->path + 5 : n->path))
 			warning("tag '%s' is really '%s' here", n->tag->tag, n->path);
+		n->name_checked = 1;
 	}
 
 	if (n->tag)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 065dead..876d1ab 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -8,7 +8,7 @@ test_description='test describe
  o----o----o----o----o----.    /
        \        A    c        /
         .------------o---o---o
-                     D   e
+                   D,R   e
 '
 . ./test-lib.sh
 
@@ -68,6 +68,8 @@ test_expect_success setup '
 	echo D >another && git add another && git commit -m D &&
 	test_tick &&
 	git tag -a -m D D &&
+	test_tick &&
+	git tag -a -m R R &&
 
 	test_tick &&
 	echo DD >another && git commit -a -m another &&
@@ -89,10 +91,10 @@ test_expect_success setup '
 
 check_describe A-* HEAD
 check_describe A-* HEAD^
-check_describe D-* HEAD^^
+check_describe R-* HEAD^^
 check_describe A-* HEAD^^2
 check_describe B HEAD^^2^
-check_describe D-* HEAD^^^
+check_describe R-* HEAD^^^
 
 check_describe c-* --tags HEAD
 check_describe c-* --tags HEAD^
-- 
1.7.1.rc1.246.g978a8

Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Andreas Ericsson]

On 04/13/2010 01:25 AM, Shawn O. Pearce wrote:
> If more than one annotated tag points at the same commit, use the
> tag whose tagger field has a more recent date stamp.  This resolves
> non-deterministic cases where the maintainer has done:
> 
>    $ git tag -a -m "2.1-rc1" v2.1-rc1  deadbeef
>    $ git tag -a -m "2.1"     v2.1      deadbeef
> 

We use this patch-series in our buildsystem, where we just now
tagged about 14 projects as stable (on top of an earlier beta tag).
Everything's working just fine, so...

Tested-By: Andreas Ericsson <ae@op5.se>

Thanks a lot, Shawn.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Thomas Rast]

Shawn O. Pearce wrote:
> If the tag is an older-style annotated tag with no tagger date,
> we assume a date stamp of 1 second after the UNIX epoch.

This patch doesn't seem to actually set this, but as a minor nit: the
'1 second' contradicts the 0 mentioned in the last patch.

(The effect is the same for all practical purposes.)

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

Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Shawn O. Pearce]

Thomas Rast <trast@student.ethz.ch> wrote:
> Shawn O. Pearce wrote:
> > If the tag is an older-style annotated tag with no tagger date,
> > we assume a date stamp of 1 second after the UNIX epoch.
> 
> This patch doesn't seem to actually set this, but as a minor nit: the
> '1 second' contradicts the 0 mentioned in the last patch.
> 
> (The effect is the same for all practical purposes.)

Whoops.  Old commit message from v1.  I skim read it knowing I
needed to adjust something in the message before reusing it, but
didn't see anything, so sent it as-is.

Junio, it might make sense to drop the part Thomas quoted above
before you apply this.

-- 
Shawn.

Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Thomas Rast <trast@student.ethz.ch> wrote:
>> Shawn O. Pearce wrote:
>> > If the tag is an older-style annotated tag with no tagger date,
>> > we assume a date stamp of 1 second after the UNIX epoch.
>> 
>> This patch doesn't seem to actually set this, but as a minor nit: the
>> '1 second' contradicts the 0 mentioned in the last patch.
>> 
>> (The effect is the same for all practical purposes.)
>
> Whoops.  Old commit message from v1.  I skim read it knowing I
> needed to adjust something in the message before reusing it, but
> didn't see anything, so sent it as-is.
>
> Junio, it might make sense to drop the part Thomas quoted above
> before you apply this.

Surely I can, but I am curious as to the motivation behind '1 second' in
the original patch.  If you wanted to give these '1 second' ones slight
preference over the ones with 'date stamp at the UNIX epoch', that logic
is not there anymore in the re-rolled series, no?

Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Thomas Rast <trast@student.ethz.ch> wrote:
> >> Shawn O. Pearce wrote:
> >> > If the tag is an older-style annotated tag with no tagger date,
> >> > we assume a date stamp of 1 second after the UNIX epoch.
> >> 
> >> This patch doesn't seem to actually set this, but as a minor nit: the
> >> '1 second' contradicts the 0 mentioned in the last patch.
> 
> Surely I can, but I am curious as to the motivation behind '1 second' in
> the original patch.  If you wanted to give these '1 second' ones slight
> preference over the ones with 'date stamp at the UNIX epoch', that logic
> is not there anymore in the re-rolled series, no?

The 1-second in the original patch had nothing to do about giving
one tag an edge over another tag.

The meaning of date in the original patch was:

  date = 0: we haven't yet looked up the date
  date = 1: we looked it up, but there is no tagger present
  date > 0: "valid" date, we can sort with it

So date = 1 was just about getting ourselves out of the !date case so
that we didn't keep parsing a tag which has no tagger field present.

In the new version this is handled by the standard parsed field
of struct object.  So I didn't need to overload the meaning of the
date field anymore.

Make sense?

-- 
Shawn.