{"thread":{"id":"22569","subject":"[RFC PATCH] ls-tree: dump full tree if it was named as such","startedAt":"2010-02-08T15:59:45Z","lastAt":"2010-02-15T01:47:16Z","messageCount":3,"participants":["Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"133928","messageId":"f3e853a9329a68cc9b1d544b204ed6a0469b10a2.1265644609.git.trast@student.ethz.ch","threadId":"22569","inReplyTo":null,"subject":"[RFC PATCH] ls-tree: dump full tree if it was named as such","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-02-08T15:59:45Z","receivedAt":"2010-02-08T15:59:45Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"git-ls-tree acted weird when it was called in a subdirectory but with\na tree (not commit) argument.  For example,\n\n  # in git.git\n  mkdir valgrind\n  cd valgrind\n  git ls-tree HEAD:t\n\nwould list the contents of HEAD:t/valgrind even though the user is\nlooking at /valgrind.\n\nFix this by not doing the implied-prefix logic when the specified\nobject is a tree (as in 'HEAD:t' or 'HEAD:'), as opposed to a commit\n('HEAD').\n\nTo this end, we first introduce a variant of parse_tree_indirect()\nthat also reports back how many levels it had to traverse.  Then we\nteach cmd_ls_tree() to ignore the prefix if that depth was 0.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nRFC because I'm breaking an existing test, though I can't see why\nanyone would rely on such weird behaviour.\n\n\n builtin-ls-tree.c          |    9 +++++++--\n t/t3101-ls-tree-dirname.sh |   19 ++++++++++++++++++-\n tree.c                     |   11 ++++++++++-\n tree.h                     |    1 +\n 4 files changed, 36 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-ls-tree.c b/builtin-ls-tree.c\nindex 4484185..04c6a65 100644\n--- a/builtin-ls-tree.c\n+++ b/builtin-ls-tree.c\n@@ -121,6 +121,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tunsigned char sha1[20];\n \tstruct tree *tree;\n \tint full_tree = 0;\n+\tint depth;\n \tconst struct option ls_tree_options[] = {\n \t\tOPT_BIT('d', NULL, &ls_options, \"only show trees\",\n \t\t\tLS_TREE_ONLY),\n@@ -166,10 +167,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tif (get_sha1(argv[0], sha1))\n \t\tdie(\"Not a valid object name %s\", argv[0]);\n \n-\tpathspec = get_pathspec(prefix, argv + 1);\n-\ttree = parse_tree_indirect(sha1);\n+\ttree = parse_tree_indirect_depth(sha1, &depth);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\n+\tif (!depth) {\n+\t\tls_tree_prefix = prefix = NULL;\n+\t\tchomp_prefix = 0;\n+\t}\n+\tpathspec = get_pathspec(prefix, argv + 1);\n \tread_tree_recursive(tree, \"\", 0, 0, pathspec, show_tree, NULL);\n \n \treturn 0;\ndiff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh\nindex 06654c6..a3cd80a 100755\n--- a/t/t3101-ls-tree-dirname.sh\n+++ b/t/t3101-ls-tree-dirname.sh\n@@ -37,6 +37,7 @@ test_expect_success \\\n      find *.txt path* \\( -type f -o -type l \\) -print |\n      xargs git update-index --add &&\n      tree=`git write-tree` &&\n+     commit=`echo dummy | git commit-tree $tree` &&\n      echo $tree'\n \n test_output () {\n@@ -142,7 +143,7 @@ test_expect_success 'ls-tree filter is leading path match' '\n test_expect_success 'ls-tree --full-name' '\n \t(\n \t\tcd path0 &&\n-\t\tgit ls-tree --full-name $tree a\n+\t\tgit ls-tree --full-name $commit a\n \t) >current &&\n \tcat >expected <<\\EOF &&\n 040000 tree X\tpath0/a\n@@ -224,4 +225,20 @@ EOF\n \ttest_output\n '\n \n+test_expect_success 'ls-tree $tree does not restrict to pwd' '\n+\t(\n+\t\tcd path0 &&\n+\t\tgit ls-tree $tree\n+\t) >current &&\n+\tcat >expected <<\\EOF &&\n+100644 blob X\t1.txt\n+100644 blob X\t2.txt\n+040000 tree X\tpath0\n+040000 tree X\tpath1\n+040000 tree X\tpath2\n+040000 tree X\tpath3\n+EOF\n+\ttest_output\n+'\n+\n test_done\ndiff --git a/tree.c b/tree.c\nindex 5ab90af..e9e8481 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -266,9 +266,11 @@ int parse_tree(struct tree *item)\n \treturn parse_tree_buffer(item, buffer, size);\n }\n \n-struct tree *parse_tree_indirect(const unsigned char *sha1)\n+struct tree *parse_tree_indirect_depth(const unsigned char *sha1,\n+\t\t\t\t       int *depth)\n {\n \tstruct object *obj = parse_object(sha1);\n+\t*depth = 0;\n \tdo {\n \t\tif (!obj)\n \t\t\treturn NULL;\n@@ -282,5 +284,12 @@ struct tree *parse_tree_indirect(const unsigned char *sha1)\n \t\t\treturn NULL;\n \t\tif (!obj->parsed)\n \t\t\tparse_object(obj->sha1);\n+\t\t(*depth)++;\n \t} while (1);\n }\n+\n+struct tree *parse_tree_indirect(const unsigned char *sha1)\n+{\n+\tint unused;\n+\treturn parse_tree_indirect_depth(sha1, &unused);\n+}\ndiff --git a/tree.h b/tree.h\nindex 2ff01a4..97f171c 100644\n--- a/tree.h\n+++ b/tree.h\n@@ -18,6 +18,7 @@ struct tree {\n int parse_tree(struct tree *tree);\n \n /* Parses and returns the tree in the given ent, chasing tags and commits. */\n+struct tree *parse_tree_indirect_depth(const unsigned char *sha1, int *depth);\n struct tree *parse_tree_indirect(const unsigned char *sha1);\n \n #define READ_TREE_RECURSIVE 1\n-- \n1.7.0.rc2.178.g109e1\n"},{"id":"134556","messageId":"201002141733.53137.trast@student.ethz.ch","threadId":"22569","inReplyTo":"7vpr4fo85r.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] ls-tree: dump full tree if it was named as such","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-02-14T16:33:52Z","receivedAt":"2010-02-14T16:33:52Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"[I noticed only just now that you only sent this to me.  Accidentally\nI suppose?]\n\nOn Monday 08 February 2010 20:10:40 Junio C Hamano wrote:\n> \n> This obviously was meant to be used with the full tree recorded by a\n> commit and what you are seeing (e.g. cd to \"valgrind\" that is not even\n> tracked, and pretend HEAD:t were the full contents---the full contents of\n> the tree limited to the work tree location \"valgrind\" is shown) is an\n> artifact of that.\n> \n> I think the right solution is along the lines of --full-tree option to\n> allow people (i.e. scripts) to ask for the exact tree contents without any\n> funny path limiting based on the location in the work tree.  They can\n> apply whatever path limiting from the command line, e.g. by running\n> \n>     git ls-tree --full-tree HEAD:t valgrind\n> \n> instead of running\n> \n>     mkdir -p valgrind && cd valgrind && git ls-tree HEAD:t\n> \n> when they want to apply path limit to the ls-tree output.\n\nI guess in the (very) long run, the scripts should be forced to always\nuse --full-tree so that we can eventually make it the default?\n\nI'm just not sure how the existing behaviour could ever be useful,\nthough admittedly 'git ls-tree $(git write-tree)' would change\nsemantics if you're in a subdirectory.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"134589","messageId":"7vy6iv46e3.fsf@alter.siamese.dyndns.org","threadId":"22569","inReplyTo":"201002141733.53137.trast@student.ethz.ch","subject":"Re: [RFC PATCH] ls-tree: dump full tree if it was named as such","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-15T01:47:16Z","receivedAt":"2010-02-15T01:47:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> I guess in the (very) long run, the scripts should be forced to always\n> use --full-tree so that we can eventually make it the default?\n>\n> I'm just not sure how the existing behaviour could ever be useful,\n> though admittedly 'git ls-tree $(git write-tree)' would change\n> semantics if you're in a subdirectory.\n\nHysterical raisins.  I am reasonably sure that it behaves for end-user\nconvenience and consistency while running these from the command line:\n\n    $ cd t\n    $ git ls-files\n    $ git ls-tree HEAD\n\nThe --full-tree option came as an afterthought much later to help\nPorcelain scripts, that had to cd-up-to-top to get to the full tree\ncontents.\n\nThe order of events and historical backstory does not matter.\n\nThe Porcelain scripts have learned to either (1) going to top before\nrunning, or (2) giving --full-tree when they want to see a full tree\noutput, but at the same time they also learned that (3) running the\ncommand without --full-tree will give them the contents of the tree that\ncorresponds to where the current directory is in the work tree.\n\nAll three are documented plumbing interfaces---not just the first two\npoints.  I haven't seen any good excuse to break the third promise with\nPorcelain scripts. I didn't find one myself when I did d4789c6 (ls-tree:\nadd --full-tree option, 2008-12-25) long time ago.\n\nSo even in \"(very) long run\" nothing will happen.  I was about to write\n\"except documenting them better, perhaps\", but after re-reading the\nintroductory paragraphs of the manual page, I don't think anything more\nneeds to be said there.\n"}]}