threads / patch / 34481

patch, 2 partsdescribe: fix --contains when a tag is given as input

Subject: [PATCH 2/2] describe: fix --contains when a tag is given as input

## tl;dr

3 messages between Jul 18, 2013 and Jul 18, 2013. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Junio C Hamano· Jul 18, 2013, 22:16 UTC · lore

[PATCH 0/2] Finishing touches to "name-rev" fix

This is an update to finish the jc/name-rev-exact-ref topic, which fixed the command to convert an object name that points at a tag to a refname of the tag (earlier, it did not show anything). The codepath to handle its command line arguments, however, fed the commit that the tag points at to the underlying naming machinery.

The first patch in this follow-up series corrects it for the command line codepath.

The second patch is a related fix for "git describe". The command is about naming the given commit in relation to a tag in its neighbourhood, and while it does allow the input to be a commit-ish (e.g. a tag that points at a commit), it did not unwrap it down to commit, which is a bug (it is like "git commit-tree -p $tag" that would mistakenly record a tag object as one of the parents of the resulting commit).

Junio C Hamano (2):
  name-rev: differentiate between tags and commits they point at
  describe: fix --contains when a tag is given as input
 builtin/describe.c  |  3 ++-
 builtin/name-rev.c  | 41 ++++++++++++++++++++++++++++++++---------
 t/t6120-describe.sh | 24 ++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 10 deletions(-)
-- 
1.8.3.3-992-gf0e5e44
Junio C Hamano· Jul 18, 2013, 22:16 UTC · re: Junio C Hamano · lore

[PATCH 1/2] name-rev: differentiate between tags and commits they point at

"git name-rev --stdin" has been fixed to convert an object name that points at a tag to a refname of the tag. The codepath to handle its command line arguments, however, fed the commit that the tag points at to the underlying naming machinery.

With this fix, you will get this:
    $ git name-rev --refs=tags/\* --name-only $(git rev-parse v1.8.3 v1.8.3^0)
    v1.8.3
    v1.8.3^0
which is the same as what you would get from the fixed "--stdin" variant:
    $ git rev-parse v1.8.3 v1.8.3^0 | git name-rev --refs=tags/\* --name-only
    v1.8.3
    v1.8.3^0
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/name-rev.c  | 24 ++++++++++++++++--------
 t/t6120-describe.sh | 12 ++++++++++++
 2 files changed, 28 insertions(+), 8 deletions(-)
Show changes to 2 files +28 −8

builtin/name-rev.c, t/t6120-describe.sh

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 29a6f56..4c7cc62 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -334,7 +334,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
 	for (; argc; argc--, argv++) {
 		unsigned char sha1[20];
-		struct object *o;
+		struct object *object;
 		struct commit *commit;
 
 		if (get_sha1(*argv, sha1)) {
@@ -343,17 +343,25 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 
-		o = deref_tag(parse_object(sha1), *argv, 0);
-		if (!o || o->type != OBJ_COMMIT) {
-			fprintf(stderr, "Could not get commit for %s. Skipping.\n",
+		commit = NULL;
+		object = parse_object(sha1);
+		if (object) {
+			struct object *peeled = deref_tag(object, *argv, 0);
+			if (peeled && peeled->type == OBJ_COMMIT)
+				commit = (struct commit *)peeled;
+		}
+
+		if (!object) {
+			fprintf(stderr, "Could not get object for %s. Skipping.\n",
 					*argv);
 			continue;
 		}
 
-		commit = (struct commit *)o;
-		if (cutoff > commit->date)
-			cutoff = commit->date;
-		add_object_array((struct object *)commit, *argv, &revs);
+		if (commit) {
+			if (cutoff > commit->date)
+				cutoff = commit->date;
+		}
+		add_object_array(object, *argv, &revs);
 	}
 
 	if (cutoff)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index a25729f..1d20854 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -174,4 +174,16 @@ check_describe "test2-lightweight-*" --tags --match="test2-*"
 
 check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
 
+test_expect_success 'name-rev with exact tags' '
+	echo A >expect &&
+	tag_object=$(git rev-parse refs/tags/A) &&
+	git name-rev --tags --name-only $tag_object >actual &&
+	test_cmp expect actual &&
+
+	echo "A^0" >expect &&
+	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+	git name-rev --tags --name-only $tagged_commit >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.3.3-992-gf0e5e44
Junio C Hamano· Jul 18, 2013, 22:16 UTC · re: Junio C Hamano · lore

"git describe" takes a commit and gives it a name based on tags in its neighbourhood. The command does take a commit-ish but when given a tag that points at a commit, it should dereference the tag before computing the name for the commit.

As the whole processing is internally delegated to name-rev, if we unwrap tags down to the underlying commit when invoking name-rev, it will make the name-rev issue an error message based on the unwrapped object name (i.e. either 40-hex object name, or "$tag^0") that is different from what the end-user gave to the command when the commit cannot be described. Introduce an internal option --peel-tag to the name-rev to tell it to unwrap a tag in its input from the command line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/describe.c  |  3 ++-
 builtin/name-rev.c  | 17 ++++++++++++++++-
 t/t6120-describe.sh | 12 ++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)
Show changes to 3 files +30 −2

builtin/describe.c, builtin/name-rev.c, t/t6120-describe.sh

diff --git a/builtin/describe.c b/builtin/describe.c
index db41cd7..7d73722 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -446,7 +446,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		struct argv_array args;
 
 		argv_array_init(&args);
-		argv_array_pushl(&args, "name-rev", "--name-only", "--no-undefined",
+		argv_array_pushl(&args, "name-rev",
+				 "--peel-tag", "--name-only", "--no-undefined",
 				 NULL);
 		if (always)
 			argv_array_push(&args, "--always");
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 4c7cc62..0aaa19e 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -307,7 +307,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)
 int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
-	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0;
+	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
 	struct name_ref_data data = { 0, 0, NULL };
 	struct option opts[] = {
 		OPT_BOOLEAN(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
@@ -320,6 +320,12 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "undefined", &allow_undefined, N_("allow to print `undefined` names")),
 		OPT_BOOLEAN(0, "always",     &always,
 			   N_("show abbreviated commit object as fallback")),
+		{
+			/* A Hidden OPT_BOOL */
+			OPTION_SET_INT, 0, "peel-tag", &peel_tag, NULL,
+			N_("dereference tags in the input (internal use)"),
+			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1,
+		},
 		OPT_END(),
 	};
 
@@ -361,6 +367,15 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 			if (cutoff > commit->date)
 				cutoff = commit->date;
 		}
+
+		if (peel_tag) {
+			if (!commit) {
+				fprintf(stderr, "Could not get commit for %s. Skipping.\n",
+					*argv);
+				continue;
+			}
+			object = (struct object *)commit;
+		}
 		add_object_array(object, *argv, &revs);
 	}
 
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 1d20854..c0e5b2a 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -186,4 +186,16 @@ test_expect_success 'name-rev with exact tags' '
 	test_cmp expect actual
 '
 
+test_expect_success 'describe --contains with the exact tags' '
+	echo "A^0" >expect &&
+	tag_object=$(git rev-parse refs/tags/A) &&
+	git describe --contains $tag_object >actual &&
+	test_cmp expect actual &&
+
+	echo "A^0" >expect &&
+	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+	git describe --contains $tagged_commit >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.3.3-992-gf0e5e44

← back to recent threads