Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> Hi Petr,

> some comments below.

> On 4/14/25 17:41, Petr Vorel wrote:
> > Hi Andrea,

> > > Add support for patch-series validation in the patchwork ML.
> > > We use Github to schedule a trigger every 30 minutes, checking for new
> > > patche-series in parchwork which has not been tested yet.
> > > The way we decide if a patch-series has been tested in patchwork, is
> > > by looking at its status (in particular, if it's "Needs Review / ACK"),
> > > as well as checking if test report has been uploaded to any of the
> > > series patches.
> > > All communication to Patchwrok is done via REST API, using curl and js
> > > tools.
> > > First, we create a script called patchwork-ci.sh that provides all the
> > > commands to read new untested patch-series, set their status and testing
> > > report. Then, we create a scheduled workflow in Gitlab, checking every
> > > 30 minutes if there are new untested patch-series. At the end, we
> > > trigger the main build workflow, used to validate LTP commits in our
> > > Github mainline. All the times we trigger the build workflow, we also
> > > provide the patch-series ID, that will be fetched and applied on the
> > > current branch before running the tests.
> > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> > > ---
> > > Changes in v4:
> > > - patchwork script is now a tool that can be used independently to ci
> > > Andrea Cervesato (4):
> > >    ci: install dependences for patchwork-ci script
> > >    ci: add patchwork communication script
> > Thanks a lot for this patchset!

> > We had some discussion about 3rd patch missing.
> > For these who want to see it, I suppose your branch is online on your LTP fork:
> > https://github.com/acerv/ltp/tree/refs/heads/b4/patchwork_ci

> > TL;DR
> > 1) and 2) should be solved, the rest can be ignored.

> > 1) Run only on single repo (IMPORTANT)

> > I was worried that the workflow from 3rd (missing) patch will cause to be run on
> > any fork which has this merged to master and indeed it does:

> > https://github.com/pevik/ltp/actions/workflows/ci-patchwork-trigger.yml

> > We should limit it to the repository we want to use it, e.g. for official LTP
> > it'd be:

> > if: ${{ github.repository == 'linux-test-project/ltp' }}

> > Sure, it will fail when run without patchwork token and even if token set on
> > more repos it will quit testing when status is already set, but still - why to
> > pollute all forks GitHub Actions and query patchwork from many forks? (FYI we
> > have 1041 forks, sure most of them don't have GitHub Actions enabled and many of
> > them will never get updated).

> > And yes, I'll later today remove it from my master branch.
> The scheduling is indeed bugged at the moment. We need to restrict to
> linux-test-project user only. Thanks for pointing it out.

Great, thanks! (As I wrote this is the only thing which should be fixed).

> > 2) Where to to run the CI
> > FYI ATM the testing is done at Andrea's fork (see many jobs with "Patchwork
> > checker"): https://github.com/acerv/ltp/actions

> > IMHO it'd be worth to create test repo in linux-test-project org on github
> > (independent on any of us), maybe name it ltp-ci or ltp-test with warning
> > in the description it's just a testing repo? Or would it be too confusing?

> > Why?
> > - Independent on any of us.
> > - It pollutes project GitHub Actions quite a lot, maybe it'd be better to not
> > use on linux-test-project/ltp ("Patchwork checker" actions from cron + actual
> > builds "Test building in various distros in Docker").
> Yes it "pollutes" the actions list, but actions list has a filter on the
> left exactly for this reason.
> We can also add a new repo "ltp-ci". The only "problem" is the sync. We need
> to mirror the main ltp repo to ltp-ci and I guess GitHub has a feature for
> this. I wouldn't do it in the scripts: less commands in the workflow, less
> chance to fail it.

Sure, let's keep it on the linux-test-project/ltp unless it shows there is some
problem.

> > 3) Testing on master branch
> > The reason, why jobs in https://github.com/acerv/ltp/actions are tested on
> > master branch (not on ci/452320 - where number would be number of the series) is
> > the GitHub Action limitation when using schedule:

> > https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#schedule
> > "This event will only trigger a workflow run if the workflow file is on the default branch."
> > => that should be solved by merge

> > "Scheduled workflows will only run on the default branch."
> > => People suggest to use a reusable workflow
> > https://github.com/orgs/community/discussions/16107
> > https://docs.github.com/en/actions/sharing-automations/reusing-workflows#creating-a-reusable-workflow
> > But I'm perfectly ok with this state, unless you have energy to test it with

> > with:
> >      ref: ci

> > This should use 'ci' branch instead of the default 'master'.
> Same problem above: ci should be synced with master via GitHub. Less
> commands in the workflow, less chance to fail it.

Adding 'ref: whatever-branch-we-want-for-ci' does not seem to be too
complicated, but let's keep master now (I'll have look into it later, whether
it's working as expected, then we can reconsider it).

> > 4) Workflow just to push branch (different approach)
> > I originally had a different idea: with first workflow apply patches and push
> > them to some fork (not to the official repo, e.g. to linux-test-project/ci
> > repo), which would trigger CI build.

