Re: [workflow] Automatically close bug report when a patch is committed

[Maxim Cournoyer]

Hi Giovanni,

Giovanni Biscuolo <g@xelera.eu> writes:

[...]

>>>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>>>> > WDYT about the following
>>>> >   Applies: [patch] <namespace:#bug-number>
>>>> >   Closes: [patch] <namespace:#bug-number>
>>>> >   Resolves: [patch] <namespace:#bug-number>
>>>> >   Done: [patch] <namespace:#bug-number>

[...]

> Anyway, I agree with Liliana that having more keyworks will help humans
> better capture (and remember) the implied semantics (that we should
> definitely document, anyway); for this reason my proposal is to accept
> all this _lowercased_ keywords (all followed by a ":" with NO spaces in
> between): fix, fixes, close, closes, resolve, resolves, do, done.

OK, I now get the point; we're not discussing synonyms but various
actions both 'Closes and 'Fixes' would be implement via the
NNNNN-done@debbugs.gnu.org special email sent to the control server, but
they convey a different idea to us humans.  I agree.

[...]

>> If we choose this simple scheme where the top commit of a series can be
>> annotated with Debbugs control commands, I'd opt for:
>>
>> --8<---------------cut here---------------start------------->8---
>> Applies: #bug-number
>> --8<---------------cut here---------------end--------------->8---
>
> Uh I think I get it: you mean we could use a keyword in the commit
> message to allow the committer to effectively link a commit to a
> #bug-number, right?

Yes!  I think my thought train went that way while Liliana and yours
were more focused on a post push action to close *fixed* issues, right?
What I described here was a general process by which we could close
*patches* series that were forgotten in an 'open' state.

> This is something we sould consider, but it's another topic: how to
> effectively link commits to #bug-num (I guess we already talked about
> this in some other thread)
>
>> I'm not sure what [patch] or namespace add (is it for a fancy URL?), so
>> I'd drop them.
>
> I'll try to recap, sorry for the repetitions!
>
> Namespace has been assumed as part of the proposed URI to try address
> Vagrant's concerns [1]:
>
>
>    Sooooo... I maintain the guix package in Debian, and want to make sure
>    that whatever bug-closing indicator guix upstream uses, does not end up
>    triggering when I push new upstream versions to salsa.debian.org ... and
>    start incorrectly marking incorrect bug numbers on bugs.debian.org that
>    were meant for debbugs.gnu.org.

I don't understand how this risk could be triggered; we're strictly
talking about commits made in the Guix repository, not in one of
Debian's?  Why/how would a Guix commit message trigger a Debian Debbugs
server action?  Maybe if Vagrant put something like:

Fixes: <debbugs.debian.org/NNNNN> that could cause problems?  But then
the URL is different, so we could filter out these, so I don't see the
problem, if we use URLs.  And if we accept just 'Fixes: #NNNNN', it
should be documented that these are strictly intended to refer to Guix
issues, not that of other projects.  I don't think we'll be able to make
this error proof, so we should instead aim to make it convenient for the
main use cases, which is to refer to Guix issues.  Does that make sense?

