git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] ls-tree: fix --long implying -r regression in 9c4d58ff2c3

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 4, 2022, 23:45 UTC
Message-ID
<patch-v2-1.1-ed83b3b74ab-20220404T234507Z-avarab@gmail.com>
In-Reply-To
<9ce4dadf140204e934f7025bb91385c376118940.1649111831.git.steadmon@google.com>

Fix a regression introduced in 9c4d58ff2c3 (ls-tree: split up "fast path" callbacks, 2022-03-23), and improve the tests added in the 1041d58b4d9 (Merge branch 'tl/ls-tree-oid-only', 2022-04-04) topic it was merged as part of to test the full expected output of various "ls-tree" options with and without -r.

Let's fix it, and also add tests not only for that blindspot, but also any other potential blindspots. To that end test the "modes" of -d, -r and -t (as well as "no mode") against all of the format options.

These tests all pass with that topic reverted (except those that would fail because they're testing the new --object-only feature introduced in that topic), which should give us confidence that there were no further regressions in this area.

Reported-By: Josh Steadmon <steadmon@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
On Mon, Apr 04 2022, Josh Steadmon wrote:
> I believe this is the correct fix for the change in `git ls-tree -l`
> output. I would also like to add tests in a future fix, but I do not
> have time to add them today.
Indeed. I guess that makes this a proposed v2,

I refreshed my E-Mail when I was just about to submit this and spotted that you'd sent your fix in, but I came up with this (in retrospect a pretty obvious think-o) fix independently, sorry about the bug.

The tests took me a bit longer though...

Haing written them I guess we could do them post-release, since the important thing is to validate the changes. As noted in the commit message we're now testing all combinations of the "mode" and "format" options.

 builtin/ls-tree.c         |   2 +-
 t/t3104-ls-tree-format.sh | 126 +++++++++++++++++++++++++++++++++++---
 2 files changed, 119 insertions(+), 9 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 5dac9ee5b9d..e279be8bb63 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -255,7 +255,7 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 	printf("%06o %s %s %7s\t", data.mode, type_name(data.type),
 	       find_unique_abbrev(data.oid, abbrev), size_text);
 	show_tree_common_default_long(base, pathname, data.base->len);
-	return 1;
+	return recurse;
 }
 
 static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
diff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh
index 0769a933d69..520b5a95c08 100755
--- a/t/t3104-ls-tree-format.sh
+++ b/t/t3104-ls-tree-format.sh
@@ -23,6 +23,19 @@ test_ls_tree_format () {
 	fmtopts=$3 &&
 	shift 2 &&
 
+	cat >expect &&
+	cat <&6 >expect.-d &&
+	cat <&7 >expect.-r &&
+	cat <&8 >expect.-t &&
+
+	for opt in '' '-d' '-r' '-t'
+	do
+		test_expect_success "'ls-tree $opts${opt:+ $opt}' output" '
+			git ls-tree ${opt:+$opt }$opts $opt HEAD >actual &&
+			test_cmp expect${opt:+.$opt} actual
+		'
+	done
+
 	test_expect_success "ls-tree '--format=<$format>' is like options '$opts $fmtopts'" '
 		git ls-tree $opts -r HEAD >expect &&
 		git ls-tree --format="$format" -r $fmtopts HEAD >actual &&
@@ -39,38 +52,135 @@ test_ls_tree_format () {
 
 test_ls_tree_format \
 	"%(objectmode) %(objecttype) %(objectname)%x09%(path)" \
-	""
+	"" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	OUT_D
+	100644 blob $(git rev-parse HEAD:dir/sub-file.t)	dir/sub-file.t
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_R
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)" \
-	"--long"
+	"--long" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	040000 tree $(git rev-parse HEAD:dir)       -	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)       9	top-file.t
+	OUT
+	040000 tree $(git rev-parse HEAD:dir)       -	dir
+	OUT_D
+	100644 blob $(git rev-parse HEAD:dir/sub-file.t)      13	dir/sub-file.t
+	100644 blob $(git rev-parse HEAD:top-file.t)       9	top-file.t
+	OUT_R
+	040000 tree $(git rev-parse HEAD:dir)       -	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)       9	top-file.t
+	OUT_T
 
 test_ls_tree_format \
 	"%(path)" \
