[RFC PATCH] ls-tree: dump full tree if it was named as such

[RFC PATCH] ls-tree: dump full tree if it was named as such

[Thomas Rast]

git-ls-tree acted weird when it was called in a subdirectory but with
a tree (not commit) argument.  For example,

  # in git.git
  mkdir valgrind
  cd valgrind
  git ls-tree HEAD:t

would list the contents of HEAD:t/valgrind even though the user is
looking at /valgrind.

Fix this by not doing the implied-prefix logic when the specified
object is a tree (as in 'HEAD:t' or 'HEAD:'), as opposed to a commit
('HEAD').

To this end, we first introduce a variant of parse_tree_indirect()
that also reports back how many levels it had to traverse.  Then we
teach cmd_ls_tree() to ignore the prefix if that depth was 0.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

RFC because I'm breaking an existing test, though I can't see why
anyone would rely on such weird behaviour.


 builtin-ls-tree.c          |    9 +++++++--
 t/t3101-ls-tree-dirname.sh |   19 ++++++++++++++++++-
 tree.c                     |   11 ++++++++++-
 tree.h                     |    1 +
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index 4484185..04c6a65 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -121,6 +121,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	unsigned char sha1[20];
 	struct tree *tree;
 	int full_tree = 0;
+	int depth;
 	const struct option ls_tree_options[] = {
 		OPT_BIT('d', NULL, &ls_options, "only show trees",
 			LS_TREE_ONLY),
@@ -166,10 +167,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	if (get_sha1(argv[0], sha1))
 		die("Not a valid object name %s", argv[0]);
 
-	pathspec = get_pathspec(prefix, argv + 1);
-	tree = parse_tree_indirect(sha1);
+	tree = parse_tree_indirect_depth(sha1, &depth);
 	if (!tree)
 		die("not a tree object");
+	if (!depth) {
+		ls_tree_prefix = prefix = NULL;
+		chomp_prefix = 0;
+	}
+	pathspec = get_pathspec(prefix, argv + 1);
 	read_tree_recursive(tree, "", 0, 0, pathspec, show_tree, NULL);
 
 	return 0;
diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh
index 06654c6..a3cd80a 100755
--- a/t/t3101-ls-tree-dirname.sh
+++ b/t/t3101-ls-tree-dirname.sh
@@ -37,6 +37,7 @@ test_expect_success \
      find *.txt path* \( -type f -o -type l \) -print |
      xargs git update-index --add &&
      tree=`git write-tree` &&
+     commit=`echo dummy | git commit-tree $tree` &&
      echo $tree'
 
 test_output () {
@@ -142,7 +143,7 @@ test_expect_success 'ls-tree filter is leading path match' '
 test_expect_success 'ls-tree --full-name' '
 	(
 		cd path0 &&
-		git ls-tree --full-name $tree a
+		git ls-tree --full-name $commit a
 	) >current &&
 	cat >expected <<\EOF &&
 040000 tree X	path0/a
@@ -224,4 +225,20 @@ EOF
 	test_output
 '
 
+test_expect_success 'ls-tree $tree does not restrict to pwd' '
+	(
+		cd path0 &&
+		git ls-tree $tree
+	) >current &&
+	cat >expected <<\EOF &&
+100644 blob X	1.txt
+100644 blob X	2.txt
+040000 tree X	path0
+040000 tree X	path1
+040000 tree X	path2
+040000 tree X	path3
+EOF
+	test_output
+'
+
 test_done
diff --git a/tree.c b/tree.c
index 5ab90af..e9e8481 100644
--- a/tree.c
+++ b/tree.c
@@ -266,9 +266,11 @@ int parse_tree(struct tree *item)
 	return parse_tree_buffer(item, buffer, size);
 }
 
-struct tree *parse_tree_indirect(const unsigned char *sha1)
+struct tree *parse_tree_indirect_depth(const unsigned char *sha1,
+				       int *depth)
 {
 	struct object *obj = parse_object(sha1);
+	*depth = 0;
 	do {
 		if (!obj)
 			return NULL;
@@ -282,5 +284,12 @@ struct tree *parse_tree_indirect(const unsigned char *sha1)
 			return NULL;
 		if (!obj->parsed)
 			parse_object(obj->sha1);
+		(*depth)++;
 	} while (1);
 }
+
+struct tree *parse_tree_indirect(const unsigned char *sha1)
+{
+	int unused;
+	return parse_tree_indirect_depth(sha1, &unused);
+}
diff --git a/tree.h b/tree.h
index 2ff01a4..97f171c 100644
--- a/tree.h
+++ b/tree.h
@@ -18,6 +18,7 @@ struct tree {
 int parse_tree(struct tree *tree);
 
 /* Parses and returns the tree in the given ent, chasing tags and commits. */
+struct tree *parse_tree_indirect_depth(const unsigned char *sha1, int *depth);
 struct tree *parse_tree_indirect(const unsigned char *sha1);
 
 #define READ_TREE_RECURSIVE 1
-- 
1.7.0.rc2.178.g109e1

Re: [RFC PATCH] ls-tree: dump full tree if it was named as such

[Thomas Rast]

[I noticed only just now that you only sent this to me.  Accidentally
I suppose?]

On Monday 08 February 2010 20:10:40 Junio C Hamano wrote:
> 
> This obviously was meant to be used with the full tree recorded by a
> commit and what you are seeing (e.g. cd to "valgrind" that is not even
> tracked, and pretend HEAD:t were the full contents---the full contents of
> the tree limited to the work tree location "valgrind" is shown) is an
> artifact of that.
> 
> I think the right solution is along the lines of --full-tree option to
> allow people (i.e. scripts) to ask for the exact tree contents without any
> funny path limiting based on the location in the work tree.  They can
> apply whatever path limiting from the command line, e.g. by running
> 
>     git ls-tree --full-tree HEAD:t valgrind
> 
> instead of running
> 
>     mkdir -p valgrind && cd valgrind && git ls-tree HEAD:t
> 
> when they want to apply path limit to the ls-tree output.

I guess in the (very) long run, the scripts should be forced to always
use --full-tree so that we can eventually make it the default?

I'm just not sure how the existing behaviour could ever be useful,
though admittedly 'git ls-tree $(git write-tree)' would change
semantics if you're in a subdirectory.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [RFC PATCH] ls-tree: dump full tree if it was named as such

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> I guess in the (very) long run, the scripts should be forced to always
> use --full-tree so that we can eventually make it the default?
>
> I'm just not sure how the existing behaviour could ever be useful,
> though admittedly 'git ls-tree $(git write-tree)' would change
> semantics if you're in a subdirectory.

Hysterical raisins.  I am reasonably sure that it behaves for end-user
convenience and consistency while running these from the command line:

    $ cd t
    $ git ls-files
    $ git ls-tree HEAD

The --full-tree option came as an afterthought much later to help
Porcelain scripts, that had to cd-up-to-top to get to the full tree
contents.

The order of events and historical backstory does not matter.

The Porcelain scripts have learned to either (1) going to top before
running, or (2) giving --full-tree when they want to see a full tree
output, but at the same time they also learned that (3) running the
command without --full-tree will give them the contents of the tree that
corresponds to where the current directory is in the work tree.

All three are documented plumbing interfaces---not just the first two
points.  I haven't seen any good excuse to break the third promise with
Porcelain scripts. I didn't find one myself when I did d4789c6 (ls-tree:
add --full-tree option, 2008-12-25) long time ago.

So even in "(very) long run" nothing will happen.  I was about to write
"except documenting them better, perhaps", but after re-reading the
introductory paragraphs of the manual page, I don't think anything more
needs to be said there.