Re: [PATCH 1/3] name-rev: fix assumption about --name-only usage

2013-07-08 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 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/.

 If you _know_ v1.8.3 does not appear outside tags/, this does look
 inconsistent, but I do not think the code checks it.  Ahd if the
 code does not, I am not sure not stripping tags/ is necessarily a
 bad thing, because --all allows names to come outside tags/
 hierarchy.

Yeah, you asked for it using --all.

 Also how should this interact with v1.8.3-1-g98c5c4a that changed
 the rule somewhat so that the common prefix is stripped when we know
 the result is not ambiguous?

Completely independent of everything else.  The condition is if
name-only  prefix == refs/tags, strip that prefix.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] name-rev: fix assumption about --name-only usage

2013-07-07 Thread Ramkumar Ramachandra
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 artag...@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 aut...@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 aut...@example.com
 |
 | third
 |
-* commit tags/side-1~2
+* commit side-1~2
 | Author: A U Thor aut...@example.com
 |
 | second
 |
-* commit tags/side-1~3
+* commit side-1~3
   Author: A U Thor aut...@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 

Re: [PATCH 1/3] name-rev: fix assumption about --name-only usage

2013-07-07 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 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

 ...  As a result, while

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

The above two look consistent, yes.


 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/.

If you _know_ v1.8.3 does not appear outside tags/, this does look
inconsistent, but I do not think the code checks it.  Ahd if the
code does not, I am not sure not stripping tags/ is necessarily a
bad thing, because --all allows names to come outside tags/
hierarchy.

Also how should this interact with v1.8.3-1-g98c5c4a that changed
the rule somewhat so that the common prefix is stripped when we know
the result is not ambiguous?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html