* Apply Patchseries Script
@ 2023-09-27 16:31 Patrick Robb
2023-09-27 20:22 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Robb @ 2023-09-27 16:31 UTC (permalink / raw)
To: ci
[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]
Hello all,
The Community Lab is reviewing and rewriting some parts of our (currently
internal) apply patchseries script. The reasons are:
1. We want to remove any dependency the apply script has on our internal
infrastructure, so that it can be upstreamed and utilized by others in the
community.
2. We want to add in new features like “depends-on patch” applying (like
ovsrobot is doing currently)
3. Some DPDK project processes have changed (like moving next branches from
the main repo to being their own distinct repos). We have added on
workarounds along the way to account for this, but an overall rework is now
in order to clean up our process.
Before we do the work and attempt to upstream the script, I want to verify
with the community that our current assumptions regarding the apply
patchseries process are appropriate and should not be tweaked. Assumptions:
1. There are two inputs, A. The pw series url and B. The branch output of
pw_maintainers_cli.py
2. Do not apply and run if the series is an RFC series
3. Always check out to the current head of tree when applying a patch,
regardless of whether the tree state has changed between patch submission
and patch application in CI.
4. If the cover letter contains “depends-on,” extract the dependency series
id(s), apply those, then attempt to apply the patch
5. If patch does not cleanly apply to the branch supplied by
pw_maintainers_cli.py, attempt to apply on dpdk main. If this also fails,
report an apply failure.
6. If apply is successful, attempt a sanity build, and report a build
failure if that fails. If it succeeds, proceed with all CI testing.
Note: The Community Lab does not currently use pw-client. If it is better
for the CI community, we could stop maintaining a dedicated script for the
apply process, try moving the pw-client, and direct our efforts at patching
pw-client with the goal of adding support for features like depends-on. Are
other labs using pw-client right now and do you recommend it?
[-- Attachment #2: Type: text/html, Size: 8994 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Apply Patchseries Script
2023-09-27 16:31 Apply Patchseries Script Patrick Robb
@ 2023-09-27 20:22 ` Thomas Monjalon
2023-09-28 0:16 ` Patrick Robb
2023-09-28 12:06 ` Aaron Conole
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Monjalon @ 2023-09-27 20:22 UTC (permalink / raw)
To: Patrick Robb; +Cc: ci
27/09/2023 18:31, Patrick Robb:
> Hello all,
>
> The Community Lab is reviewing and rewriting some parts of our (currently
> internal) apply patchseries script. The reasons are:
>
> 1. We want to remove any dependency the apply script has on our internal
> infrastructure, so that it can be upstreamed and utilized by others in the
> community.
Good news.
> 2. We want to add in new features like “depends-on patch” applying (like
> ovsrobot is doing currently)
Yes would be fantastic.
> 3. Some DPDK project processes have changed (like moving next branches from
> the main repo to being their own distinct repos). We have added on
> workarounds along the way to account for this, but an overall rework is now
> in order to clean up our process.
I don't think it changed.
> Before we do the work and attempt to upstream the script, I want to verify
> with the community that our current assumptions regarding the apply
> patchseries process are appropriate and should not be tweaked. Assumptions:
>
> 1. There are two inputs, A. The pw series url and B. The branch output of
> pw_maintainers_cli.py
So it is only 1 input, because B can be deduced from A.
> 2. Do not apply and run if the series is an RFC series
Not sure about this requirement.
What is the problem in running tests on RFC?
> 3. Always check out to the current head of tree when applying a patch,
> regardless of whether the tree state has changed between patch submission
> and patch application in CI.
I don't think it is reasonable to look for the exact tree state
of patch submission, so yes I agree to use the head of the tree.
If it becomes quickly non applicable, then the author needs to update.
It does not happen frequently.
> 4. If the cover letter contains “depends-on,” extract the dependency series
> id(s), apply those, then attempt to apply the patch
Yes
> 5. If patch does not cleanly apply to the branch supplied by
> pw_maintainers_cli.py, attempt to apply on dpdk main. If this also fails,
> report an apply failure.
Yes
> 6. If apply is successful, attempt a sanity build, and report a build
> failure if that fails. If it succeeds, proceed with all CI testing.
Yes
> Note: The Community Lab does not currently use pw-client. If it is better
> for the CI community, we could stop maintaining a dedicated script for the
> apply process, try moving the pw-client, and direct our efforts at patching
> pw-client with the goal of adding support for features like depends-on. Are
> other labs using pw-client right now and do you recommend it?
In general I think it is a good idea to use common tools.
About adding depends-on support, it looks a great idea.
Other projects could use the same syntax then.
That's the same for the CI support in patchwork: we invented it in DPDK.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Apply Patchseries Script
2023-09-27 20:22 ` Thomas Monjalon
@ 2023-09-28 0:16 ` Patrick Robb
2023-09-28 12:05 ` Aaron Conole
2023-09-28 12:06 ` Aaron Conole
1 sibling, 1 reply; 6+ messages in thread
From: Patrick Robb @ 2023-09-28 0:16 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: ci
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
On Wed, Sep 27, 2023 at 4:22 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 27/09/2023 18:31, Patrick Robb:
>
> > 2. Do not apply and run if the series is an RFC series
>
> Not sure about this requirement.
> What is the problem in running tests on RFC?
>
> I see that currently ovsrobot and UNH Lab have rules saying don't test on
RFC series, and Loongson and Intel do test on RFC series. I'm guessing the
thinking was something like "RFC patches are at least one stage away from
merge, and probably do not represent the final state of the patch, so CI
testing is not very valuable." On the other hand, I'm sure in many cases
getting that early feedback, even for an RFC, is helpful to developers.
I'll bring it up in the CI testing meeting tomorrow and see if any of the
CI testing people have an opinion. Anyways, I think all labs should have
the same policy, be it testing or not testing on RFC patches.
Thanks for the feedback.
[-- Attachment #2: Type: text/html, Size: 1298 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Apply Patchseries Script
2023-09-28 0:16 ` Patrick Robb
@ 2023-09-28 12:05 ` Aaron Conole
2023-09-28 12:31 ` David Marchand
0 siblings, 1 reply; 6+ messages in thread
From: Aaron Conole @ 2023-09-28 12:05 UTC (permalink / raw)
To: Patrick Robb; +Cc: Thomas Monjalon, ci
Patrick Robb <probb@iol.unh.edu> writes:
> On Wed, Sep 27, 2023 at 4:22 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 27/09/2023 18:31, Patrick Robb:
>
> > 2. Do not apply and run if the series is an RFC series
>
> Not sure about this requirement.
> What is the problem in running tests on RFC?
>
> I see that currently ovsrobot and UNH Lab have rules saying don't test on RFC series, and Loongson and Intel do test on
> RFC series. I'm guessing the thinking was something like "RFC patches are at least one stage away from merge, and
> probably do not represent the final state of the patch, so CI testing is not very valuable." On the other hand, I'm sure in
> many cases getting that early feedback, even for an RFC, is helpful to developers. I'll bring it up in the CI testing
> meeting tomorrow and see if any of the CI testing people have an opinion. Anyways, I think all labs should have the
> same policy, be it testing or not testing on RFC patches.
We do currently skip running RFCs as well. IIRC they were eating into
our timing budget on Travis, and we never bothered to re-evaluate after
the switch to github actions. I think it would be good to discuss it.
> Thanks for the feedback.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Apply Patchseries Script
2023-09-28 12:05 ` Aaron Conole
@ 2023-09-28 12:31 ` David Marchand
0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2023-09-28 12:31 UTC (permalink / raw)
To: Aaron Conole; +Cc: Patrick Robb, Thomas Monjalon, ci
On Thu, Sep 28, 2023 at 2:05 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Patrick Robb <probb@iol.unh.edu> writes:
>
> > On Wed, Sep 27, 2023 at 4:22 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 27/09/2023 18:31, Patrick Robb:
> >
> > > 2. Do not apply and run if the series is an RFC series
> >
> > Not sure about this requirement.
> > What is the problem in running tests on RFC?
> >
> > I see that currently ovsrobot and UNH Lab have rules saying don't test on RFC series, and Loongson and Intel do test on
> > RFC series. I'm guessing the thinking was something like "RFC patches are at least one stage away from merge, and
> > probably do not represent the final state of the patch, so CI testing is not very valuable." On the other hand, I'm sure in
> > many cases getting that early feedback, even for an RFC, is helpful to developers. I'll bring it up in the CI testing
> > meeting tomorrow and see if any of the CI testing people have an opinion. Anyways, I think all labs should have the
> > same policy, be it testing or not testing on RFC patches.
>
> We do currently skip running RFCs as well. IIRC they were eating into
> our timing budget on Travis, and we never bothered to re-evaluate after
> the switch to github actions. I think it would be good to discuss it.
I once missed some issues in a RFC of mine that I only discovered when
sending the first non-RFC patch.
As far as time budget is concerned now, I don't think testing RFC is
that much of an issue.
We have way more people sending 3-4 revisions in a single day than
people sending RFCs :-).
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Apply Patchseries Script
2023-09-27 20:22 ` Thomas Monjalon
2023-09-28 0:16 ` Patrick Robb
@ 2023-09-28 12:06 ` Aaron Conole
1 sibling, 0 replies; 6+ messages in thread
From: Aaron Conole @ 2023-09-28 12:06 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Patrick Robb, ci
Thomas Monjalon <thomas@monjalon.net> writes:
> 27/09/2023 18:31, Patrick Robb:
>> Hello all,
>>
>> The Community Lab is reviewing and rewriting some parts of our (currently
>> internal) apply patchseries script. The reasons are:
>>
>> 1. We want to remove any dependency the apply script has on our internal
>> infrastructure, so that it can be upstreamed and utilized by others in the
>> community.
>
> Good news.
>
>> 2. We want to add in new features like “depends-on patch” applying (like
>> ovsrobot is doing currently)
>
> Yes would be fantastic.
>
>> 3. Some DPDK project processes have changed (like moving next branches from
>> the main repo to being their own distinct repos). We have added on
>> workarounds along the way to account for this, but an overall rework is now
>> in order to clean up our process.
>
> I don't think it changed.
>
>> Before we do the work and attempt to upstream the script, I want to verify
>> with the community that our current assumptions regarding the apply
>> patchseries process are appropriate and should not be tweaked. Assumptions:
>>
>> 1. There are two inputs, A. The pw series url and B. The branch output of
>> pw_maintainers_cli.py
>
> So it is only 1 input, because B can be deduced from A.
>
>> 2. Do not apply and run if the series is an RFC series
>
> Not sure about this requirement.
> What is the problem in running tests on RFC?
>
>> 3. Always check out to the current head of tree when applying a patch,
>> regardless of whether the tree state has changed between patch submission
>> and patch application in CI.
>
> I don't think it is reasonable to look for the exact tree state
> of patch submission, so yes I agree to use the head of the tree.
> If it becomes quickly non applicable, then the author needs to update.
> It does not happen frequently.
+1
>> 4. If the cover letter contains “depends-on,” extract the dependency series
>> id(s), apply those, then attempt to apply the patch
>
> Yes
>
>> 5. If patch does not cleanly apply to the branch supplied by
>> pw_maintainers_cli.py, attempt to apply on dpdk main. If this also fails,
>> report an apply failure.
>
> Yes
>
>> 6. If apply is successful, attempt a sanity build, and report a build
>> failure if that fails. If it succeeds, proceed with all CI testing.
>
> Yes
>
>> Note: The Community Lab does not currently use pw-client. If it is better
>> for the CI community, we could stop maintaining a dedicated script for the
>> apply process, try moving the pw-client, and direct our efforts at patching
>> pw-client with the goal of adding support for features like depends-on. Are
>> other labs using pw-client right now and do you recommend it?
>
> In general I think it is a good idea to use common tools.
> About adding depends-on support, it looks a great idea.
> Other projects could use the same syntax then.
> That's the same for the CI support in patchwork: we invented it in DPDK.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-28 12:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 16:31 Apply Patchseries Script Patrick Robb
2023-09-27 20:22 ` Thomas Monjalon
2023-09-28 0:16 ` Patrick Robb
2023-09-28 12:05 ` Aaron Conole
2023-09-28 12:31 ` David Marchand
2023-09-28 12:06 ` Aaron Conole
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).