[Bug Infrastructure/30042] New: patchworks should automatically notice a pushed patch

[Bug Infrastructure/30042] New: patchworks should automatically notice a pushed patch

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

            Bug ID: 30042
           Summary: patchworks should automatically notice a pushed patch
           Product: sourceware
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Infrastructure
          Assignee: overseers at sourceware dot org
          Reporter: tromey at sourceware dot org
  Target Milestone: ---

Consider this patch:

https://patchwork.sourceware.org/project/gdb/patch/20230122212801.1926063-1-tom@tromey.com/

I used git send-email to send this patch and patchworks dutifully
logged it.  Then I pushed the patch -- but patchworks did nothing,
I had to go into the web interface to explicitly mark it as committed.

I think patchworks ought to notice this situation.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos at redhat dot com

--- Comment #1 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Tom Tromey from comment #0)
> Consider this patch:
> 
> https://patchwork.sourceware.org/project/gdb/patch/20230122212801.1926063-1-
> tom@tromey.com/
> 
> I used git send-email to send this patch and patchworks dutifully
> logged it.  Then I pushed the patch -- but patchworks did nothing,
> I had to go into the web interface to explicitly mark it as committed.
> 
> I think patchworks ought to notice this situation.

This is easier said than done unfortunately.

(1) Out of the box feature.

This is not functionality that patchwork provides.

I would be an RFE for patchwork upstream
(https://github.com/getpatchwork/patchwork/).

(2) Commit cleanup script.

Alternatively, in glibc we run a script to compare commits against patchwork
patches and close those that have been committed.

The first problem we saw here was that it requires discipline from the
community to commit *exactly* what was posted and agreed upon in order to make
the checking less error prone.

In glibc we have adopted a stronger policy that you should only commit what the
author provided, or ask the author to change it. My own opinion is that it is
disrespectful to change what the author is submitting.

The only allowed changes are additions of Reviewed-by, Tested-by tags in the
commit message.

In either (1) or (2) I think this is up to the projects to run the scripts that
they need.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |siddhesh at sourceware dot org

--- Comment #2 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
FWIW, glibc uses a variant of this script:

https://gist.github.com/siddhesh/b7be0970d8a70e5d68c36f3957ae19a4

I basically run it as a cron job.  I think this could be adapted by gdb too
with the caveats that Carlos mentioned in comment 1.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Carlos O'Donell from comment #1)

> The first problem we saw here was that it requires discipline from the
> community to commit *exactly* what was posted and agreed upon in order to
> make the checking less error prone.

This would work fine for [pushed] patches, which seems good enough for
this ticket.

For other patches, this seems like more work because it means there
will have to be one final series sent, but patchworks is also bad about
removing old series.

> In either (1) or (2) I think this is up to the projects to run the scripts
> that they need.

Are you saying this shouldn't be a sourceware PR?
I can close it if so.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #4 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Tom Tromey from comment #3)
> (In reply to Carlos O'Donell from comment #1)
> 
> > The first problem we saw here was that it requires discipline from the
> > community to commit *exactly* what was posted and agreed upon in order to
> > make the checking less error prone.
> 
> This would work fine for [pushed] patches, which seems good enough for
> this ticket.

yeah, one step at a time. Siddhesh script could be run under gdbadmin or we
could do it from the buildbot since we build each gdb commit on push.

> For other patches, this seems like more work because it means there
> will have to be one final series sent, but patchworks is also bad about
> removing old series.

We could have a separate script or mail filter that checks for patches that
supersede a previous patch/series (check subject is the same modulo vX+1).

> > In either (1) or (2) I think this is up to the projects to run the scripts
> > that they need.
> 
> Are you saying this shouldn't be a sourceware PR?
> I can close it if so.

We could move it to the gdb product, but lets keep it open here to document the
steps we did so other projects can adopt them if they like.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

--- Comment #5 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
(In reply to Tom Tromey from comment #3)
> (In reply to Carlos O'Donell from comment #1)
> 
> > The first problem we saw here was that it requires discipline from the
> > community to commit *exactly* what was posted and agreed upon in order to
> > make the checking less error prone.
> 
> This would work fine for [pushed] patches, which seems good enough for
> this ticket.
> 
> For other patches, this seems like more work because it means there
> will have to be one final series sent, but patchworks is also bad about
> removing old series.

Yeah I've been dragging my feet on this; I've been meaning to write a script
that automatically marks outdated patches as superseded (e.g. with a matching
subject line) but I never got to doing it.  If someone does, it would be great
to use it for glibc too because right now we clean up manually.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Siddhesh Poyarekar from comment #5)

> Yeah I've been dragging my feet on this; I've been meaning to write a script
> that automatically marks outdated patches as superseded (e.g. with a
> matching subject line) but I never got to doing it.  If someone does, it
> would be great to use it for glibc too because right now we clean up
> manually.

For gdb it would be fine to require gerrit-like Change-Id tags.
We don't care about the noise (a bunch of patches already come with
these anyway), and at least I personally care a lot about automation.
I once tried patching patchworks for this, and I could maybe dig
up my patch, but I didn't have a good way to test it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net

--- Comment #7 from Pedro Alves <pedro at palves dot net> ---
> For gdb it would be fine to require gerrit-like Change-Id tags.

+1

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug Infrastructure/30042] patchworks should automatically notice a pushed patch

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30042

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=30997

-- 
You are receiving this mail because:
You are the assignee for the bug.