> Fixes: [optional bug description] <namespace:#bug-number>
>
> (here, URI is <namespace>:#<bug-number>)
>
> ...but then you, Maxim, suggested [3] this form:
>
> Fixes: bug#65738 (java-ts-mode tests)

Note that we can stick with the <issues.guix.gnu.org/NNNNNN> URL and
achieve the same result with some extra config (see: bug#65883).

>>>> If so, that's adding rather than reducing friction, and I'm not sure
>>>> it'd gain much traction.  The way I see it, it needs to happen
>>>> automatically.
>>> I mean, the way I imagine is that you type this as part of your message
>>> and then debbugs would do the work of closing the bug.  In short, "git
>>> push" saves you the work of writing a mail because there's a hook for
>>> it.
>
> I guess all of us are looking for this very same thing: a server side
> web hook that automatically closes bugs (via email) when committers
> pushing "instructs" it to do so.
>
> The automatic email message will be sent to our "bug control and
> manipulation server" [5], with this header:
>
>
> From: GNU Guix git hook <noreply@gnu.org>
> Reply-To: <Committer> <<committer-email>>
> To: control@debbugs.gnu.org
>
>
> and this body:
>
>
> package guix
> close <bug-number> [<commit-hash>]
> quit
>
>
> The "Reply-To:" (I still have to test it) will receive a notification
> from the control server with the results of the commands, including
> errors if any.
>
> Then, the documentation for the close command [5] states:

Note that 'close' is deprecated in favor of 'done', which does send a
reply.

> A notification is sent to the user who reported the bug, but (in
> contrast to mailing bugnumber-done) the text of the mail which caused
> the bug to be closed is not included in that notification.
>
> If you supply a fixed-version, the bug tracking system will note that
> the bug was fixed in that version of the package.
>
> Last but not least, the very fact that "GNU Guix git hook" have closed
> the bug report is tracked and showed in the bug report history, as any
> other action made via email using the Debbugs control server.
>
> WDYT?

Yes, these last bits are what an implementation would have to do,
whether it's a simple hook working from Git trailers or the more complex
idea I had working from mumi and mapping commit sets (via Change-Ids) to
Debbugs guix-patches issues.

> [...]
>
>> The process (*for closing lingering *guix-patches* issues) could go
>> like this:
>>
>> 1. commits of a series pushed to master
>> 2. Savannah sends datagram to a remote machine to trigger the
>> post-commit job, with the newly pushed commits 'Change-Id' values (a
>> list of them).
>> 3. The remote machine runs something like 'mumi close-issues [change-id-1
>> change-id-2 ...]'
>
> I think that extending the already existing post-receive hook is better
> since it does not depend on the availability of a remote service
> receiving a **UDP** datagram.
>
> For sure, we need an enhanced version of mumi CLI (capable of indexing
> Change-Id) on the server running the post-receive hook to achieve this.


>> In case it couldn't close an issue, it could send a notification to the
>> submitter: "hey, I've seen some commits of series NNNN landing to
>> master, but not all of the commits appears to have been pushed, please
>> check"
>
> Interesting!  This could also be done by a server post-receive hook, in
> contrast to a remote service listening for UDP datagrams.

The reason in my scheme why the more capable mumi CLI would be needed is
because closed series would be inferred from commits Change-IDs rather
than explicitly declared.

>> What mumi does internally would be something like:
>>
>> a) Check in its database to establish the Change-Id <-> Issue # relation,
>> if any.
>>
>> b) For each issue, if issue #'s known Change-Ids are all covered by the
>> change-ids in the arguments, close it
>
> I think that b) is better suited for a git post-receive hook and not for
> mumi triggered by a third service; as said above for sure such a script
> needs mumi (CLI) to query the mumi (server) database.

To clarify, the above should be a sequential process; with the Change-Id
scheme, you don't have a direct mapping between a series and the Debbugs
issue -- it needs to be figured out by checking in the Mumi database.

>> This is a bit more complex (UDP datagram, mumi database) but it does
>> useful work for us committers (instead of simply changing the way we
>> currently do the work).
>
> I agree: an automatic bug closing "machinery" when patches are pushed to
> master (and any other official branch?) is the best approach
>
>> When not provided any change-id argument, 'mumi close-issues' could run
>> the process on its complete list of issues.
>
> Do you mean the list of issues provided by "Close/Fix/Resolve:
> #bug-number"?
>
> If I don't miss something, again this is someting that should be
> provided by a git post-receive hook and not by an enhanced version on
> mumi

It could process the 'Fixes: #NNNNN' and other git trailers we choose to
use as well, but what I had on mind was processing the *guix-patches*
outstanding Debbugs issues based on the presence of unique Change-Ids in
them.  It complements the other proposal as it could be useful for when
a committer didn't specify the needed trailers and forgot to close the
issue in *guix-patches*, for example.

>> Since it'd be transparent and requires nothing from a committer, it'd
>> provide value without having to document yet more processes.
>
> No, but we should however document the design of this new kind of
> machinery, so we can always check that the implementation respects the
> design and eventually redesign and refactor if needed.

Yes, it should be summarily described at least in the documentation,
with pointers to the source.

Oof, that's getting long.

To recap:

We have two propositions in there:

1. A simple one that is declarative: new git trailers added to commit
messages would convey actions to be done by the server-side hook.

2. A more complex one that would allow us to close *some* (not all) of
the *guix-patches* issues automatically, relying on Change-Ids (and
Mumi's ability to parse them) to infer which patch series saw all their
commits merged already.

I think both have value to be pursued, but 1. could be implemented first
since it is simpler.

Thanks,

Maxim