Re: [PATCH] commit: drop uses of get_cached_commit_buffer()

[Derrick Stolee <stolee@gmail.com>]

On 2/21/2018 2:17 PM, Derrick Stolee wrote:
> The get_cached_commit_buffer() method provides access to the buffer
> loaded for a struct commit, if it was ever loadead and was not freed.
>
> Two places use this to inform how to output information about commits.
>
> log-tree.c uses this method to short-circuit the output of commit
> information when the buffer is not cached. However, this leads to
> incorrect output in 'git log --oneline' where the short-OID is written
> but then the rest of the commit information is dropped and the next
> commit is written on the same line.
>
> rev-list uses this method for two reasons:
>
> - First, if the revision walk visits a commit twice, the buffer was
>    freed by rev-list in the first write. The output then does not
>    match the format expectations, since the OID is written without the
>    rest of the content.
>
> - Second, if the revision walk visits a commit that was marked
>    UNINTERESTING, the walk may never load a buffer and hence rev-list
>    will not output the verbose information.
>
> These behaviors are undocumented, untested, and unlikely to be
> expected by users or other software attempting to parse this output.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>

This would be a good time to allow multiple authors, or to just change 
the author, since this is exactly the diff you (Peff) provided in an 
earlier email. The commit message hopefully summarizes our discussion, 
but I welcome edits.

> ---
>   builtin/rev-list.c | 2 +-
>   log-tree.c         | 3 ---
>   2 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 48300d9..d320b6f 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -134,7 +134,7 @@ static void show_commit(struct commit *commit, void *data)
>   	else
>   		putchar('\n');
>   
> -	if (revs->verbose_header && get_cached_commit_buffer(commit, NULL)) {
> +	if (revs->verbose_header) {
>   		struct strbuf buf = STRBUF_INIT;
>   		struct pretty_print_context ctx = {0};
>   		ctx.abbrev = revs->abbrev;
> diff --git a/log-tree.c b/log-tree.c
> index fc0cc0d..22b2fb6 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -659,9 +659,6 @@ void show_log(struct rev_info *opt)
>   		show_mergetag(opt, commit);
>   	}
>   
> -	if (!get_cached_commit_buffer(commit, NULL))
> -		return;
> -
>   	if (opt->show_notes) {
>   		int raw;
>   		struct strbuf notebuf = STRBUF_INIT;