* [PATCH v2 1/5] tag.c: Correct indentation
2010-04-11 2:40 ` Shawn O. Pearce
@ 2010-04-12 23:25 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 2/5] tag.h: Remove unused signature field Shawn O. Pearce
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-12 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 2/5] tag.h: Remove unused signature field
2010-04-11 2:40 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 1/5] tag.c: Correct indentation Shawn O. Pearce
@ 2010-04-12 23:25 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 3/5] tag.c: Refactor parse_tag_buffer to be saner to program Shawn O. Pearce
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-12 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 3/5] tag.c: Refactor parse_tag_buffer to be saner to program
2010-04-11 2:40 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 1/5] tag.c: Correct indentation Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 2/5] tag.h: Remove unused signature field Shawn O. Pearce
@ 2010-04-12 23:25 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 4/5] tag.c: Parse tagger date (if present) Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 5/5] describe: Break annotated tag ties by tagger date Shawn O. Pearce
4 siblings, 0 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-12 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4/5] tag.c: Parse tagger date (if present)
2010-04-11 2:40 ` Shawn O. Pearce
` (2 preceding siblings ...)
2010-04-12 23:25 ` [PATCH v2 3/5] tag.c: Refactor parse_tag_buffer to be saner to program Shawn O. Pearce
@ 2010-04-12 23:25 ` Shawn O. Pearce
2010-04-12 23:25 ` [PATCH v2 5/5] describe: Break annotated tag ties by tagger date Shawn O. Pearce
4 siblings, 0 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-12 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-11 2:40 ` Shawn O. Pearce
` (3 preceding siblings ...)
2010-04-12 23:25 ` [PATCH v2 4/5] tag.c: Parse tagger date (if present) Shawn O. Pearce
@ 2010-04-12 23:25 ` Shawn O. Pearce
2010-04-13 9:27 ` Andreas Ericsson
2010-04-13 9:32 ` Thomas Rast
4 siblings, 2 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-12 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-12 23:25 ` [PATCH v2 5/5] describe: Break annotated tag ties by tagger date Shawn O. Pearce
@ 2010-04-13 9:27 ` Andreas Ericsson
2010-04-13 9:32 ` Thomas Rast
1 sibling, 0 replies; 17+ messages in thread
From: Andreas Ericsson @ 2010-04-13 9:27 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-12 23:25 ` [PATCH v2 5/5] describe: Break annotated tag ties by tagger date Shawn O. Pearce
2010-04-13 9:27 ` Andreas Ericsson
@ 2010-04-13 9:32 ` Thomas Rast
2010-04-13 14:08 ` Shawn O. Pearce
1 sibling, 1 reply; 17+ messages in thread
From: Thomas Rast @ 2010-04-13 9:32 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-13 9:32 ` Thomas Rast
@ 2010-04-13 14:08 ` Shawn O. Pearce
2010-04-13 19:26 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-13 14:08 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, git
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-13 14:08 ` Shawn O. Pearce
@ 2010-04-13 19:26 ` Junio C Hamano
2010-04-13 19:46 ` Shawn O. Pearce
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2010-04-13 19:26 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Thomas Rast, git
"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?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] describe: Break annotated tag ties by tagger date
2010-04-13 19:26 ` Junio C Hamano
@ 2010-04-13 19:46 ` Shawn O. Pearce
0 siblings, 0 replies; 17+ messages in thread
From: Shawn O. Pearce @ 2010-04-13 19:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git
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.
^ permalink raw reply [flat|nested] 17+ messages in thread