threads / patch / 16865

patchls-tree: add --full-tree option

Subject: [PATCH] ls-tree: add --full-tree option

## tl;dr

6 messages between Dec 26, 2008 and Dec 26, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Junio C Hamano· Dec 26, 2008, 00:54 UTC · lore

The established behaviour of "git ls-tree $commit" run from a subdirectory "sub/dir" in a work tree is to limit the output to the paths in the subdirectory, and strip off the leading "sub/dir" from the output, since 3c5e846 (ls-tree: major rewrite to do pathspec, 2005-11-26).

This was a "usability" feature made back in the days when the line between Porcelain and plumbing was blurry, and in retrospect, it probably was misguided. The behaviour may be what the end user would expect when the command is run interactively from a subdirectory, but it also means that a scripted Porcelain that wants to use the command to list the full contents of a tree object has to do cd_to_toplevel (and save the output from "rev-parse --show-prefix" before doing so, so that it can be used as a pathspec if it wants to limit its operation to the original subdirectory in other commands).

This new option makes the command operate on the full tree object, regardless of where in the work tree it is run from. It also implies the behaviour that is triggered by the existing --full-name option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * There was a discussion on #git between Ilari and deskin this morning;
   hopefully I captured what they wanted correctly.
   I do not care too deeply with this change myself, though...
 builtin-ls-tree.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to builtin-ls-tree.c +5 −0
diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index cb61717..c386aa5 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -156,6 +156,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 				chomp_prefix = 0;
 				break;
 			}
+			if (!strcmp(argv[1]+2, "full-tree")) {
+				ls_tree_prefix = prefix = NULL;
+				chomp_prefix = 0;
+				break;
+			}
 			if (!prefixcmp(argv[1]+2, "abbrev=")) {
 				abbrev = strtoul(argv[1]+9, NULL, 10);
 				if (abbrev && abbrev < MINIMUM_ABBREV)
-- 
1.6.1
Johannes Sixt· Dec 26, 2008, 08:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] ls-tree: add --full-tree option

On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
> This new option makes the command operate on the full tree object,
> regardless of where in the work tree it is run from.  It also implies the
> behaviour that is triggered by the existing --full-name option.
What's wrong with using 'git ls-tree ${rev}:'?

Except that it does not work and still silently inserts the subdirectory, which I consider a serious bug...

-- Hannes
Junio C Hamano· Dec 26, 2008, 08:55 UTC · re: Johannes Sixt · lore

Re: [PATCH] ls-tree: add --full-tree option

Johannes Sixt <j6t@kdbg.org> writes:
Show 8 quoted lines
> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>> This new option makes the command operate on the full tree object,
>> regardless of where in the work tree it is run from.  It also implies the
>> behaviour that is triggered by the existing --full-name option.
>
> What's wrong with using 'git ls-tree ${rev}:'?
>
> Except that it does not work...

Hmph... you seem to be describing the exact issue they discussed on #git, which triggered the patch in the message you are responding to. I am not sure what to say to your "What's wrong with...".

demerphq· Dec 26, 2008, 10:31 UTC · re: Junio C Hamano · lore

Re: [PATCH] ls-tree: add --full-tree option

2008/12/26 Junio C Hamano <gitster@pobox.com>:
Show 14 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>>> This new option makes the command operate on the full tree object,
>>> regardless of where in the work tree it is run from.  It also implies the
>>> behaviour that is triggered by the existing --full-name option.
>>
>> What's wrong with using 'git ls-tree ${rev}:'?
>>
>> Except that it does not work...
>
> Hmph... you seem to be describing the exact issue they discussed on #git,
> which triggered the patch in the message you are responding to.  I am not
> sure what to say to your "What's wrong with...".

AFAIK I started that discussion, and during it we also mentioned that it would be very useful to be able to use git ls-files in non recursive mode. As the advantage of using ls-tree, prior to your fix, was that it is the only way to get a list of files for the current working directory and only the current working directory.

Since you have fixed that bug what command does one us to get a list of tracked files for the current working directory without recursing into subdirectories? Can we have a switch on ls-files that disables recursion? Please?

Cheers, Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"
Johannes Sixt· Dec 26, 2008, 19:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] ls-tree: add --full-tree option

Junio C Hamano schrieb:
Show 13 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>>> This new option makes the command operate on the full tree object,
>>> regardless of where in the work tree it is run from.  It also implies the
>>> behaviour that is triggered by the existing --full-name option.
>> What's wrong with using 'git ls-tree ${rev}:'?
>>
>> Except that it does not work...
> 
> Hmph... you seem to be describing the exact issue they discussed on #git,
> which triggered the patch in the message you are responding to.  I am not
> sure what to say to your "What's wrong with...".

I'm saying that if a script has to be fixed to use --full-tree, then it can be fixed just as well by appending the colon to the ${rev}.

Not even porcelain 'git show ${rev}:' "is allowed" to insert the prefix. Dscho has argued the case passionately in the past:

http://thread.gmane.org/gmane.comp.version-control.git/68786/focus=68846
Therefore, plumbing 'ls-tree ${rev}:' shouldn't do that, either.

OTOH, you had yourself argued somewhat in favor of the current ls-tree behavior:

http://thread.gmane.org/gmane.comp.version-control.git/46232/focus=46400

I'm personally more in line with Dscho, and think that the current ls-tree behavior is a terrible bug.

-- Hannes
Junio C Hamano· Dec 26, 2008, 21:38 UTC · re: Johannes Sixt · lore

Re: [PATCH] ls-tree: add --full-tree option

Johannes Sixt <j6t@kdbg.org> writes:
> I'm saying that if a script has to be fixed to use --full-tree, then
> it can be fixed just as well by appending the colon to the ${rev}.

I do not agree with your "just as well"; an existing script that feeds a tree object to the plumbing would be broken by such a change.

But I think perhaps you were responding to the first paragraph of the commit log message that you omitted from your quote?

JC> The established behaviour of "git ls-tree $commit" run from a JC> subdirectory "sub/dir" in a work tree is to...

If that is the case, I understand what you meant.

The patch is about the behaviour of the command for not just $commit but any $tree_ish, so "git ls-tree ${commit}:" shares the exact same issue (i.e. historical background that forbids us from changing the behaviour without an explicit option, and that --full-tree can be a way to help new scripts without breaking existing scripts' expectations).

I've updated the commit log message with s/$commit/$tree_ish/;
> OTOH, you had yourself argued somewhat in favor of the current ls-tree
> behavior:
>
> http://thread.gmane.org/gmane.comp.version-control.git/46232/focus=46400

That's not really "somewhat in favor". I can be (and am more often than not) sympathetic without agreeing to the end result. My sympathy extends from "I can sort-of-kind-of imagine that it may hurt, and even though I do not think your approach is a way to properly address it at all, I'd agree it might be nice to have some solution to the issue" to "I do not think it is feasible to change this anymore, but I wish we could, too."

In any case, the quoted message was from May 2007, way before v1.6.0 when we learned the hard way that people do not want any change.

I really hate "take it or leave it", especially when it is not my itch to scratch, but in this case, I think I've spent enough time making myself clear that I think (1) that the current behaviour is a result of misguided attempt for interactive user expectation, which shouldn't have made to the plumbing, (2) that we cannot change the default behaviour now even (1) is true; and (3) that the only possible approach to help new scripts would be a new option.

← back to recent threads