> > Pros
> > * Not having to download from patchwork and apply patches for each of our 16
> > builds. Our patchwork instance maintainers might appreciate if we try to
> > minimize the load :).
> > * Having non-default branch (e.g. not everything on master, solves 3).
> > * Having branch created for anybody who wants to test it (probably nobody will
> > not import remote with so many branches).

> > Cons
> > * Probably better would be to do some branch cleanup.

> > Maybe we could start using --volume to share stored data:

> > options: --volume /tmp/shared:/shared

> > 5) Link to patchwork series in GitHub Actions job
> > Currently if you spot failure on GitHub Actions job, you will not be
> > able to find the code on patchwork (even series ID would be enough).

> > See:
> > https://github.com/acerv/ltp/actions/runs/14447250705
> > does not link to
> > https://patchwork.ozlabs.org/project/ltp/patch/20250415013944.173030-3-wegao@suse.com/

> > Some info in CI job about the patch would be helpful (more important due 4),
> > because with specific branches ci/452320 it'd be more obvious what is being tested).

> > One can dig the info from logs from "Apply Patchwork series":
> > + curl -k https://patchwork.ozlabs.org/series/452267/mbox/

> > That can be enough.

> > But ideally there would be link to patchwork series, or at least "Test building
> > in various distros in Docker" title was extended to have also series ID.

> Same problem of above: we should simplify as much as possible instead of
> adding layers :-) we are currently running on master, but inside a
> container, so the real issue we have is to show on GiHub UI that we are
> testing a specific patch-series from Patchwork (I already tried but it's not
> easy to do it, like in Gitlab for instance..).  The branching solution is
> interesting if we want to check from GitHub the applied commits, but we
> already have them in Patchwork. Also, we would need to cleanup branches,
> which means to loose them anyway one day or another.

> So I would just keep the actual solution because it's easier to maintain and
> it doesn't require more workflows, layers, commands, etc.

Sure, let's ignore it.

FYI I did not think to add another layer. It would be git push in
ci-patchwork-trigger.yml (more code, but OTOH ci-docker-build.yml would not have
to be modified).

Also, you decide to trigger ci-docker-build.yml via ci-patchwork-trigger.yml,
which ignores ci-sphinx-doc.yml. Therefore patches which modify just
documentation, e.g. even with this CI we can have changes which break
readthedocs.org.

I created a single job ci-sphinx-doc.yml because 1) it's easier to spot what got
broken 2) I consider building the doc in all distros as a waste of time. Should
we reconsider it? I could move building of the doc to ci-sphinx-doc.yml and
remove ci-sphinx-doc.yml. Other option is that you trigger also ci-sphinx-doc.yml
(nothing urgent, can be done later).

> > 6) Links in Patchwork contains job ID
> > It would be nice if links in the patchwork table contain also job to the specific distro, e.g.
> > https://github.com/acerv/ltp/actions/runs/14447250705/job/40510755305
> It 's something I tried at the very beginning but I didn't find a solution
> to get that "40510755305" from /job . I need to read documentation again and
> to try a couple of solutions...it's just a really slow implementation
> process for a simple improvement, so I bothered more about stability and
> basic functionalities :-) We can add this improvement later if it's ok.
Sure, it can wait.

> There are still some things which are more important, like showing linting
> warnings in Patchwork due to "make check" command.

I don't consider this important until LTP is in the state when it's clean. ATM I
would do it only for new files. For modified files I would print warning only
when there are new warnings (compare count warning on master; comparing diff of
warning on master vs. particular patchset will not work because line number
changes).

It should be warning only (not a failure).

I guess this will be separate workflow, right? Once anybody start on it, I guess
we should have script which takes input of changed files and generates output
of make check-* commands.

> > Instead of plain
> > https://github.com/acerv/ltp/actions/runs/14447250705
> > for all jobs. No problem if not easily done.

> > Kind regards,
> > Petr

> > >    ci: add ci-patchwork-trigger workflow
> > >    ci: apply patchwork series in ci-docker-build workflow
> > >   .github/workflows/ci-docker-build.yml      |  39 +++-
> > >   .github/workflows/ci-patchwork-trigger.yml |  63 +++++++
> > >   ci/alpine-runtime.sh                       |   2 +
> > >   ci/alpine.sh                               |   2 +
> > >   ci/debian.i386.sh                          |   2 +
> > >   ci/debian.sh                               |  28 +--
> > >   ci/fedora.sh                               |   2 +
> > >   ci/tools/patchwork.sh                      | 197 +++++++++++++++++++++
> > >   ci/tumbleweed.sh                           |   2 +
> > >   9 files changed, 323 insertions(+), 14 deletions(-)
> > >   create mode 100644 .github/workflows/ci-patchwork-trigger.yml
> > >   create mode 100755 ci/tools/patchwork.sh

> Let's fix the last things and send v6. Still, we will miss 3/4 but I will
> eventually redirect it on ML........

Thanks!

Kind regards,
Petr

> - Andrea


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp