Re: [PATCH 4/4] xdiff/xmerge: fix chg0 initialization

[Elijah Newren <newren@gmail.com>]

On Tue, Jun 15, 2021 at 12:51 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Elijah Newren wrote:
> > On Sun, Jun 13, 2021 at 4:04 PM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> > >
...
> > I'm somewhat worried by the cavalier posting of the zdiff3 patch[1],
>
> When Uwe sent the patch [1] nobody said it was a "cavalier posting".
> Jeff King said "I think the patch is correct".

The difference here is that Jeff reported segfaults after Uwe sent the
patch.  Uwe could not have read those reports before he posted the
patch.  You did read those emails before re-posting the patch.

> > Peff already pointed out that you reposted the zdiff3 patch that had
> > knonw segfaults without even stating as much[3].
>
> He knew that. I didn't.

You knew he had reported it as a possibility.  That behooved you to
try to investigate, either by asking him for details to try to
reproduce, or attempt it on a large range of merges, and then to
report the results when you reposted the patch.  Or, at the absolute
minimum, to at least report Peff's report along with your reposting of
the patch.

> > and that you would have included multiple new testcases to the
> > testsuite both to make sure we don't cause any obvious bugs and to
> > provide some samples of how the option functions,
>
> It is not uncommon for v1 of a patch series to be missing something.
> Uwe's patch series [1] did not include tests either, and nobody focused
> on that. It is completely reasonable to assume that a reboot of the
> series wouldn't initially focus on that either.

Not when folks spent several emails in response to the original about
the finer points of how splitting should or should not work.  Nor when
folks had reported segfaults with the original patch.  With that
background, I'd say it's completely unreasonable to assume that a
reboot of the series comes without any tests unless marked as RFC.

> > You didn't do any of that, making me wonder whether this patch is a
> > solid fix, or whether it represents just hacking around the first edge
> > case you ran into and posting a shot in the dark.  It could well be
> > either; how are we to know whether we should trust these patches?
>
> By assuming good faith [2], and simply asking questions. I don't think
> assuming the worst and calling into question the competence of a
> contributor is a good approach.

I did not assume the worst with your patches.  I assumed good faith
until I was provided with strong counter-evidence as perhaps best
summarized at https://lore.kernel.org/git/YMbexfeUG78yBix4@coredump.intra.peff.net/.
That made it clear something was heavily problematic.  If you hadn't
reposted patches for which there were reported segfaults without
saying anything about those reports, I would have asked you none of
those pointed questions.  In fact, I probably wouldn't have asked them
if your original response to Peff were along the lines of "sorry,
won't do that again" rather than "I don't know how I was supposed to
investigate the few segfaults you mentioned. All you said is that you
never tracked the bug."  It was precisely that cavalier attitude that
made me question this patch in this manner.

> If you want to deride the hours I
> spent working for the community for free which resulted in a patch that
> most likely is a net positive, feel free, I think this doesn't help the
> community, which needs more of this kind of work, not less.

I was not deriding your work.  I was asking deep and pointed questions
due to the cavalier manner in which you were posting, and which you
seem to be doubling down on.  I would say that after
https://lore.kernel.org/git/YMhx2BFlwUxZ2aFJ@coredump.intra.peff.net/,
I'd have to agree with Peff that you have displayed a level of
carelessness with which I am not comfortable for the project.  That
kind of carelessness leads to skepticism in others, moreso when you
double down on it.  It makes me think that if I review any of your
future patches, I need to check them far more rigorously because you
are willing to eschew what I view as even the most basic of
responsibilities in ensuring you are submitting valid patches, and
even worse, you are unwilling to change how you approach the project
even when those are pointed out to you.