RFE: use patchwork to submit a patch
Stephen Finucane
stephen at that.guru
Wed Oct 16 00:55:58 AEDT 2019
On Thu, 2019-10-10 at 10:41 -0400, Konstantin Ryabitsev wrote:
> Hi, all:
>
> I would like to propose a new (large) feature to patchwork with the goal
> to make the process of submitting a patch easier for newbies and people
> generally less familiar with patch-based development. This was discussed
> previously on the workflows list:
> https://lore.kernel.org/workflows/20190930202451.GA14403@pure.paranoia.local/
>
> How I envision this would work:
I've got a counterproposal.
> - user creates an account (which requires a mail confirmation)
Yup.
> - they choose a "submit patch" option from the menu
Yup.
> - the patch submission screen has a succession of screens:
>
> 1. a screen with a single field allowing a user to paste a URL to
> their fork of the git repository. Once submitted, patchwork does a
> "git ls-remote" to attempt to get a list of refs and to verify that
> this is indeed a valid git repository
>
> 2. next screen asks the user to select the ref to work from using the
> list obtained from the remote. Once submitted, patchwork performs a
> `git clone --reference` to clone the repository locally using a
> local fork of the same repo to minimize object transfer. This part
> requires that:
> a. patchwork project is configured with a path to a local fork,
> if this feature is enabled for a project
> b. that fork is kept current via some mechanism outside of
> patchwork (e.g. with grokmirror)
> c. there is some sanity-checking during the clone process to
> avoid abuse (e.g. a sane timeout, a tmpdir with limited size,
> etc -- other suggestions welcome)
>
> 3. next screen asks the user to pick a starting commit from the log.
> Once submitted, patchwork generates the patch from the commit
> provided to the tip of the branch selected by the user earlier,
> using git format-patch.
>
> 4. next screen asks the user to review the patch to make sure this is
> what they want to submit. Once confirmed, patchwork performs two
> admin-defined optional hooks:
>
> a. a hook to generate a list of cc's (e.g. get_maintainer.pl)
> b. a sanity check hook (e.g. checkpatch.pl)
>
> 5. if sanity checking is defined, next screen shows the output of the
> sanity check hook, asking confirmation to proceed.
>
> 6. next screen shows the user three fields:
>
> a. title of the patch/series
> b. cover letter for the patch/series
> c. message-id of the previous patch revision (can be picked from
> the list of previously submitted series by this user --
> patchwork should have them already)
> d. number of the revision (can be auto-filled if previous field
> is provided) and other tags to include in []
>
> 7. next screen shows final review of what would be sent out to the
> list (and cc's, if the hook to get cc's is defined and returned any
> results). Once submitted, patchwork sends the patch/series using
> patchwork's envelope-from and the user's own email in the From:
> header.
>
> 8. once sent successfully, cleanups are performed (also needs to be
> done as part of the regular cron job, for any aborted attempts)
I'd be very reluctant to do any of this, at least from Patchwork. It's
too complicated and is way too likely to break. Instead, let's provide
a way to send email (patches and replies) from the web interface. Think
something like Google Groups or Mailman 3.0 (see attached images). The
flow would be something like this:
- the patch submission screen contains the following
1. An upload button that allows multiple files. Filenames are checked
client- and server-side to ensure they are valid patches, match the
output generated by 'git-format-patch' [1] are under a certain
filesize, don't skip some numbers, etc.
2. Optional fields for a subject and description. If these are
provided, they form a cover letter email.
3. A submit buton.
- once submitted, Patchwork will send the email on behalf of the email
associated with the logged in user. Patchwork would then throw
everything away
- the patch is received by the list and returned to Patchwork.
Patchwork then parses this like any other mail
- people can reply to the patch via email or via the web UI. If the
latter, the reply is sent, in plain-text format, on behalf of the user.
One thing I'm still on the fence about is whether Patchwork should
store the patches there and there or wait until the email is received
back from the list and parse that. The latter could be potentially
confusing if a user sends a patch but it's not accepted by the list
(the list is down, doesn't allow emails from non-members, etc.).
However, if we pursued the former we'd end up with stuff in Patchwork
that never made it to the list. Maybe we should store both and just
flag emails that we haven't received back yet.
There are a couple of additional issues we're also going to need to be
aware of here:
* Spam prevention. We don't want anyone using Patchwork to submit
rubbish to a list
* Validation. We're taking untrusted input so we need to think about
how to parse this without compromising the underlying host. With
that said, we effectively do this already so not much is changing.
* Other stuff I haven't thought of
With that said though, this _seems_ to provide much of what you want
with only a slight bit more work on the user end. We can provide
instructions for how to generate the patch in the UI and wrap the tool
locally ('git pw series create'). If someone really wanted, they could
provide what you've suggested above and simply buzz the Patchwork API
to actually push the patch(es), but I don't think that's all that
necessary.
What do you think?
Stephen
[1] This does take us away from the "we're VCS independent" claim
Patchwork has. We might want to make it pluggable for those using
Mercurial, Darcs, or anything else.
> I know this is a pretty big RFE, and I would like to hear your thoughts
> about this. If there is general agreement that this is doable/good idea,
> I may be able to come up with funding for this development as part of
> the overall tooling improvement proposal.
>
> Best regards,
> -K
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screen Shot 2019-10-15 at 14.18.22.png
Type: image/png
Size: 356694 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20191015/9de67f0f/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screen Shot 2019-10-15 at 14.18.29.png
Type: image/png
Size: 123990 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20191015/9de67f0f/attachment-0003.png>
More information about the Patchwork
mailing list