git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: Derrick Stolee <derrickstolee@github.com>,
	git@vger.kernel.org, peff@peff.net, ps@pks.im,
	wfc@wfchandler.org
Subject: Re: [PATCH 1/3] t5318: demonstrate commit-graph generation v2 corruption
Date: Fri, 15 Jul 2022 17:01:16 -0700	[thread overview]
Message-ID: <xmqqo7xqx6wj.fsf@gitster.g> (raw)
In-Reply-To: <YtHkmb4Qwvo9n00A@nand.local> (Taylor Blau's message of "Fri, 15 Jul 2022 18:05:13 -0400")

Taylor Blau <me@ttaylorr.com> writes:

> On Thu, Jul 14, 2022 at 11:15:42PM -0400, Derrick Stolee wrote:
>> > +		# This commit will have a date at two seconds past the Epoch,
>> > +		# and a (v1) generation number of 1, since it is a root commit.
>> > +		#
>> > +		# The offset will then be computed as 2-1, which will underflow
>>
>> I have verified that your test works, but this explanation is confusing me.
>> "2 - 1" is 1, which does not underflow. There must be something else going
>> on.
>>
>> Looking ahead, you describe the situation correctly in Patch 3 to show that
>> we take "generation - date", so you really just need s/2-1/1-2/ here.
>
> Yes, absolutely. Thanks for catching it.
>
> Junio: you may want to s/2-1/1-2 in this patch's message, or I can send
> you a replacement or reroll, whatever is easier.

I've already done "rebase -i" to do so.

But for future reference, the easiest for me is if the author said,
after saying "Thanks for catching it", "Will reroll after waiting
for a bit to see if there are other comments".  That way, I only
have to edit the latest draft of "What's cooking" report to mark the
topic to be expecting a reroll (which will prevent me from merging
the topic down to 'next' prematurely by mistake) and forget about
it, until I actually see the updated set of patches.  It would be
even easier if the updated set of patches said which topic it is
meant to replace.  That way, I can trust other reviewers about the
details of the change between iterations and play a patch monkey,
when I am short of time.

It is more work to (1) look at the message you are responding to and
understand what "2 minus 1" vs "1 minus 2" being discussed is about,
and (2) go one level up in the thread to find the line in the patch
that was being discussed, and (3) run "rebase -i" and change "pick"
to "edit" and (4) find the line in the file that was affected by the
hunk in question and edit it.  The worst part of it is I'd either
have to do it right away before I forget, or mark the message unread
so that I can revisit it when I have enough time.

Either way is fine, but a straight resend with two notices (one that
says "will reroll", the other that says "this replaces topic X") is
the easiest to handle for me.

Thanks.


  reply	other threads:[~2022-07-16  0:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-12 23:10 [PATCH 0/3] commit-graph: fix corruption during generation v2 upgrade Taylor Blau
2022-07-12 23:10 ` [PATCH 1/3] t5318: demonstrate commit-graph generation v2 corruption Taylor Blau
2022-07-15  3:15   ` Derrick Stolee
2022-07-15 22:05     ` Taylor Blau
2022-07-16  0:01       ` Junio C Hamano [this message]
2022-07-16  0:17         ` Taylor Blau
2022-07-12 23:10 ` [PATCH 2/3] commit-graph: introduce `repo_find_commit_pos_in_graph()` Taylor Blau
2022-07-15  3:17   ` Derrick Stolee
2022-07-12 23:10 ` [PATCH 3/3] commit-graph: fix corrupt upgrade from generation v1 to v2 Taylor Blau
2022-07-13 17:41 ` [PATCH 0/3] commit-graph: fix corruption during generation v2 upgrade Junio C Hamano
2022-07-15  2:02   ` Taylor Blau
2022-07-15  3:20 ` Derrick Stolee

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=xmqqo7xqx6wj.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=wfc@wfchandler.org \
    /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).