From: Jacob Keller <jacob.keller@gmail.com>
To: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
Git mailing list <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Stefan Beller <stefanbeller@gmail.com>, Jeff King <peff@peff.net>,
Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output
Date: Wed, 19 Oct 2016 15:13:49 -0700 [thread overview]
Message-ID: <CA+P7+xo65Wg+jTkGTBW87Xv8O5-FO9EWqkpinfW48jBMG_oNrQ@mail.gmail.com> (raw)
In-Reply-To: <1476908699.26043.9.camel@kaarsemaker.net>
Hi,
On Wed, Oct 19, 2016 at 1:24 PM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
> On Wed, 2016-08-31 at 16:27 -0700, Jacob Keller wrote:
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> 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.
next prev parent reply other threads:[~2016-10-19 22:14 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-31 23:27 [PATCH v12 0/8] submodule inline diff format Jacob Keller
2016-08-31 23:27 ` [PATCH v12 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
2016-08-31 23:27 ` [PATCH v12 2/8] diff.c: remove output_prefix_length field Jacob Keller
2016-08-31 23:27 ` [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output Jacob Keller
2016-10-19 20:24 ` Dennis Kaarsemaker
2016-10-19 21:04 ` [PATCH] rev-list: restore the NUL commit separator in --header mode Dennis Kaarsemaker
2016-10-19 22:15 ` Jacob Keller
2016-10-19 22:39 ` Junio C Hamano
2016-10-20 16:07 ` Keller, Jacob E
2016-10-20 18:02 ` Dennis Kaarsemaker
2016-10-19 22:41 ` Junio C Hamano
2016-10-20 18:04 ` Dennis Kaarsemaker
2016-10-20 18:12 ` Jacob Keller
2016-10-20 18:46 ` Junio C Hamano
2016-10-20 6:51 ` Torsten Bögershausen
2016-10-19 22:13 ` Jacob Keller [this message]
2016-08-31 23:27 ` [PATCH v12 4/8] diff: prepare for additional submodule formats Jacob Keller
2016-08-31 23:27 ` [PATCH v12 5/8] allow do_submodule_path to work even if submodule isn't checked out Jacob Keller
2016-08-31 23:27 ` [PATCH v12 6/8] submodule: convert show_submodule_summary to use struct object_id * Jacob Keller
2016-08-31 23:27 ` [PATCH v12 7/8] submodule: refactor show_submodule_summary with helper function Jacob Keller
2016-08-31 23:27 ` [PATCH v12 8/8] diff: teach diff to display submodule difference with an inline diff Jacob Keller
2016-08-31 23:47 ` [PATCH v12 0/8] submodule inline diff format Stefan Beller
2016-09-01 1:08 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+P7+xo65Wg+jTkGTBW87Xv8O5-FO9EWqkpinfW48jBMG_oNrQ@mail.gmail.com \
--to=jacob.keller@gmail.com \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jacob.e.keller@intel.com \
--cc=peff@peff.net \
--cc=stefanbeller@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).