Re: More thoughts on Patchwork and Guix patch review/quality assurance

[Ludovic Courtès]

Hi Chris!

Christopher Baines <mail@cbaines.net> skribis:

> If you're interested in using this to review patches, the place to start
> is Patchwork, so visit:
>
>   https://patchwork.cbaines.net/
>
> If you want an account, please email me. Unfortunately I had to disable
> registration due to spam.
>
> When you click through to a patch, the thing to look at is the
> checks. These should link you to the relevant bug, Git branch and Guix
> Data Service comparison. For the Guix Data Service comparison, the
> things to note are the lint warnings and then clicking through to the
> "Compare package derivations" page.

Very nice!  This is useful info when reviewing, and it’s much harder to
get this at a glance locally.

> ### Use Patchwork, rather than Debbugs to track patches
>
> So this has been a thing for 4 years now (see [2] for some graphs). I
> think I was there in the bar in Brussels when it was discussed.
>
> 2: https://debbugs.gnu.org/rrd/guix-patches.html

I remember that night in Brussels, it’s been a while!  :-)

> The main disadvantage I see of using Debbugs is that you have to get a
> bug number before you can Git send-email, in the case where you're
> sending multiple patches.
>
> The main advantage I get from Debbugs is that searching via the bug
> number is really useful.
>
> I'm unsure, I think Debbugs was introduced to help keep track of
> patches, and avoid them getting forgotten in guix-devel. Patchwork will
> do this too, but maybe Debbugs is providing more value than the cost of
> multi-patch series being slightly more difficult to submit?

To me, the main advantages of Debbugs are:

  1. the Emacs interface (I use it a lot as a reviewer, no need to open
     a browser);

  2. the clean web interface at <https://issues.guix.gnu.org>, which I
     find less intimidating, less cluttered, and hopefully more familiar
     than that of Patchwork; it also has a useful search interface now;

  3. Having numbers and short URLs to reference issues.

Now, Patchwork “checks” and how you use them are great; simplifying
multi-patch submissions is great too.

If Mumi had hooks that could be used to run and display those checks,
I’d be super happy.  But I don’t know whether it’s reasonable, nor
whether that’s something others would also like.  :-)

All this could be complemented by CLIs to “the official Guix review
services” (Patchwork, Mumi, Data Service, etc.).  You could run:

  guix review 1234

and it’d fetch the patch(es), apply them, and/or display a status
summary based on data published by Patchwork & co.  It’s probably not
that much work if those services have HTTP APIs, and it could facilitate
adoption.

> ### Scheduled and regular collaboration on IRC to review patches
>
> When I can make some time, I'll try this out, but if anyone has some
> time they can set aside, please go ahead!

Yes!  A weekly meeting where committers and submitters are around and
focusing on getting patches merged would be great!

> ### Help users with the submitting part of submitting patches
>
> Using git send-email works well with Patchwork at least, and it's fine
> with Debbugs if you've got a single patch, but a little more time
> consuming if you're got more than one patch.
>
> Attaching a single patch file to an email works OK I think, however
> attaching multiple patch files to the same email confuses git am and
> Patchwork I believe.
>
> I think other approaches like copying the patch in to the body of an
> email, or just copying the output of git diff in to an email are very
> brittle and can make it more time consuming to try and recover or
> recreate the commit(s).
>
> Having the emails for patches is useful for reviewing, but maybe there's
> a way users could push a branch somewhere and then have some service do
> the git send-email thing on their behalf?

Yes, that would be nice.  To me that’s less important than getting the
checks you implemented actually used, though.

> ### Notify interested people about patches
>
> This is something I've been thinking about more generally, supporting
> email subscriptions for things like a new version of a package being
> available, or the package failing to build on
> master/staging/core-updates.
>
> But specifically for patches, maybe there could be a way of subscribing
> so that you're emailed when a patch series upgrades a package you're
> subscribed to, or breaks a package you're subscribed to.
>
> This could help get more people involved in reviewing patches, by making
> it easier for the interested people to find out about patches they're
> interested in.

+1!

> ### Helping people who aren't committers review patches
>
> I don't know how much of this happens, but it might be something to
> better support?
>
> Better supporting it could mean documenting how to get things that have
> passed review merged in, like emailing a list of committers when a patch
> series is ready to merge.

Agreed.  Non-committers do review occasionally and I find it very useful.

> ### Making automated testing more rigorous
>
> So, providing the patches can be automatically processed and apply
> successfully, I'm currently building packages for x86_64-linux and
> i686-linux.
>
> There are more things that could be built if I amend the relevant
> script:
>
>  - System tests (maybe just x86_64-linux is relevant?
>
>  - Cross-compiled packages (the Guix Data Service currently only
>    generates these for x86_64-linux)

ci.guix.gnu.org is doing that and (gnu ci) as well as
etc/system-tests.scm and etc/release-manifest.scm formalize it.

> There are also some things that would require more work/hardware:
>
>  - Building packages for architectures other than
>    x86_64-linux/i668-linux.
>
>    - I have some ideas on getting the Guix Build Coordinator agent
>      running in a childhurd, some general substitute availability would
>      also be good for getting this to work

Note that GNU/Hurd currently uses out-of-chroot builds; IMO that part
needs to be addressed (see <https://issues.guix.gnu.org/43857>).

>  - Building packages on a range of hardware with a range of
>    resources/configurations. This is kind of a better version of just
>    building a package multiple times. It would help spot odd failures
>    earlier, and give valuable data about the reproducibility of the
>    build outputs.
>
>    - Single core vs many cores
>
>    - Not much RAM vs plenty of RAM
>
>    - Different filesystems (btrfs, ext4, ...)
>
>    - Different Linux-libre versions
>
>    - Different system times (2025, 2101, ...) 
>
>    - Maybe building on other OS's (Debian, ...)
>
>    These things require hardware availability and/or being able to
>    control resources for a single build through the guix-daemon. It also
>    requires a way in the Guix Build Coordinator to specify that you want
>    builds to happen in this way, so build a single derivation many
>    times, in each of these environments.

Some of these tests could be done by offloading to “childguix” VMs or by
creating derviations that build derivations in VM.

Thanks!

Ludo’.