[PATCH] ls-tree path restriction semantics fixes

[PATCH] ls-tree path restriction semantics fixes

[Jason McMullan]

This patch fixes the git-ls-tree semantics to be less stupid, namely:
	
	* ls of a 'tree' path should just return the SHA1 of the tree
	* ls of a 'tree' path with a trailing '/' should work properly
	* ls of two identical paths should have the same output as ls of
	  a single path. (I consider ls-tree's output to be a hash dictionary)

Also, I added test cases to verify that these issues are fixed.

Old Results:

	$ git-ls-tree t
	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t
	100644 blob 6882e23be568ccf14f3adb0c766139086f2ee952    t/Makefile
	100644 blob 2a94fdb0b83ab5fcbf1a2c6edaf36c2dbe765ec6    t/README
	100644 blob d920c6b3a3bfbb5994244a78d1ad99ce02748122    t/lib-read-tree-m-3way.sh
	...

	$ git-ls-tree t/
	(no output)

	$ git-ls-tree t t
	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t

New Results:

	$ git-ls-tree f
	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t

	$ git-ls-tree t/
	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t

	$ git-ls-tree t t
	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t

Signed-Off-By: Jason McMullan <jason.mcmullan@timesys.com>

diff --git a/ls-tree.c b/ls-tree.c
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -13,8 +13,6 @@ struct path_prefix {
 	const char *name;
 };
 
-#define DEBUG(fmt, ...)	
-
 static int string_path_prefix(char *buff, size_t blen, struct path_prefix *prefix)
 {
 	int len = 0;
@@ -118,6 +116,8 @@ static void list_recursive(void *buffer,
 			mtype = pathcmp(match[mindex],&this_prefix);
 			if (mtype >= 0) {
 				matched = match[mindex];
+				/* Skip over any duplicates */
+				for (; mindex+1 < matches && strcmp(match[mindex+1],matched)==0; mindex++);
 				break;
 			}
 		}
@@ -140,19 +140,22 @@ static void list_recursive(void *buffer,
 		if (matches && ! matched)
 			continue;
 
-		if (! (eltbuf = read_sha1_file(sha1, elttype, &eltsize)) ) {
-			error("cannot read %s", sha1_to_hex(sha1));
-			continue;
-		}
-
 		/* If this is an exact directory match, we may have
 		 * directory files following this path. Match on them.
-		 * Otherwise, we're at a pach subcomponent, and we need
+		 * Otherwise, we're at a path subcomponent, and we need
 		 * to try to match again.
 		 */
 		if (mtype == 0)
 			mindex++;
 
+		if (matched && matches-mindex==0)
+			continue;
+
+		if (! (eltbuf = read_sha1_file(sha1, elttype, &eltsize)) ) {
+			error("cannot read %s", sha1_to_hex(sha1));
+			continue;
+		}
+
 		list_recursive(eltbuf, elttype, eltsize, &this_prefix, &match[mindex], matches-mindex);
 		free(eltbuf);
 	}
@@ -169,9 +172,14 @@ static int list(unsigned char *sha1,char
 	unsigned long size;
 	int npaths;
 
-	for (npaths = 0; path[npaths] != NULL; npaths++)
-		;
+	/* Count the paths, and any trailling '/' */
+	for (npaths = 0; path[npaths] != NULL; npaths++) {
+		char *cp = strrchr(path[npaths],'/');
+		if (cp != NULL && *(cp+1) == 0)
+			*cp=0;
+	}	
 
+	/* Sort the paths */
 	qsort(path,npaths,sizeof(char *),qcmp);
 
 	buffer = read_object_with_reference(sha1, "tree", &size, NULL);
diff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh
--- a/t/t3100-ls-tree-restrict.sh
+++ b/t/t3100-ls-tree-restrict.sh
@@ -84,10 +84,22 @@ test_expect_success \
     'git-ls-tree $tree path2 >current &&
      cat >expected <<\EOF &&
 040000 tree X	path2
-040000 tree X	path2/baz
-100644 blob X	path2/baz/b
-120000 blob X	path2/bazbo
-100644 blob X	path2/foo
+EOF
+     test_output'
+
+test_expect_success \
+    'ls-tree filtered' \
+    'git-ls-tree $tree path2/ >current &&
+     cat >expected <<\EOF &&
+040000 tree X	path2
+EOF
+     test_output'
+
+test_expect_success \
+    'ls-tree filtered' \
+    'git-ls-tree $tree path2 path2 >current &&
+     cat >expected <<\EOF &&
+040000 tree X	path2
 EOF
      test_output'
 
@@ -96,7 +108,16 @@ test_expect_success \
     'git-ls-tree $tree path2/baz >current &&
      cat >expected <<\EOF &&
 040000 tree X	path2/baz
-100644 blob X	path2/baz/b
+EOF
+     test_output'
+
+test_expect_success \
+    'ls-tree filtered' \
+    'git-ls-tree $tree path2 path2/bazbo path2/baz >current &&
+     cat >expected <<\EOF &&
+040000 tree X	path2
+040000 tree X	path2/baz
+120000 blob X	path2/bazbo
 EOF
      test_output'
 

Re: [PATCH] ls-tree path restriction semantics fixes

[Junio C Hamano]

>>>>> "JM" == Jason McMullan <jason.mcmullan@timesys.com> writes:

JM> This patch fixes the git-ls-tree semantics to be less stupid, namely:
JM> 	* ls of a 'tree' path should just return the SHA1 of the tree
JM> 	* ls of a 'tree' path with a trailing '/' should work properly
JM> 	* ls of two identical paths should have the same output as ls of
JM> 	  a single path. (I consider ls-tree's output to be a hash dictionary)

I haven't read your code yet, but...

JM> Old Results:

JM> 	$ git-ls-tree t
JM> 	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t
JM> 	100644 blob 6882e23be568ccf14f3adb0c766139086f2ee952    t/Makefile
JM> 	100644 blob 2a94fdb0b83ab5fcbf1a2c6edaf36c2dbe765ec6    t/README
JM> 	100644 blob d920c6b3a3bfbb5994244a78d1ad99ce02748122    t/lib-read-tree-m-3way.sh
JM> 	...

I presume the counterpart to this one in your "New Results"
example, which is spelled "git-ls-tree f" is a typo of
"git-ls-tree t", but if that is the case I strongly disagree.

What you really want is something similar to '-d' flag to
/bin/ls.  You are interested in the directory itself not its
contents and I think your gripe is that giving a path that
matches a tree always descends into it (i.e. there is no way to
do the equivalent of "/bin/ls -d t").  I agree that it is a
problem, but changing "/bin/ls t" not to show the directory
contents of "t" is not a solution.

JM> 	$ git-ls-tree t/
JM> 	(no output)

I agree with you that this is not what we want and we should
behave the same way as "git-ls-tree t" would in this case.

JM> 	$ git-ls-tree t t
JM> 	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t

I do not know what you wanted to say in this example.  Your
"Old" and "New" look the same to me.

Re: [PATCH] ls-tree path restriction semantics fixes

[Jason McMullan]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

On Fri, 2005-05-27 at 11:16 -0700, Junio C Hamano wrote:
> What you really want is something similar to '-d' flag to
> /bin/ls.  You are interested in the directory itself not its
> contents and I think your gripe is that giving a path that
> matches a tree always descends into it (i.e. there is no way to
> do the equivalent of "/bin/ls -d t").  I agree that it is a
> problem, but changing "/bin/ls t" not to show the directory
> contents of "t" is not a solution.

  git-ls-tree reporting just the tree's hash is valid, because if
you want everything in that tree, you can just do:

git-ls-tree `git-ls-tree HEAD path/dir | (read m t h n; echo $h)`

  I don't see the problem there.


> JM> 	$ git-ls-tree t t
> JM> 	040000 tree 4eeb3990955b8badc4c14712b89d8cd9fff02f15    t
> 
> I do not know what you wanted to say in this example.  Your
> "Old" and "New" look the same to me.

The problem was that 't' and 't t' produced *vastly* different output
in the old code. 't' would emit everything in the tree, and 't t' would
only emit t's hash.

-- 
Jason McMullan <jason.mcmullan@timesys.com>
TimeSys Corporation


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ls-tree path restriction semantics fixes

[Junio C Hamano]

>>>>> "JM" == Jason McMullan <jason.mcmullan@timesys.com> writes:

JM>   git-ls-tree reporting just the tree's hash is valid, because if
JM> you want everything in that tree, you can just do:

JM> git-ls-tree `git-ls-tree HEAD path/dir | (read m t h n; echo $h)`

JM>   I don't see the problem there.

I do not see the problem either in Turing sense, but that is
like saying you could code anything given an assembler.  There
is a difference between being possible and being practical.

I do think the current behaviour is broken, so I think we are in
half agreement.  What I think is the cleanest would be to make
"git-ls-tree $tree" behave similarly to what "/bin/ls -a" does.
Then we have various combination of options, and also path
arguments, to think about.  How about doing something like this?

 - Running without any paths.

   "git-ls-tree $tree" shows everything first level, just like
   "/bin/ls -a" shows everything in cwd.  There is nothing to
   fix here.

 - Running with paths.

   "git-ls-tree $tree path1 path2..." should show path$n if
   path$n is not a tree and everything under path$n including
   path$n itself if path$n is a tree, just like the way "/bin/ls
   -a path1 path2..." works.  There is major breakage here as
   you pointed out with your "git-ls-tree $tree t" vs
   "git-ls-tree $tree t t" example.

 - Recursive behaviour without paths.

   "git-ls-tree -r $tree" should show everything recursively,
   just like what "/bin/ls -a -R" does.  There is nothing to
   fix.

 - Recursive behaviour with paths.

   "git-ls-tree -r $tree path1 path2..." should show everything
   recursively under path$n, just like what "/bin/ls -a -R path1
   path2..." does.  Again this is not how it currently works as
   you pointed out.

 - With paths but not descending into them.

   "git-ls-tree -d $tree path1 path2..." should show only the
   named path$n even when path$n is a tree, just like what
   "/bin/ls -a -R -d path1 path2..." does.  This is what is
   missing from today's git-ls-tree.

[PATCH-RFC] Rewrite ls-tree to behave more like "/bin/ls -a"

[Junio C Hamano]

This is a complete rewrite of ls-tree to make it behave more
like what "/bin/ls -a" does in the current working directory.

Namely, the changes are:

 - Unlike the old ls-tree behaviour that used paths arguments to
   restrict output (not that it worked as intended---as pointed
   out in the mailing list discussion, it was quite incoherent),
   this rewrite uses paths arguments to specify what to show.

 - Without arguments, it implicitly uses the root level as its
   sole argument ("/bin/ls -a" behaves as if "." is given
   without argument).

 - Without -r (recursive) flag, it shows the named blob (either
   file or symlink), or the named tree and its immediate
   children.

 - With -r flag, it shows the named path, and recursively
   descends into it if it is a tree.

 - With -d flag, it shows the named path and does not show its
   children even if the path is a tree, nor descends into it
   recursively.

This is still request-for-comments patch.  There is no mailing
list consensus that this proposed new behaviour is a good one.

The patch to t/t3100-ls-tree-restrict.sh illustrates
user-visible behaviour changes.  Namely:

 * "git-ls-tree $tree path1 path0" lists path1 first and then
   path0.  It used to use paths as an output restrictor and
   showed output in cache entry order (i.e. path0 first and then
   path1) regardless of the order of paths arguments.

 * "git-ls-tree $tree path2" lists path2 and its immediate
   children but having explicit paths argument does not imply
   recursive behaviour anymore, hence paths/baz is shown but not
   paths/baz/b.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

Documentation/git-ls-tree.txt |   20 +-
ls-tree.c                     |  333 +++++++++++++++++++++++-------------------
t/t3100-ls-tree-restrict.sh   |    3 
tree.c                        |    2 
tree.h                        |    1 
5 files changed, 199 insertions(+), 160 deletions(-)

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt
@@ -4,23 +4,26 @@ v0.1, May 2005
 
 NAME
 ----
-git-ls-tree - Displays a tree object in human readable form
+git-ls-tree - Lists the contents of a tree object.
 
 
 SYNOPSIS
 --------
-'git-ls-tree' [-r] [-z] <tree-ish> [paths...]
+'git-ls-tree' [-d] [-r] [-z] <tree-ish> [paths...]
 
 DESCRIPTION
 -----------
-Converts the tree object to a human readable (and script processable)
-form.
+Lists the contents of a tree object, like what "/bin/ls -a" does
+in the current working directory.
 
 OPTIONS
 -------
 <tree-ish>::
 	Id of a tree.
 
+-d::
+	show only the named tree entry itself, not its children
+
 -r::
 	recurse into sub-trees
 
@@ -28,18 +31,19 @@ OPTIONS
 	\0 line termination on output
 
 paths::
-	Optionally, restrict the output of git-ls-tree to specific
-	paths. Directories will only list their tree blob ids.
-	Implies -r.
+	When paths are given, shows them.  Otherwise implicitly
+	uses the root level of the tree as the sole path argument.
+
 
 Output Format
 -------------
-        <mode>\t	<type>\t	<object>\t	<file>
+        <mode> SP <type> SP <object> TAB <file>
 
 
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
+Completely rewritten from scratch by Junio C Hamano <junkio@cox.net>
 
 Documentation
 --------------
diff --git a/ls-tree.c b/ls-tree.c
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -4,188 +4,217 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 #include "cache.h"
+#include "blob.h"
+#include "tree.h"
 
 static int line_termination = '\n';
-static int recursive = 0;
+#define LS_RECURSIVE 1
+#define LS_TREE_ONLY 2
+static int ls_options = 0;
 
-struct path_prefix {
-	struct path_prefix *prev;
-	const char *name;
-};
-
-#define DEBUG(fmt, ...)	
-
-static int string_path_prefix(char *buff, size_t blen, struct path_prefix *prefix)
-{
-	int len = 0;
-	if (prefix) {
-		if (prefix->prev) {
-			len = string_path_prefix(buff,blen,prefix->prev);
-			buff += len;
-			blen -= len;
-			if (blen > 0) {
-				*buff = '/';
-				len++;
-				buff++;
-				blen--;
-			}
-		}
-		strncpy(buff,prefix->name,blen);
-		return len + strlen(prefix->name);
-	}
+static struct tree_entry_list root_entry;
 
-	return 0;
+static void prepare_root(unsigned char *sha1)
+{
+	unsigned char rsha[20];
+	unsigned long size;
+	void *buf;
+	struct tree *root_tree;
+
+	buf = read_object_with_reference(sha1, "tree", &size, rsha);
+	free(buf);
+	if (!buf)
+		die("Could not read %s", sha1_to_hex(sha1));
+
+	root_tree = lookup_tree(rsha);
+	if (!root_tree)
+		die("Could not read %s", sha1_to_hex(sha1));
+
+	/* Prepare a fake entry */
+	root_entry.directory = 1;
+	root_entry.executable = root_entry.symlink = 0;
+	root_entry.mode = S_IFDIR;
+	root_entry.name = "";
+	root_entry.item.tree = root_tree;
+	root_entry.parent = NULL;
 }
 
-static void print_path_prefix(struct path_prefix *prefix)
+static int prepare_children(struct tree_entry_list *elem)
 {
-	if (prefix) {
-		if (prefix->prev) {
-			print_path_prefix(prefix->prev);
-			putchar('/');
-		}
-		fputs(prefix->name, stdout);
+	if (!elem->directory)
+		return -1;
+	if (!elem->item.tree->object.parsed) {
+		struct tree_entry_list *e;
+		if (parse_tree(elem->item.tree))
+			return -1;
+		/* Set up the parent link */
+		for (e = elem->item.tree->entries; e; e = e->next)
+			e->parent = elem;
 	}
+	return 0;
 }
 
-/*
- * return:
- * 	-1 if prefix is *not* a subset of path
- * 	 0 if prefix == path
- * 	 1 if prefix is a subset of path
- */
-static int pathcmp(const char *path, struct path_prefix *prefix)
-{
-	char buff[PATH_MAX];
-	int len,slen;
+static struct tree_entry_list *find_entry_0(struct tree_entry_list *elem,
+					    const char *path,
+					    const char *path_end)
+{
+	const char *ep;
+	int len;
+
+	while (path < path_end) {
+		if (prepare_children(elem))
+			return NULL;
 
-	if (prefix == NULL)
-		return 1;
+		/* In elem->tree->entries, find the one that has name
+		 * that matches what is between path and ep.
+		 */
+		elem = elem->item.tree->entries;
 
-	len = string_path_prefix(buff, sizeof buff, prefix);
-	slen = strlen(path);
+		ep = strchr(path, '/');
+		if (!ep || path_end <= ep)
+			ep = path_end;
+		len = ep - path;
+
+		while (elem) {
+			if ((strlen(elem->name) == len) &&
+			    !strncmp(elem->name, path, len))
+				break;
+			elem = elem->next;
+		}
+		if (path_end <= ep || !elem)
+			return elem;
+		while (*ep == '/' && ep < path_end)
+			ep++;
+		path = ep;
+	}
+	return NULL;
+}
 
-	if (slen < len)
-		return -1;
+static struct tree_entry_list *find_entry(const char *path,
+					  const char *path_end)
+{
+	/* Find tree element, descending from root, that
+	 * corresponds to the named path, lazily expanding
+	 * the tree if possible.
+	 */
+	if (path == path_end) {
+		/* Special.  This is the root level */
+		return &root_entry;
+	}
+	return find_entry_0(&root_entry, path, path_end);
+}
 
-	if (strncmp(path,buff,len) == 0) {
-		if (slen == len)
-			return 0;
-		else
-			return 1;
+static void show_entry_name(struct tree_entry_list *e)
+{
+	/* This is yucky.  The root level is there for
+	 * our convenience but we really want to do a
+	 * forest.
+	 */
+	if (e->parent && e->parent != &root_entry) {
+		show_entry_name(e->parent);
+		putchar('/');
 	}
+	printf("%s", e->name);
+}
 
-	return -1;
-}	
+static const char *entry_type(struct tree_entry_list *e)
+{
+	return (e->directory ? "tree" : "blob");
+}
 
-/*
- * match may be NULL, or a *sorted* list of paths
- */
-static void list_recursive(void *buffer,
-			   const char *type,
-			   unsigned long size,
-			   struct path_prefix *prefix,
-			   char **match, int matches)
-{
-	struct path_prefix this_prefix;
-	this_prefix.prev = prefix;
-
-	if (strcmp(type, "tree"))
-		die("expected a 'tree' node");
-
-	if (matches)
-		recursive = 1;
-
-	while (size) {
-		int namelen = strlen(buffer)+1;
-		void *eltbuf = NULL;
-		char elttype[20];
-		unsigned long eltsize;
-		unsigned char *sha1 = buffer + namelen;
-		char *path = strchr(buffer, ' ') + 1;
-		unsigned int mode;
-		const char *matched = NULL;
-		int mtype = -1;
-		int mindex;
-
-		if (size < namelen + 20 || sscanf(buffer, "%o", &mode) != 1)
-			die("corrupt 'tree' file");
-		buffer = sha1 + 20;
-		size -= namelen + 20;
-
-		this_prefix.name = path;
-		for ( mindex = 0; mindex < matches; mindex++) {
-			mtype = pathcmp(match[mindex],&this_prefix);
-			if (mtype >= 0) {
-				matched = match[mindex];
-				break;
-			}
-		}
+static const char *entry_hex(struct tree_entry_list *e)
+{
+	return sha1_to_hex(e->directory
+			   ? e->item.tree->object.sha1
+			   : e->item.blob->object.sha1);
+}
 
-		/*
-		 * If we're not matching, or if this is an exact match,
-		 * print out the info
-		 */
-		if (!matches || (matched != NULL && mtype == 0)) {
-			printf("%06o %s %s\t", mode,
-			       S_ISDIR(mode) ? "tree" : "blob",
-			       sha1_to_hex(sha1));
-			print_path_prefix(&this_prefix);
-			putchar(line_termination);
-		}
+/* forward declaration for mutually recursive routines */
+static int show_entry(struct tree_entry_list *, int);
 
-		if (! recursive || ! S_ISDIR(mode))
-			continue;
+static int show_children(struct tree_entry_list *e, int level)
+{
+	if (prepare_children(e))
+		die("internal error: ls-tree show_children called with non tree");
+	e = e->item.tree->entries;
+	while (e) {
+		show_entry(e, level);
+		e = e->next;
+	}
+	return 0;
+}
 
-		if (matches && ! matched)
-			continue;
+static int show_entry(struct tree_entry_list *e, int level)
+{
+	int err = 0; 
 
-		if (! (eltbuf = read_sha1_file(sha1, elttype, &eltsize)) ) {
-			error("cannot read %s", sha1_to_hex(sha1));
-			continue;
-		}
+	if (e != &root_entry) {
+		printf("%06o %s %s	", e->mode, entry_type(e),
+		       entry_hex(e));
+		show_entry_name(e);
+		putchar(line_termination);
+	}
 
-		/* If this is an exact directory match, we may have
-		 * directory files following this path. Match on them.
-		 * Otherwise, we're at a pach subcomponent, and we need
-		 * to try to match again.
+	if (e->directory) {
+		/* If this is a directory, we have the following cases:
+		 * (1) This is the top-level request (explicit path from the
+		 *     command line, or "root" if there is no command line).
+		 *  a. Without any flag.  We show direct children.  We do not 
+		 *     recurse into them.
+		 *  b. With -r.  We do recurse into children.
+		 *  c. With -d.  We do not recurse into children.
+		 * (2) We came here because our caller is either (1-a) or
+		 *     (1-b).
+		 *  a. Without any flag.  We do not show our children (which
+		 *     are grandchildren for the original request).
+		 *  b. With -r.  We continue to recurse into our children.
+		 *  c. With -d.  We should not have come here to begin with.
 		 */
-		if (mtype == 0)
-			mindex++;
-
-		list_recursive(eltbuf, elttype, eltsize, &this_prefix, &match[mindex], matches-mindex);
-		free(eltbuf);
+		if (level == 0 && !(ls_options & LS_TREE_ONLY))
+			/* case (1)-a and (1)-b */
+			err = err | show_children(e, level+1);
+		else if (level && ls_options & LS_RECURSIVE)
+			/* case (2)-b */
+			err = err | show_children(e, level+1);
 	}
+	return err;
 }
 
-static int qcmp(const void *a, const void *b)
+static int list_one(const char *path, const char *path_end)
 {
-	return strcmp(*(char **)a, *(char **)b);
+	int err = 0;
+	struct tree_entry_list *e = find_entry(path, path_end);
+	if (!e) {
+		/* traditionally ls-tree does not complain about
+		 * missing path.  We may change this later to match
+		 * what "/bin/ls -a" does, which is to complain.
+		 */
+		return err;
+	}
+	err = err | show_entry(e, 0);
+	return err;
 }
 
-static int list(unsigned char *sha1,char **path)
+static int list(char **path)
 {
-	void *buffer;
-	unsigned long size;
-	int npaths;
-
-	for (npaths = 0; path[npaths] != NULL; npaths++)
-		;
-
-	qsort(path,npaths,sizeof(char *),qcmp);
-
-	buffer = read_object_with_reference(sha1, "tree", &size, NULL);
-	if (!buffer)
-		die("unable to read sha1 file");
-	list_recursive(buffer, "tree", size, NULL, path, npaths);
-	free(buffer);
-	return 0;
+	int i;
+	int err = 0;
+	for (i = 0; path[i]; i++) {
+		int len = strlen(path[i]);
+		while (0 <= len && path[i][len] == '/')
+			len--;
+		err = err | list_one(path[i], path[i] + len);
+	}
+	return err;
 }
 
-static const char *ls_tree_usage = "git-ls-tree [-r] [-z] <key> [paths...]";
+static const char *ls_tree_usage =
+	"git-ls-tree [-d] [-r] [-z] <tree-ish> [path...]";
 
 int main(int argc, char **argv)
 {
+	static char *path0[] = { "", NULL };
+	char **path;
 	unsigned char sha1[20];
 
 	while (1 < argc && argv[1][0] == '-') {
@@ -194,7 +223,10 @@ int main(int argc, char **argv)
 			line_termination = 0;
 			break;
 		case 'r':
-			recursive = 1;
+			ls_options |= LS_RECURSIVE;
+			break;
+		case 'd':
+			ls_options |= LS_TREE_ONLY;
 			break;
 		default:
 			usage(ls_tree_usage);
@@ -206,7 +238,10 @@ int main(int argc, char **argv)
 		usage(ls_tree_usage);
 	if (get_sha1(argv[1], sha1) < 0)
 		usage(ls_tree_usage);
-	if (list(sha1, &argv[2]) < 0)
+
+	path = (argc == 2) ? path0 : (argv + 2);
+	prepare_root(sha1);
+	if (list(path) < 0)
 		die("list failed");
 	return 0;
 }
diff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh
--- a/t/t3100-ls-tree-restrict.sh
+++ b/t/t3100-ls-tree-restrict.sh
@@ -74,8 +74,8 @@ test_expect_success \
     'ls-tree filtered' \
     'git-ls-tree $tree path1 path0 >current &&
      cat >expected <<\EOF &&
-100644 blob X	path0
 120000 blob X	path1
+100644 blob X	path0
 EOF
      test_output'
 
@@ -85,7 +85,6 @@ test_expect_success \
      cat >expected <<\EOF &&
 040000 tree X	path2
 040000 tree X	path2/baz
-100644 blob X	path2/baz/b
 120000 blob X	path2/bazbo
 100644 blob X	path2/foo
 EOF
diff --git a/tree.c b/tree.c
--- a/tree.c
+++ b/tree.c
@@ -133,7 +133,7 @@ int parse_tree_buffer(struct tree *item,
 		}
 		if (obj)
 			add_ref(&item->object, obj);
-
+		entry->parent = NULL; /* needs to be filled by the user */
 		*list_p = entry;
 		list_p = &entry->next;
 	}
diff --git a/tree.h b/tree.h
--- a/tree.h
+++ b/tree.h
@@ -16,6 +16,7 @@ struct tree_entry_list {
 		struct tree *tree;
 		struct blob *blob;
 	} item;
+	struct tree_entry_list *parent;
 };
 
 struct tree {
------------------------------------------------

Re: [PATCH-RFC] Rewrite ls-tree to behave more like "/bin/ls -a"

[Jason McMullan]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

On Sat, 2005-05-28 at 00:05 -0700, Junio C Hamano wrote:
>  - Unlike the old ls-tree behaviour that used paths arguments to
>    restrict output (not that it worked as intended---as pointed
>    out in the mailing list discussion, it was quite incoherent),
>    this rewrite uses paths arguments to specify what to show.
> 
>  - Without arguments, it implicitly uses the root level as its
>    sole argument ("/bin/ls -a" behaves as if "." is given
>    without argument).
> 
>  - Without -r (recursive) flag, it shows the named blob (either
>    file or symlink), or the named tree and its immediate
>    children.
> 
>  - With -r flag, it shows the named path, and recursively
>    descends into it if it is a tree.
> 
>  - With -d flag, it shows the named path and does not show its
>    children even if the path is a tree, nor descends into it
>    recursively.

This behavior pattern is very agreeable. I'll take it!

Consider your patch:

Signed-Off-By: Jason McMullan <jason.mcmullan@timesys.com>

-- 
Jason McMullan <jason.mcmullan@timesys.com>
TimeSys Corporation


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH-RFC] Rewrite ls-tree to behave more like "/bin/ls -a"

[Junio C Hamano]

>>>>> "JM" == Jason McMullan <jason.mcmullan@timesys.com> writes:

JM> On Sat, 2005-05-28 at 00:05 -0700, Junio C Hamano wrote:

>> - Unlike the old ls-tree behaviour that used paths arguments to
>> restrict output (not that it worked as intended---as pointed
>> out in the mailing list discussion, it was quite incoherent),
>> this rewrite uses paths arguments to specify what to show.
>> ...

JM> This behavior pattern is very agreeable. I'll take it!

Glad to know we are in agreement.

BTW, long after finishing the rewrite, I realized that all of
the problems you raised did not exist in the very original
version of ls-tree, but were bugs in _your_ patch that added the
paths restriction.  I was merely cleaning up your mess for you
without knowing what I was doing. ;-) Not that I do not like
what the resulting code does, though.

I am not going to re-submit this to Linus right now, since he
seems to be quiet here and spending more time and attention to
the kernel, which is what I want to see.  When Linus starts
pulling in update for GIT, and if you see this one not applied
to his tree, please remind him.