Re: [RFC PATCH] ls-tree: `-l` should not imply recursive listing

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 6, 2022, 15:41 UTC
Message-ID: <xmqq8rsi1ap4.fsf@gitster.g>
In-Reply-To: <9ce4dadf140204e934f7025bb91385c376118940.1649111831.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

Show 15 quoted lines

> In 9c4d58ff2c (ls-tree: split up "fast path" callbacks, 2022-03-23), a
> refactoring of the various read_tree_at() callbacks caused us to
> unconditionally recurse into directories if `-l` (long format) was
> passed on the command line, regardless of whether or not we also pass
> the `-r` (recursive) flag.
>
> Fix this by making show_tree_long() return the value of `recurse`,
> rather than always returning 1. This value is interpreted by
> read_tree_at() to be a signal on whether or not to recurse.
>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
> 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.

It's rather an obvious bug and it is sad that existing tests did not find (and no, I do not think lower-level unit testing would have helped us in any way, either).

Will queue.

Thanks for a quick fix.

Show 18 quoted lines

>  builtin/ls-tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index 5dac9ee5b9..e279be8bb6 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,
>
> base-commit: faa21c10d44184f616d391c158dcbb13b9c72ef3