Re: [PATCH] Documentation/git-ls-tree.txt: Add a caveat about prefixing pathspec

Previous thread: [RFC] Stopping those fat "What's cooking in git.git" threads by Miklos Vajna on Sunday, July 20, 2008 - 1:51 pm. (15 messages)

Next thread: [PATCH v2] Ensure that SSH runs in non-interactive mode by Fredrik Tolf on Sunday, July 20, 2008 - 5:00 pm. (8 messages)
From: Steve Frécinaux
Date: Sunday, July 20, 2008 - 3:25 pm

The notice covers this behaviour:
if you are in the git/ subdirectory of your repository, it will pick
the tree corresponding to that directory instead of the root one if you
specify the root tree object id.

Compare the output of both of those commands:
 git-ls-tree cb44e6571708aa2792c73a289d87586fe3c0c362
 git-cat-file -p cb44e6571708aa2792c73a289d87586fe3c0c362
---
 Documentation/git-ls-tree.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index 1cdec22..7cba394 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt
@@ -21,6 +21,10 @@ though - 'paths' denote just a list of patterns to match, e.g. so specifying
 directory name (without '-r') will behave differently, and order of the
 arguments does not matter.
 
+Note that if you give ls-tree the sha1 id of a parent of the tree
+corresponding to the directory you're in, it will resolve that tree and list
+its contents instead of listing the contents of the tree you gave.
+
 OPTIONS
 -------
 <tree-ish>::
-- 
1.5.6.2


--

From: Petr Baudis
Date: Sunday, July 20, 2008 - 4:08 pm

It's hard to make out what do you mean, the patch description is much
clearer, paradoxically. Also, this in fact holds for the root tree
instead of the parent tree, and the behaviour changes from "weird" to
"simply broken" when you try to list a tree object that is _not_ the
root project tree from within a subdirectory:

	git$ git ls-tree HEAD Documentation
	040000 tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1    Documentation
	git/Documentation$ git ls-tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1
	git/Documentation$

I think that ls-tree simply shouldn't auto-fill its pathspec based on
current prefix in case no pathspec was supplied. Patch to follow.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Steve Frécinaux
Date: Sunday, July 20, 2008 - 4:22 pm

It is hard to explain such a strange behaviour with a description that
is both short and generic enough... But I agree with you. I just got

I also thought this behaviour was broken. But I didn't want to patch
it because I was afraid of breaking things that would rely on it,
despite it seems unexpected enough not to be used...
--

From: Junio C Hamano
Date: Sunday, July 20, 2008 - 4:24 pm

Have you dug the list archive from mid-to-late December 2005 that prompted
the current behaviour (and introduction of --full-name)?  I haven't.  A
change to always do the --full-name can only be justified by doing so and
rehashing the issues.

On the other hand, "fix" is welcome.


	
--

From: Petr Baudis
Date: Sunday, July 20, 2008 - 4:39 pm

You are right, now that I understand the issue better, there's no good
fix for this except perhaps introducing --no-prefix, which is not my
itch to scratch. Here's my original wording improvement:

	Note that if you are within a subdirectory of your working copy,
	'git ls-tree' will automatically prepend the subdirectory prefix
	to the specified paths, and assume the prefix specified in case
	no paths were given - no matter what the tree object is! Thus,
	within a subdirectory, 'git ls-tree' behaves as expected only
	when run on a root tree object.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Petr Baudis
Date: Monday, July 21, 2008 - 12:56 am

When run in a working copy subdirectory, git-ls-tree will automagically
add the prefix to the pathspec, which can result in an unexpected behavior
when the tree object accessed is not the root tree object.

This was originally pointed out and described in a patch by
Steve Frénaux <code@istique.net>, this is a shot at clearer and more
accurate description.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-ls-tree.txt |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index 1cdec22..8bb1864 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt
@@ -21,6 +21,15 @@ though - 'paths' denote just a list of patterns to match, e.g. so specifying
 directory name (without '-r') will behave differently, and order of the
 arguments does not matter.
 
+Note that within a subdirectory of the working copy, 'git ls-tree'
+will automatically prepend the subdirectory prefix to the specified
+paths and assume just the prefix was specified in case no paths were
+given --- no matter what the tree object is!
+Thus, within a subdirectory, 'git ls-tree' behaves as expected
+only when run on a root tree object (e.g. with a 'HEAD' tree-ish,
+but not anymore when passed 'HEAD:Documentation' instead).
+
+
 OPTIONS
 -------
 <tree-ish>::

--

From: Junio C Hamano
Date: Monday, July 21, 2008 - 1:00 am

