All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 1/3] name-rev: fix assumption about --name-only usage
Date: Sun,  7 Jul 2013 18:13:14 +0530	[thread overview]
Message-ID: <1373200996-9753-2-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1373200996-9753-1-git-send-email-artagnon@gmail.com>

236157 (Teach git-describe how to run name-rev, 2007-05-21) introduced
`git name-rev --name-only`, with the intent of using it to implement
`git describe --contains`.  According to the message, users wanted to
use describe to figure out which tags contains a specific commit.
name-rev already did this, but didn't print out in the same format as
describe:

  $ git describe v1.8.3~1
  v1.8.3-rc3-8-g5e49f30

  $ git name-rev v1.8.3~1
  v1.8.3~1 tags/v1.8.3~1

There are two problems with using the output of name-rev in describe:
first, it prints out the given argument before describing it.  Second,
it prefixes "tags/" to the tag description.  To eliminate these two
problems, 236157 proposed that the --name-rev option would strip these
things when used with --tags, to match the describe output more closely:

  $ git name-rev --name-only --tags v1.8.3~1
  v1.8.3~1

236157 did not anticipate a problem with always combining --name-rev
with --tags, because it was primarily intended to be used from describe,
where it hard-coded these two arguments in the execv() of name-rev.

Later, 3f7701 (make 'git describe --all --contains' work, 2007-12-19)
noticed that describe didn't work with --contains and --all.  This is
because --contains implied a call to --name-rev (in with --tags was
hard-coded), but --all implied that any ref should be used to describe
the given argument (not just tags).  3f7701 took the band-aid approach,
and made --all disable --tags when calling name-rev.  As a result, while

  $ git describe --contains v1.8.3~1
  v1.8.3~1

would get name-rev to print output in the same format as describe,

  $ git describe --contains --all v1.8.3~1
  tags/v1.8.3~1

would not strip the leading "tags/".

The bug exists in git to this day.  Fix it by removing the assumption
that name-rev --name-only is only intended to be used with --tags.  Also
update some tests.

Users and scripts have learnt to live with 3f7701, and it will continue
to be a small quirk.  Even after this patch, notice

  $ git checkout -b foom v1.8.3
  $ git describe --contains @~1
  v1.8.3~1
  $ git describe --contains --all @~1
  foom~1

In other words, --contains implies --tags in name-rev, which gives
precedence to tags; --all cancels that effect thereby giving precedence
to branches in the case of a tie.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-name-rev.txt       |  6 +++---
 builtin/name-rev.c                   |  3 +++
 t/t4202-log.sh                       |  8 ++++----
 t/t6007-rev-list-cherry-pick-file.sh | 32 ++++++++++++++++----------------
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index 6b0f1ba..7cde4b3 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -37,9 +37,9 @@ OPTIONS
 
 --name-only::
 	Instead of printing both the SHA-1 and the name, print only
-	the name.  If given with --tags the usual tag prefix of
-	"tags/" is also omitted from the name, matching the output
-	of `git-describe` more closely.
+	the name.  The usual tag prefix of "tags/" is also omitted
+	from the name, matching the output of `git-describe` more
+	closely.
 
 --no-undefined::
 	Die with error code != 0 when a reference is undefined,
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 87d4854..37207a9 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -138,6 +138,9 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 			path = shorten_unambiguous_ref(path, 0);
 		else if (!prefixcmp(path, "refs/heads/"))
 			path = path + 11;
+		else if (data->name_only
+		    && !prefixcmp(path, "refs/tags/"))
+			path = path + 10;
 		else if (!prefixcmp(path, "refs/"))
 			path = path + 5;
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index cb03d28..9bec360 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -302,7 +302,7 @@ cat > expect <<\EOF
 | |
 | |     side-2
 | |
-| * commit tags/side-1
+| * commit side-1
 | | Author: A U Thor <author@example.com>
 | |
 | |     side-1
@@ -327,17 +327,17 @@ cat > expect <<\EOF
 |
 |       fourth
 |
-* commit tags/side-1~1
+* commit side-1~1
 | Author: A U Thor <author@example.com>
 |
 |     third
 |
-* commit tags/side-1~2
+* commit side-1~2
 | Author: A U Thor <author@example.com>
 |
 |     second
 |
-* commit tags/side-1~3
+* commit side-1~3
   Author: A U Thor <author@example.com>
 
       initial
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index 28d4f6b..5a8175e 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -49,8 +49,8 @@ test_expect_success setup '
 '
 
 cat >expect <<EOF
-<tags/B
->tags/C
+<B
+>C
 EOF
 
 test_expect_success '--left-right' '
@@ -70,7 +70,7 @@ test_expect_success '--cherry-pick foo comes up empty' '
 '
 
 cat >expect <<EOF
->tags/C
+>C
 EOF
 
 test_expect_success '--cherry-pick bar does not come up empty' '
@@ -88,8 +88,8 @@ test_expect_success 'bar does not come up empty' '
 '
 
 cat >expect <<EOF
-<tags/F
->tags/E
+<F
+>E
 EOF
 
 test_expect_success '--cherry-pick bar does not come up empty (II)' '
@@ -100,10 +100,10 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
 '
 
 cat >expect <<EOF
-+tags/F
-=tags/D
-+tags/E
-=tags/C
++F
+=D
++E
+=C
 EOF
 
 test_expect_success '--cherry-mark' '
@@ -114,10 +114,10 @@ test_expect_success '--cherry-mark' '
 '
 
 cat >expect <<EOF
-<tags/F
-=tags/D
->tags/E
-=tags/C
+<F
+=D
+>E
+=C
 EOF
 
 test_expect_success '--cherry-mark --left-right' '
@@ -128,7 +128,7 @@ test_expect_success '--cherry-mark --left-right' '
 '
 
 cat >expect <<EOF
-tags/E
+E
 EOF
 
 test_expect_success '--cherry-pick --right-only' '
@@ -146,8 +146,8 @@ test_expect_success '--cherry-pick --left-only' '
 '
 
 cat >expect <<EOF
-+tags/E
-=tags/C
++E
+=C
 EOF
 
 test_expect_success '--cherry' '
-- 
1.8.3.2.737.gcbc076a.dirty

  reply	other threads:[~2013-07-07 12:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-07 12:43 [PATCH 0/3] Iron output of describe --contains --all Ramkumar Ramachandra
2013-07-07 12:43 ` Ramkumar Ramachandra [this message]
2013-07-07 18:18   ` [PATCH 1/3] name-rev: fix assumption about --name-only usage Junio C Hamano
2013-07-08 13:18     ` Ramkumar Ramachandra
2013-07-07 12:43 ` [PATCH 2/3] name-rev: strip trailing ^0 in when --name-only Ramkumar Ramachandra
2013-07-07 18:16   ` Junio C Hamano
2013-07-07 22:03     ` Junio C Hamano
2013-07-08 13:00     ` Ramkumar Ramachandra
2013-07-07 12:43 ` [PATCH 3/3] name-rev doc: rewrite --stdin paragraph Ramkumar Ramachandra
2013-07-07 18:04   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1373200996-9753-2-git-send-email-artagnon@gmail.com \
    --to=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.