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
next prev parent 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
List information: http://vger.kernel.org/majordomo-info.html
* 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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).