Don't be negative upfront.  Explain why this is a good thing first.

	... were given.  This is useful when you are deep in a
	subdirectory and want to inspect the list of files in an arbitrary
	commit.  E.g.

		$ cd some/deep/path
		$ git ls-tree --name-only -r HEAD~20

	will list the files in some/deep/path (i.e. where you are) 20
	commits ago, just like running "/bin/ls" there will give you the
--

From: Petr Baudis
Date: Monday, July 21, 2008 - 2:04 pm

Frankly, I think this is overdoing it. I'm all for being positive, but
it is obvious why this is good thing when you inspect a root tree and
there's no need to be too wordy about it - it should be enough to
acknowledge this later by the "as expected" as I note below.

The documentation should be detailed and complete, but not too chatty,

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Junio C Hamano
Date: Monday, July 21, 2008 - 5:32 pm

I mildly disagree.

If the person had truly understood that, why do we even have this thread
to begin with?

Description on *what* it does (i.e. "like what ls -a does in the current
working directory" we have in the Description section) obviously was not
good enough.  It will be better understood if you describe *why* it does
it that way at the same time.


--

From: Petr Baudis
Date: Tuesday, July 22, 2008 - 3:47 pm

We may throw a dice or go with your version, I don't care *that* much


I don't understand; what does auto-prefixing have to do with the

My version implies that for examining the root tree, without surplusage.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Petr Baudis
Date: Sunday, July 27, 2008 - 5:46 pm

What is the status of this patch? :-) Dropped altogether?

				Petr "Pasky" Baudis
--

From: Junio C Hamano
Date: Sunday, July 27, 2008 - 6:26 pm

Left behind on the far side of oblivion; I do not offhand recall what this
was about, sorry.
--

From: Junio C Hamano
Date: Monday, July 28, 2008 - 2:23 am

Ok, I now recall this bit:

        You are right, now that I understand the issue better, there's no good
        fix for this except perhaps introducing --no-prefix, which is not my
        itch to scratch. Here's my original wording improvement:

                Note that if you are within a subdirectory of your working copy,
                'git ls-tree' will automatically prepend the subdirectory prefix
                to the specified paths, and assume the prefix specified in case
                no paths were given - no matter what the tree object is! Thus,
                within a subdirectory, 'git ls-tree' behaves as expected only
                when run on a root tree object.

Eventually somebody may write a Porcelain that benefits from --no-prefix,
but it is safe to defer the implementation until the need becomes real.

I'll add some explanatory message to the documentation.
--

From: Steve Frécinaux
Date: Monday, July 21, 2008 - 1:45 am

There is a typo in my name ;-)
--

From: Junio C Hamano
Date: Sunday, July 20, 2008 - 4:53 pm

Now, I did:

    http://thread.gmane.org/gmane.comp.version-control.git/13028/focus=13135

I think the answer is --full-name (cf. a69dd58 (ls-tree: chomp leading
directories when run from a subdirectory, 2005-12-23)).

--

From: Petr Baudis
Date: Sunday, July 20, 2008 - 5:08 pm

I don't understand your point now.  --full-name cares only about the
displaying part; do you suggest that it should be extended to also turn
off prepending the prefix during the filtering phase? That would make a
lot of sense, if you are not worried about compatibility trouble.

-- 
			Petr "Pasky, missing something" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Junio C Hamano
Date: Sunday, July 20, 2008 - 5:14 pm

Ah, sorry, I thought you were talking about the display part.

I never thought you would think "showing relative to tree-root" is even an
option.  That would make it inconsistent with not just the established
semantics of what the plumbing did, but also with what ls-files does.
--

From: Petr Baudis
Date: Sunday, July 20, 2008 - 5:22 pm

But ls-files always works on the index; ls-tree can work on trees,
and when you're inspecting a non-root tree object from within
a subdirectory, this behaviour can be rather unexpected. But as I said,
I'm fine with just documenting it.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie
--

From: Junio C Hamano
Date: Sunday, July 20, 2008 - 9:28 pm

Sorry, I may have been unclear.  I meant "showing relative to tree-root,
unlike showing relative to cwd like we have done forever".

Changing the behaviour would affect usage like this:

	$ cd some/where
        $ git ls-files
        $ git ls-tree --name-only -r HEAD^

    cf. http://thread.gmane.org/gmane.comp.version-control.git/13028/focus=13080
--

From: Petr Baudis
Date: Monday, July 21, 2008 - 12:47 am

Yes, as I said, by now I agree that this is not acceptable, and thus
opted for just documenting the behaviour. :-)

				Petr "Pasky" Baudis
--

Previous thread: [RFC] Stopping those fat "What's cooking in git.git" threads by Miklos Vajna on Sunday, July 20, 2008 - 1:51 pm. (15 messages)

Next thread: [PATCH v2] Ensure that SSH runs in non-interactive mode by Fredrik Tolf on Sunday, July 20, 2008 - 5:00 pm. (8 messages)