threads / rfc / 22569

RFC patchls-tree: dump full tree if it was named as such

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

## tl;dr

3 messages between Feb 8, 2010 and Feb 15, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Thomas Rast· Feb 8, 2010, 15:59 UTC · lore

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(-)
Show changes to 4 files +36 −4

builtin-ls-tree.c, t/t3101-ls-tree-dirname.sh, tree.c, tree.h

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
Thomas Rast· Feb 14, 2010, 16:33 UTC · lore

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

[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:
Show 19 quoted lines
> 
> 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
Junio C Hamano· Feb 15, 2010, 01:47 UTC · re: Thomas Rast · lore

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

Thomas Rast <trast@student.ethz.ch> writes:
Show 6 quoted lines
> 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.

← back to recent threads