[PATCH] git-describe: Add a --match option to limit considered tags.

[PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/git-describe.txt |    4 ++++
 builtin-describe.c             |   11 ++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index ac23e28..cb869e9 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -51,6 +51,10 @@ OPTIONS
 	being employed to standard error.  The tag name will still
 	be printed to standard out.
 
+--match <pattern>::
+        Only consider tags matching the given pattern (can be used to avoid
+        leaking private tags made from the repository).
+
 EXAMPLES
 --------
 
diff --git a/builtin-describe.c b/builtin-describe.c
index 7a148a2..982a355 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -19,6 +19,7 @@ static int all;	/* Default to annotated tags only */
 static int tags;	/* But allow any tags if --tags is specified */
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
+const char *pattern = NULL;
 
 struct commit_name {
 	int prio; /* annotated tag = 2, tag = 1, head = 0 */
@@ -57,9 +58,11 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	 * Otherwise only annotated tags are used.
 	 */
 	if (!prefixcmp(path, "refs/tags/")) {
-		if (object->type == OBJ_TAG)
+		if (object->type == OBJ_TAG) {
 			prio = 2;
-		else
+			if (pattern && fnmatch(pattern, path + 10, 0))
+				prio = 0;
+		} else
 			prio = 1;
 	}
 	else
@@ -253,7 +256,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "tags",       &tags, "use any tag in .git/refs/tags"),
 		OPT__ABBREV(&abbrev),
 		OPT_INTEGER(0, "candidates", &max_candidates,
-					"consider <n> most recent tags (default: 10)"),
+		            "consider <n> most recent tags (default: 10)"),
+		OPT_STRING(0, "match",       &pattern, "pattern",
+		           "only consider tags matching <pattern>"),
 		OPT_END(),
 	};
 
-- 
1.5.4.rc1.1096.gde4c4-dirty

Re: [PATCH] git-describe: Add a --match option to limit considered tags.

[Junio C Hamano]

Does it work with "describe --contains" as well?

Re: [PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

On Fri, Dec 21, 2007 at 05:52:29PM +0000, Junio C Hamano wrote:
> Does it work with "describe --contains" as well?

  I think so, the idea here is that I give prio "0" to tags that don't
match, which unless you pass --all (which basically conflicts with
--match <foo> anyways) means that those are not added to the list of
tags that are considered. So I don't see why it would fail with
--contains.

  That's a patch we "need" at work because we have a repository with
different products that share a _lot_ of code (and we're not confident
with submodules yet to switch) and we use git-describe to embed the
exact version of the code that was shipped to a client. Though it sucks
when the last tag shows another product name :)

  I'd also like to use it in some scripts of mine for debian packaging
where I have quite a lot of private tags and only want to describe tags
that match 'debian-sid*'

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[FIXED PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

    On Fri, Dec 21, 2007 at 09:22:12PM +0000, Pierre Habouzit wrote:
    > On Fri, Dec 21, 2007 at 05:52:29PM +0000, Junio C Hamano wrote:
    > > Does it work with "describe --contains" as well?

    Okay, you're right … Here is an updated patch.

 Documentation/git-describe.txt |    4 ++++
 builtin-describe.c             |   21 ++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index ac23e28..cb869e9 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -51,6 +51,10 @@ OPTIONS
 	being employed to standard error.  The tag name will still
 	be printed to standard out.
 
+--match <pattern>::
+        Only consider tags matching the given pattern (can be used to avoid
+        leaking private tags made from the repository).
+
 EXAMPLES
 --------
 
diff --git a/builtin-describe.c b/builtin-describe.c
index 7a148a2..dd44df9 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -19,6 +19,7 @@ static int all;	/* Default to annotated tags only */
 static int tags;	/* But allow any tags if --tags is specified */
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
+const char *pattern = NULL;
 
 struct commit_name {
 	int prio; /* annotated tag = 2, tag = 1, head = 0 */
@@ -57,9 +58,11 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	 * Otherwise only annotated tags are used.
 	 */
 	if (!prefixcmp(path, "refs/tags/")) {
-		if (object->type == OBJ_TAG)
+		if (object->type == OBJ_TAG) {
 			prio = 2;
-		else
+			if (pattern && fnmatch(pattern, path + 10, 0))
+				prio = 0;
+		} else
 			prio = 1;
 	}
 	else
@@ -253,7 +256,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "tags",       &tags, "use any tag in .git/refs/tags"),
 		OPT__ABBREV(&abbrev),
 		OPT_INTEGER(0, "candidates", &max_candidates,
-					"consider <n> most recent tags (default: 10)"),
+		            "consider <n> most recent tags (default: 10)"),
+		OPT_STRING(0, "match",       &pattern, "pattern",
+		           "only consider tags matching <pattern>"),
 		OPT_END(),
 	};
 
