[PATCH] Add a notice to the doc of git-ls-tree.

[PATCH] Add a notice to the doc of git-ls-tree.

[Steve Frécinaux]

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Petr Baudis]

On Mon, Jul 21, 2008 at 12:25:35AM +0200, Steve Frécinaux wrote:
> 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>::

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Steve Frécinaux]

> It's hard to make out what do you mean, the patch description is much
> clearer, paradoxically.

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
bitten by this and I thought it was important enough to be specified.

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

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

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

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.


	

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Petr Baudis]

On Sun, Jul 20, 2008 at 04:24:00PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > 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.
> 
> 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.

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>> 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.
>
> 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.

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Petr Baudis]

On Sun, Jul 20, 2008 at 04:53:02PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Petr Baudis <pasky@suse.cz> writes:
> >
> >> 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.
> >
> > 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.
> 
> 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)).

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

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

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.

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Petr Baudis]

On Sun, Jul 20, 2008 at 05:14:16PM -0700, Junio C Hamano wrote:
> I never thought you would think "showing relative to tree-root" is even an
> option.

I assume you mean "not filtering relative to tree-root"?

> That would make it inconsistent with not just the established
> semantics of what the plumbing did, but also with what ls-files does.

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> On Sun, Jul 20, 2008 at 05:14:16PM -0700, Junio C Hamano wrote:
>> I never thought you would think "showing relative to tree-root" is even an
>> option.
>
> I assume you mean "not filtering relative to tree-root"?

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

Re: [PATCH] Add a notice to the doc of git-ls-tree.

[Petr Baudis]

On Sun, Jul 20, 2008 at 09:28:59PM -0700, Junio C Hamano wrote:
> 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

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

				Petr "Pasky" Baudis

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

[Petr Baudis]

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>::

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

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> +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!

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
	list of files you have right now.

> +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>::

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

[Steve Frécinaux]

On Mon, Jul 21, 2008 at 9:56 AM, Petr Baudis <pasky@suse.cz> wrote:
> 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.

There is a typo in my name ;-)

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

[Petr Baudis]

On Mon, Jul 21, 2008 at 01:00:57AM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > +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!
> 
> 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
> 	list of files you have right now.

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,
or it gets too hard to read again.

> > +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>::

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

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

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> 	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
>> 	list of files you have right now.
>
> 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...

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.

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

[Petr Baudis]

On Mon, Jul 21, 2008 at 05:32:20PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> 	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
> >> 	list of files you have right now.
> >
> > 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...
> 
> I mildly disagree.

We may throw a dice or go with your version, I don't care *that* much
about this change, I just wouldn't make it personally.

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

To nudge the person to the "aha" moment.

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

I don't understand; what does auto-prefixing have to do with the
"ls -a" mention?

> It will be better understood if you describe *why* it does
> it that way at the same time.

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

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

[Petr Baudis]

On Wed, Jul 23, 2008 at 12:47:59AM +0200, Petr Baudis wrote:
> On Mon, Jul 21, 2008 at 05:32:20PM -0700, Junio C Hamano wrote:
> > Petr Baudis <pasky@suse.cz> writes:
> > 
> > >> 	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
> > >> 	list of files you have right now.
> > >
> > > 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...
> > 
> > I mildly disagree.
> 
> We may throw a dice or go with your version, I don't care *that* much
> about this change, I just wouldn't make it personally.

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

				Petr "Pasky" Baudis

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

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> We may throw a dice or go with your version, I don't care *that* much
>> about this change, I just wouldn't make it personally.
>
> What is the status of this patch? :-) Dropped altogether?

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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>>> We may throw a dice or go with your version, I don't care *that* much
>>> about this change, I just wouldn't make it personally.
>>
>> What is the status of this patch? :-) Dropped altogether?
>
> Left behind on the far side of oblivion; I do not offhand recall what this
> was about, sorry.

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.