-	"--name-only"
+	"--name-only" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	dir
+	top-file.t
+	OUT
+	dir
+	OUT_D
+	dir/sub-file.t
+	top-file.t
+	OUT_R
+	dir
+	top-file.t
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectname)" \
-	"--object-only"
+	"--object-only" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	$(git rev-parse HEAD:dir)
+	$(git rev-parse HEAD:top-file.t)
+	OUT
+	$(git rev-parse HEAD:dir)
+	OUT_D
+	$(git rev-parse HEAD:dir/sub-file.t)
+	$(git rev-parse HEAD:top-file.t)
+	OUT_R
+	$(git rev-parse HEAD:dir)
+	$(git rev-parse HEAD:top-file.t)
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectname)" \
 	"--object-only --abbrev" \
-	"--abbrev"
+	"--abbrev" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	$(git rev-parse HEAD:dir | test_copy_bytes 7)
+	$(git rev-parse HEAD:top-file.t| test_copy_bytes 7)
+	OUT
+	$(git rev-parse HEAD:dir | test_copy_bytes 7)
+	OUT_D
+	$(git rev-parse HEAD:dir/sub-file.t | test_copy_bytes 7)
+	$(git rev-parse HEAD:top-file.t | test_copy_bytes 7)
+	OUT_R
+	$(git rev-parse HEAD:dir | test_copy_bytes 7)
+	$(git rev-parse HEAD:top-file.t | test_copy_bytes 7)
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectmode) %(objecttype) %(objectname)%x09%(path)" \
 	"-t" \
-	"-t"
+	"-t" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	OUT_D
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:dir/sub-file.t)	dir/sub-file.t
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_R
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectmode) %(objecttype) %(objectname)%x09%(path)" \
 	"--full-name" \
-	"--full-name"
+	"--full-name" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	OUT_D
+	100644 blob $(git rev-parse HEAD:dir/sub-file.t)	dir/sub-file.t
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_R
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_T
 
 test_ls_tree_format \
 	"%(objectmode) %(objecttype) %(objectname)%x09%(path)" \
 	"--full-tree" \
-	"--full-tree"
+	"--full-tree" \
+	<<-OUT 6<<-OUT_D 7<<-OUT_R 8<<-OUT_T
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	OUT_D
+	100644 blob $(git rev-parse HEAD:dir/sub-file.t)	dir/sub-file.t
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_R
+	040000 tree $(git rev-parse HEAD:dir)	dir
+	100644 blob $(git rev-parse HEAD:top-file.t)	top-file.t
+	OUT_T
 
 test_done

