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

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

[Junio C Hamano]

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

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

[Junio C Hamano]

"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(-)

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

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

[Junio C Hamano]

"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(-)

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