Re: Gerrit status update

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-11-22 7:25 p.m., Tom Tromey wrote:
> Simon> A newer version of Patchwork includes a git hook that can supposedly
> Simon> automatically close patches when they are pushed to the git repository:
> Simon>   https://patchwork.readthedocs.io/en/latest/deployment/installation/#optional-configure-your-vcs-to-automatically-update-patches
> ...
> Simon> I am just a bit skeptical about how well (if it does it at all)
> Simon> Patchwork handles patches or series versions.  If I send a v1 series
> Simon> with 15 patches, then a v2 with 17 patches, including a few renamed
> Simon> subjects, how does it cope with that?
> 
> I'm not too worried about this scenario (see below) but I think one to
> test is if a patch is abandoned from the middle of a series.  Making
> this work requires some notion of a series in the first place.  I think
> the new Patchwork has this, but it's been a while since I looked.

Yes, it does.  We could give series an id of their own (Series-Id?) and
mark all the patches from v1 as obsolete when v2 is sent.  I'm not sure if
and how that Series-Id could be automatically generated and included in
the cover letter of subsequent versions, but that's just a technical detail.

I'm not too worried about this scenario in the first place.  If you send a
lone patch and decide to abandon it, you would need to go to Patchwork to
mark it as abandonned.  If you drop a patch from a series, it's the same.

> 
> Simon> Gerrit is able to track the multiple versions of a change because we
> Simon> insert a unique Change-Id in the commit log of each patch.  This is very
> Simon> reliable, and it would make handling of new versions/merged patches very
> Simon> accurate.  But it goes against the Patchwork design:
> 
> Simon>   https://patchwork.readthedocs.io/en/latest/usage/design/
> 
> Simon>   "Don’t pollute the project’s changelogs with Patchwork poop
> 
> Simon>    A project’s changelogs are valuable - we don’t want to add Patchwork-specific metadata."
> 
> Simon> So unfortunately, we won't get that from upstream.  But it would always
> Simon> be possible to modify Patchwork to add this feature for our own
> Simon> enjoyment.
> 
> Yeah... I think this bit of Patchwork dogma is simply mistaken.
> Reliability requires a patch ID that does not vary; and since git
> doesn't provide this, it has to be provided externally.

I reached out to Patchwork devs, and it seems like they would be open to
the idea of supporting tracking patches using a Change-Id:

  https://github.com/getpatchwork/patchwork/issues/327#issuecomment-561573729

I think the idea is simply that it should not be mandatory to have them
to use Patchwork.

> Furthermore, we already accepted these IDs for gerrit.  I find them
> readily ignorable.
> 
> I think modifying Patchwork to do this is very simple.  First, we could
> either re-use the gerrit commit hook, or write our own (I wrote one
> once, it's in my github).
> 
> Second, in the Patchwork tree is a file, tools/post-receive.hook.  This
> is a shell script that has a function to get the patch hash:
> 
>     get_patchwork_hash() {
>         local hash
>         hash=$(git diff "$1~..$1" | python $PW_DIR/hasher.py)
>         echo "$hash"
>         test -n "$hash"
>     }
> 
> I think all that's needed is to replace this with something to extract
> our ID.  This can be done with "git interpret-trailers --parse".

Yep.

> One other thing gerrit provides is the ability to fetch patches from
> gerrit.  This can be done with patchworks as well, using its CLI.
> However, I imagine it is less reliable, since it isn't git on the other
> end... like, with git you can do 3-way merges, but with patchworks you
> couldn't.

When the author has used send-email, "git am -3" usually works well. I hope
that git-pw uses it.

> I've looked at patchworks a little, as you can tell; but I haven't
> looked at gerrit at all.  So, I have no idea of the relative difficulty
> of adding the features we want to gerrit.

From what I saw, it's way more than the time I have to put on it without
becoming a full-time Gerrit developer.

Simon