Re: Process for cherry-picking patches from other projects

[Julien Grall <julien@xxxxxxx>]

On 16/05/2022 16:04, Andrew Cooper wrote:
> On 13/05/2022 15:33, George Dunlap wrote:
> > Starting a new thread to make it clear that we’re discussing a wider policy 
> > here.
> >
> > This question is aimed at Jan and Andy in particular, as I think they’ve probably done the most of 
> > this; so I’m looking to them to find out what our “standard practice” is.
> >
> > There have recently been some patches that Bertrand has submitted which pull in code from 
> > Linux ("[PATCH 1/3] xen/arm: Sync sysregs and cpuinfo with Linux 5.18-rc3”), 
> > which has caused a discussion between him, Julien, and Stefano about the proper way to do 
> > such patches.
> >
> > The “Origin:” tag section of xen.git/docs/process/sending-patches.pandoc suggests that 
> > there are some standards, but doesn’t spell them out.
> >
> > The questions seem to be:
> >
> > 1) When doing this kind of update, is it permissible to send a single patch which 
> > “batches” several upstream commits together,
> Yes, absolutely.
>
> We do this all over the place.
>
> >   or should each patch be backported individually?
> >
> > 2) If “batches” are permissible, when?  When would individual patches be 
> > preferred?
> That's a matter of taste.  If it's several patches of a complicated
> bugfix, then it probably wants splitting up in the same way.
>
> If it's a bunch of misc changes, then batching is fine.
>
>
> > 3) For “batch updates”, what tags are necessary?  Do we need to note the changesets of all the commits, and 
> > if so, do we need multiple “Origin” tags?  Do we need to include anything from the original commits 
> > — commit messages?  Signed-off-by’s?
> "Update $FOO to something resembling $PROJECT, $VERSION" is perfectly good.
>
> > And a related question:
> >
> > 4) When importing an entire file from an upstream like Linux, what tags do we 
> > need?
> Any clear reference to where it came from.
>
> Nothing is ever imported verbatim.  If nothing else, paths have to be
> changed, and usually more than that.
>
> Given that, I do question whether it is appropriate to retain original
> authorship.  The original author did not write a patch for Xen, and what
> gets committed wasn't the patch they wrote.
>
> Any issues with the port into Xen should be sent to the person who did
> the port into Xen, not the original author who most likely has no idea
> that their patch has been borrowed by Xen.
I agree with that. But I don't view the "Author" line as a way to 
achieve it.
Even for Xen, it is possible that the patch was written by A but then 
fully upstreamed by B (they may be different company). In which case, 
the practice so far has been to use A as the author and add a 2nd 
Signed-off-by for B.
I view porting a patch from Linux the same. If the changes are minor, 
the original author should be credited.
> IMO, a commit message saying "port $X from project $Y" makes it crystal
> clear that the original code change isn't mine, but the porting effort
> is.  Amongst other things, porting invalidates any review/ack/test chain
> because those tags were given in the context of the original project,
> not Xen.
This seems to contradict our documentation:

"All tags **above** the `Origin:` tag are from the original patch (which
should all be kept), while tags **after** `Origin:` are related to the
normal Xen patch process as described here."

Cheers,

--
Julien Grall