Re: [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Oct 19, 2016 at 1:24 PM, Dennis Kaarsemaker
<dennis@xxxxxxxxxxxxxxx> wrote:
> On Wed, 2016-08-31 at 16:27 -0700, Jacob Keller wrote:
>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>>
>> Add an extension to git-diff and git-log (and any other graph-aware
>> displayable output) such that "--line-prefix=<string>" will print the
>> additional line-prefix on every line of output.
>
> This patch breaks git rev-list --header, also breaking gitweb.
>

Oops! Is it possible you have a test case already?

> The NUL between commits has gone missing, causing gitweb to interpret
> the output of git rev-list as one commit.
>

That is obviously not what we want!

> Sorry for not catching this earlier, I actually encountered this early
> september but thought it was caused by us running an ancient gitweb
> with a modern git. Finally managed to upgrade gitweb today, and the bug
> didn't go away. git bisect says 660e113ce is the culprit. Checking out
> 'next' and reverting this single patch makes the problem disappear.
>

Ok.

> Haven't yet tried to fix the bug, but this hunk looks suspicious:



>
> -                       if (revs->commit_format != CMIT_FMT_USERFORMAT ||
> -                           buf.len) {
> -                               fwrite(buf.buf, 1, buf.len, stdout);
> -                               putchar(info->hdr_termination);
> -                       }
> +                       /*
> +                        * If the message buffer is empty, just show
> +                        * the rest of the graph output for this
> +                        * commit.
> +                        */
> +                       if (graph_show_remainder(revs->graph))
> +                               putchar('\n');

Most likely this should have been "putchar(info->hdr_termination);" I
think? Not entirely sure.

If we can get a test case in we can use that to help debug the issue.

Thanks,
Jake

> +                       if (revs->commit_format == CMIT_FMT_ONELINE)
> +
>
> D.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]