-- 
2.35.1.1604.g35dc6517170
Previous: Josh SteadmonNext: Junio C Hamano
Message 85 of 107 in “ls-tree: "--object-only" and "--format" opts”
  1. 00/13 ls-tree: "--object-only" and "--format" optsTeng Long, Feb 8, 2022
  2. 01/13 ls-tree: remove commented-out codeTeng Long, Feb 8, 2022
  3. 03/13 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Feb 8, 2022
  4. 12/13 ls-tree: introduce function "fast_path()"Teng Long, Feb 8, 2022
  5. Ævar Arnfjörð BjarmasonFeb 19, 2022
  6. 13/13 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Feb 8, 2022
  7. Ævar Arnfjörð BjarmasonFeb 19, 2022
  8. 04/13 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Feb 8, 2022
  9. 02/13 ls-tree: add missing braces to "else" armsTeng Long, Feb 8, 2022
  10. 06/13 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Feb 8, 2022
  11. Ævar Arnfjörð BjarmasonFeb 19, 2022
  12. 05/13 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Feb 8, 2022
  13. 07/13 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Feb 8, 2022
  14. Ævar Arnfjörð BjarmasonFeb 19, 2022
  15. 08/13 ls-tree: slightly refactor `show_tree()`Teng Long, Feb 8, 2022
  16. Ævar Arnfjörð BjarmasonFeb 19, 2022
  17. 10/13 cocci: allow padding with `strbuf_addf()`Teng Long, Feb 8, 2022
  18. 09/13 ls-tree: introduce struct "show_tree_data"Teng Long, Feb 8, 2022
  19. 11/13 ls-tree.c: introduce "--format" optionTeng Long, Feb 8, 2022
  20. Ævar Arnfjörð BjarmasonFeb 19, 2022
  21. 00/12 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 4, 2022
  22. 01/12 ls-tree: remove commented-out codeTeng Long, Mar 4, 2022
  23. 02/12 ls-tree: add missing braces to "else" armsTeng Long, Mar 4, 2022
  24. 03/12 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 4, 2022
  25. 04/12 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 4, 2022
  26. 05/12 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 4, 2022
  27. 06/12 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 4, 2022
  28. 07/12 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 4, 2022
  29. 08/12 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 4, 2022
  30. 09/12 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 4, 2022
  31. 10/12 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 4, 2022
  32. 11/12 ls-tree: introduce "--format" optionTeng Long, Mar 4, 2022
  33. 12/12 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 4, 2022
  34. 0/7 fixups/suggestions/musings for tl/ls-tree-oid-onlyÆvar Arnfjörð Bjarmason, Mar 10, 2022
  35. 1/7 ls-tree tests: add tests for --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  36. 2/7 ls-tree tests: exhaustively test fast & slow path for --formatÆvar Arnfjörð Bjarmason, Mar 10, 2022
  37. 5/7 ls-tree: detect and error on --name-only --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  38. 4/7 ls-tree: remove unused "MODE_UNSPECIFIED"Ævar Arnfjörð Bjarmason, Mar 10, 2022
  39. 3/7 ls-tree: remove dead labelsÆvar Arnfjörð Bjarmason, Mar 10, 2022
  40. 6/7 ls-tree: remove FIELD_*, just use MODE_*Ævar Arnfjörð Bjarmason, Mar 10, 2022
  41. 7/7 ls-tree: split up "fast path" callbacksÆvar Arnfjörð Bjarmason, Mar 10, 2022
  42. Teng LongMar 17, 2022
  43. Ævar Arnfjörð BjarmasonMar 17, 2022
  44. 00/16 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 21, 2022
  45. 01/16 ls-tree: remove commented-out codeTeng Long, Mar 21, 2022
  46. 02/16 ls-tree: add missing braces to "else" armsTeng Long, Mar 21, 2022
  47. 03/16 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 21, 2022
  48. 04/16 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 21, 2022
  49. 05/16 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 21, 2022
  50. 06/16 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 21, 2022
  51. 07/16 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 21, 2022
  52. 08/16 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 21, 2022
  53. 09/16 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 21, 2022
  54. 10/16 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 21, 2022
  55. 11/16 ls-tree: introduce "--format" optionTeng Long, Mar 21, 2022
  56. Ævar Arnfjörð BjarmasonMar 21, 2022
  57. 12/16 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 21, 2022
  58. 13/16 ls-tree tests: add tests for --name-statusTeng Long, Mar 21, 2022
  59. Ævar Arnfjörð BjarmasonMar 21, 2022
  60. 14/16 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 21, 2022
  61. 15/16 ls-tree: remove FIELD_*, just use MODE_*Teng Long, Mar 21, 2022
  62. Ævar Arnfjörð BjarmasonMar 21, 2022
  63. 16/16 ls-tree: split up "fast path" callbacksTeng Long, Mar 21, 2022
  64. Ævar Arnfjörð BjarmasonMar 21, 2022
  65. Teng LongMar 23, 2022
  66. Junio C HamanoMar 21, 2022
  67. 00/15 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 23, 2022
  68. 01/15 ls-tree tests: add tests for --name-statusTeng Long, Mar 23, 2022
  69. 02/15 ls-tree: remove commented-out codeTeng Long, Mar 23, 2022
  70. 03/15 ls-tree: add missing braces to "else" armsTeng Long, Mar 23, 2022
  71. 04/15 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 23, 2022
  72. 05/15 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 23, 2022
  73. 06/15 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 23, 2022
  74. 07/15 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 23, 2022
  75. 08/15 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 23, 2022
  76. 09/15 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 23, 2022
  77. 10/15 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 23, 2022
  78. 11/15 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 23, 2022
  79. 12/15 ls-tree: introduce "--format" optionTeng Long, Mar 23, 2022
  80. 13/15 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 23, 2022
  81. 14/15 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 23, 2022
  82. 15/15 ls-tree: split up "fast path" callbacksTeng Long, Mar 23, 2022
  83. Josh SteadmonApr 4, 2022
  84. ls-tree: `-l` should not imply recursive listingJosh Steadmon, Apr 4, 2022
  85. ls-tree: fix --long implying -r regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Apr 4, 2022
  86. Junio C HamanoApr 6, 2022
  87. Ævar Arnfjörð BjarmasonApr 6, 2022
  88. Junio C HamanoApr 6, 2022
  89. Ævar Arnfjörð BjarmasonApr 7, 2022
  90. Junio C HamanoApr 7, 2022
  91. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, May 31, 2022
  92. Johannes SchindelinJun 2, 2022
  93. Junio C HamanoJun 2, 2022
  94. js/ci-github-workflow-markup output regression (was: [PATCH] ls-tree: test for the regression in 9c4d58ff2c3)Ævar Arnfjörð Bjarmason, Jun 3, 2022
  95. Junio C HamanoJun 3, 2022
  96. Ævar Arnfjörð BjarmasonJun 3, 2022
  97. Junio C HamanoJun 7, 2022
  98. Ævar Arnfjörð BjarmasonJun 7, 2022
  99. Johannes SchindelinJun 8, 2022
  100. Ævar Arnfjörð BjarmasonJun 9, 2022
  101. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Jun 3, 2022
  102. Johannes SchindelinJun 8, 2022
  103. Teng LongApr 7, 2022
  104. Junio C HamanoApr 6, 2022
  105. Junio C HamanoMar 23, 2022
  106. Teng LongMar 24, 2022
  107. Ævar Arnfjörð BjarmasonFeb 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.