Re: Incorrect Author on patch

[Laszlo Ersek <lersek@redhat.com>]

On 05/02/18 14:46, Evan Lloyd wrote:
> One obvious way of precluding this sort of problem would be to move to
> using the pull request mechanism on GitHub, rather than requiring
> maintainers to play with upstreaming local patches.

In my opinion:
- *git* pull requests: yes,
- *github* pull requests: please no.

We discussed this a few years (?) ago, and I believe the outcome was
that pulling remote branches was fine, as long as:

(a) the location to pull from was noted in the cover letter of a patch
    series, or in a separate pull request emailed to the list, and

(b) the maintainer pulling the remote branch verified that the patches
    reviewed on the list were formatted from the commits on the branch
    being pulled.

The remote branch being pulled may well reside on github, but the review
must occur on the list (not on the GitHub web UI), and the actual fetch
& merge actions must be carried out by a maintainer locally in their
work environment, from where they are supposed to push the new commits
to the central repo. In other words, we must not start relying on GitHub
tools for either patch reviews or for generating merge commits (or any
other kinds of commits).

>  I can well understand why it would be useful to use Gerrit as a means
>  of reviewing a patch - actually a brilliant idea,

(actually, *not* a brilliant idea, but that's just my opinion :) )

> but it does introduce problems. If, post review, the merge were  to be
> from a pull request then there would be no risk of "meta-data
> corruption".

Agreed 100% about "no risk of meta-data corruption".

However there is a new risk, namely that of getting unreviewed code into
the tree. See requirement (b). And I can imagine that such verification
is more trouble for some maintainers than simply applying the patches
from the list.

Basically, the way I would do it, is:
- identify the branch fork-off point from the pull request,
- create a local branch at that point,
- apply the patches from the mailing list to the local branch,
- fetch the remote branch,
- format the patches from the local branch,
- format the patches from the remote branch,
- compare the two sets of locally-formatted patches,
- eyeball the results.

Local reformatting of the patches applied from the mailing list is
necessary because formatting options (such as context size, file order,
diff algorithm, subject prefix) affect the comparison. And, simply
git-diffing the commits on the parallel branches isn't sufficient,
because it doesn't compare authorship or even commit messages.

In other projects, pulling from someone basically means letting that
someone commit to the project unsupervised. So it's a position of trust.
Hence signed pull requests. And those people that submit the pull
requests, they certainly need to apply the "leaf" patches from mailing
lists, from other contributors, after review.

All in all, I don't think there's a way around applying patches intact
from the mailing list. Whatever workflow we choose, that action will be
part of that workflow.

Thanks
Laszlo