@@ -266,12 +271,18 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	save_commit_buffer = 0;
 
 	if (contains) {
-		const char **args = xmalloc((4 + argc) * sizeof(char*));
+		const char **args = xmalloc((5 + argc) * sizeof(char*));
 		int i = 0;
 		args[i++] = "name-rev";
 		args[i++] = "--name-only";
-		if (!all)
+		if (!all) {
 			args[i++] = "--tags";
+			if (pattern) {
+				char *s = xmalloc(strlen("--refs=refs/tags/") + strlen(pattern) + 1);
+				sprintf(s, "--refs=refs/tags/%s", pattern);
+				args[i++] = s;
+			}
+		}
 		memcpy(args + i, argv, argc * sizeof(char*));
 		args[i + argc] = NULL;
 		return cmd_name_rev(i + argc, args, prefix);
-- 
1.5.4.rc1.1097.gd122b-dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [FIXED PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

  Like I said on IRC, I saw that git describe --contains has a bad
behaviour:

    $ git describe --match='asd*' HEAD; echo $?
    fatal: cannot describe 'e272415ab7da3bde51af2ce95c88d7be3abfba28'
    128
    $ git describe --contains HEAD; echo $?
    undefined
    0

THe "undefined" output is on stdout (not stderr), and returns 0.
The issue here is that it internally uses git-name-rev by exec-ing it, which
makes it hard to fix. Though I suppose that we could instead of fork-ing
share some logic with builtin-name-rev.c, but I'm not at home yet, so
won't likely have a patch for this issue.

Note that the use of the "new" --match here was just to be unable to
describe the HEAD to show the difference, the inconsistency has nothing
to do with the patch I propose, I just happen to noticed that.

AFAICT it's not a regression, it's just a misfeature :)
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [FIXED PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 5198 bytes --]

Rework get_rev_name to return NULL rather than "undefined" when a reference
is undefined. If --undefined is passed (default) git-name-rev prints
"undefined" for the name, else it die()s.

Make git-describe use --no-undefined when calling git-name-rev so that
--contains behavior matches the standard git-describe one.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

    On sam, déc 22, 2007 at 06:02:44 +0000, Pierre Habouzit wrote:
    >   Like I said on IRC, I saw that git describe --contains has a bad
    > behaviour:
    > 
    >     $ git describe --match='asd*' HEAD; echo $?
    >     fatal: cannot describe 'e272415ab7da3bde51af2ce95c88d7be3abfba28'
    >     128
    >     $ git describe --contains HEAD; echo $?
    >     undefined
    >     0

    Okay here is a patch to have git-name-rev have a strict mode where
    it rejects undefined's.  With this patch:

    $ git describe --contains HEAD
    fatal: cannot describe 'e26a806b93f2f2f2354831ce0f943347a8ba7c3e'

    This patch is minimal and looks safe for master to me. A better patch
    would probably to have shared some logic between name-rev and
    describe, but this would have been a quite larger patch, and I was
    uncomfortable with it.


 builtin-describe.c |    3 ++-
 builtin-name-rev.c |   34 ++++++++++++++++++++++++----------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/builtin-describe.c b/builtin-describe.c
index 18eab47..3428483 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -271,10 +271,11 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	save_commit_buffer = 0;
 
 	if (contains) {
-		const char **args = xmalloc((5 + argc) * sizeof(char*));
+		const char **args = xmalloc((6 + argc) * sizeof(char*));
 		int i = 0;
 		args[i++] = "name-rev";
 		args[i++] = "--name-only";
+		args[i++] = "--no-undefined";
 		if (!all) {
 			args[i++] = "--tags";
 			if (pattern) {
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index a0c89a8..f22c8b5 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -125,18 +125,18 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 }
 
 /* returns a static buffer */
-static const char* get_rev_name(struct object *o)
+static const char *get_rev_name(struct object *o)
 {
 	static char buffer[1024];
 	struct rev_name *n;
 	struct commit *c;
 
 	if (o->type != OBJ_COMMIT)
-		return "undefined";
+		return NULL;
 	c = (struct commit *) o;
 	n = c->util;
 	if (!n)
-		return "undefined";
+		return NULL;
 
 	if (!n->generation)
 		return n->tip_name;
@@ -159,7 +159,7 @@ static char const * const name_rev_usage[] = {
 int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = { 0, 0, NULL };
-	int all = 0, transform_stdin = 0;
+	int all = 0, transform_stdin = 0, allow_undefined = 1;
 	struct name_ref_data data = { 0, 0, NULL };
 	struct option opts[] = {
 		OPT_BOOLEAN(0, "name-only", &data.name_only, "print only names (no SHA-1)"),
@@ -169,6 +169,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_BOOLEAN(0, "all", &all, "list all commits reachable from all refs"),
 		OPT_BOOLEAN(0, "stdin", &transform_stdin, "read from stdin"),
+		OPT_BOOLEAN(0, "undefined", &allow_undefined, "allow to print `undefined` names"),
 		OPT_END(),
 	};
 
@@ -226,7 +227,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 				else if (++forty == 40 &&
 						!ishex(*(p+1))) {
 					unsigned char sha1[40];
-					const char *name = "undefined";
+					const char *name = NULL;
 					char c = *(p+1);
 
 					forty = 0;
@@ -240,11 +241,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 					}
 					*(p+1) = c;
 
-					if (!strcmp(name, "undefined"))
+					if (!name)
 						continue;
 
-					fwrite(p_start, p - p_start + 1, 1,
-					       stdout);
+					fwrite(p_start, p - p_start + 1, 1, stdout);
 					printf(" (%s)", name);
 					p_start = p + 1;
 				}
@@ -260,18 +260,32 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		max = get_max_object_index();
 		for (i = 0; i < max; i++) {
 			struct object * obj = get_indexed_object(i);
+			const char *name;
 			if (!obj)
 				continue;
 			if (!data.name_only)
 				printf("%s ", sha1_to_hex(obj->sha1));
-			printf("%s\n", get_rev_name(obj));
+			name = get_rev_name(obj);
+			if (name)
+				printf("%s\n", name);
+			else if (allow_undefined)
+				printf("undefined\n");
+			else
+				die("cannot describe '%s'", sha1_to_hex(obj->sha1));
 		}
 	} else {
 		int i;
 		for (i = 0; i < revs.nr; i++) {
+			const char *name;
 			if (!data.name_only)
 				printf("%s ", revs.objects[i].name);
-			printf("%s\n", get_rev_name(revs.objects[i].item));
+			name = get_rev_name(revs.objects[i].item);
+			if (name)
+				printf("%s\n", name);
+			else if (allow_undefined)
+				printf("undefined\n");
+			else
+				die("cannot describe '%s'", sha1_to_hex(revs.objects[i].item->sha1));
 		}
 	}
 
-- 
1.5.4.rc1.1123.ge26a8

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [FIXED PATCH] git-describe: Add a --match option to limit considered tags.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

  Woops I borked the mail, the subject was supposed to be:

Subject: [PATCH] Add a --(no-)undefined option to git-name-rev.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]