Linux Test Project
 help / color / mirror / Atom feed
* [LTP] [PATCH v5 0/4] Support for Patchwork CI
@ 2025-04-11 11:43 Andrea Cervesato
  2025-04-11 11:43 ` [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script Andrea Cervesato
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Andrea Cervesato @ 2025-04-11 11:43 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

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
  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

-- 
2.43.0


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script
  2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
@ 2025-04-11 11:43 ` Andrea Cervesato
  2025-04-14 12:51   ` Petr Vorel
  2025-04-11 11:43 ` [LTP] [PATCH v5 2/4] ci: add patchwork communication script Andrea Cervesato
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Andrea Cervesato @ 2025-04-11 11:43 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 ci/alpine-runtime.sh |  2 ++
 ci/alpine.sh         |  2 ++
 ci/debian.i386.sh    |  2 ++
 ci/debian.sh         | 28 +++++++++++++++-------------
 ci/fedora.sh         |  2 ++
 ci/tumbleweed.sh     |  2 ++
 6 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/ci/alpine-runtime.sh b/ci/alpine-runtime.sh
index 3bff42770..d0e1990d2 100755
--- a/ci/alpine-runtime.sh
+++ b/ci/alpine-runtime.sh
@@ -4,6 +4,8 @@
 
 apk add \
         acl \
+        curl \
+        jq \
         keyutils \
         libaio \
         libacl \
diff --git a/ci/alpine.sh b/ci/alpine.sh
index 5a44a6687..254f4aaec 100755
--- a/ci/alpine.sh
+++ b/ci/alpine.sh
@@ -9,6 +9,8 @@ apk add \
 	autoconf \
 	automake \
 	clang \
+	curl \
+	jq \
 	gcc \
 	git \
 	acl-dev \
diff --git a/ci/debian.i386.sh b/ci/debian.i386.sh
index 284605309..44c7ddf2f 100755
--- a/ci/debian.i386.sh
+++ b/ci/debian.i386.sh
@@ -6,6 +6,8 @@ dpkg --add-architecture i386
 apt update
 
 apt install -y --no-install-recommends \
+	curl \
+	jq \
 	linux-libc-dev:i386 \
 	gcc-multilib \
 	libacl1-dev:i386 \
diff --git a/ci/debian.sh b/ci/debian.sh
index f590b4b9a..96c2651b3 100755
--- a/ci/debian.sh
+++ b/ci/debian.sh
@@ -4,7 +4,7 @@
 
 # workaround for missing oldstable-updates repository
 # W: Failed to fetch http://deb.debian.org/debian/dists/oldstable-updates/main/binary-amd64/Packages
-grep -v oldstable-updates /etc/apt/sources.list > /tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list
+grep -v oldstable-updates /etc/apt/sources.list >/tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list
 
 apt update
 
@@ -23,6 +23,8 @@ pkg_minimal="
 	debhelper
 	devscripts
 	clang
+	curl
+	jq
 	gcc
 	git
 	iproute2
@@ -47,18 +49,18 @@ pkg_nonessential="
 "
 
 case "$ACTION" in
-	minimal)
-		echo "=== Installing only minimal dependencies ==="
-		$install $pkg_minimal
-		;;
-	remove-nonessential)
-		echo "=== Make sure devel libraries are removed ==="
-		$remove $pkg_nonessential
-		;;
-	*)
-		echo "=== Installing dependencies ==="
-		$install $pkg_minimal $pkg_nonessential
-		;;
+minimal)
+	echo "=== Installing only minimal dependencies ==="
+	$install $pkg_minimal
+	;;
+remove-nonessential)
+	echo "=== Make sure devel libraries are removed ==="
+	$remove $pkg_nonessential
+	;;
+*)
+	echo "=== Installing dependencies ==="
+	$install $pkg_minimal $pkg_nonessential
+	;;
 esac
 
 df -hT
diff --git a/ci/fedora.sh b/ci/fedora.sh
index bef5bcd2b..494de928f 100755
--- a/ci/fedora.sh
+++ b/ci/fedora.sh
@@ -9,6 +9,8 @@ $yum \
 	automake \
 	make \
 	clang \
+	curl \
+	jq \
 	gcc \
 	git \
 	findutils \
diff --git a/ci/tumbleweed.sh b/ci/tumbleweed.sh
index 33937ec63..d0607eed2 100755
--- a/ci/tumbleweed.sh
+++ b/ci/tumbleweed.sh
@@ -8,6 +8,8 @@ $zyp \
 	autoconf \
 	automake \
 	clang \
+	curl \
+	jq \
 	findutils \
 	gcc \
 	git \
-- 
2.43.0


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v5 2/4] ci: add patchwork communication script
  2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
  2025-04-11 11:43 ` [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script Andrea Cervesato
@ 2025-04-11 11:43 ` Andrea Cervesato
  2025-04-14 14:02   ` Petr Vorel
  2025-04-11 11:43 ` [LTP] [PATCH v5 4/4] ci: apply patchwork series in ci-docker-build workflow Andrea Cervesato
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Andrea Cervesato @ 2025-04-11 11:43 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Add a script to communicate with patchwork. Available commands are:

- state: change patch-series state
- check: send a tests report to patchwork
- verify: will print a list of new patch-series which has not been
  tested in the past hour (by default)

The script can be configured defining:

- PATCHWORK_URL: patchwork url to communicate with
- PATCHWORK_TOKEN: patchwork authentication token
- PATCHWORK_SINCE: timespan in seconds where we want to fetch
  patch-series

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 ci/tools/patchwork.sh | 197 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100755 ci/tools/patchwork.sh

diff --git a/ci/tools/patchwork.sh b/ci/tools/patchwork.sh
new file mode 100755
index 000000000..d43b7fd61
--- /dev/null
+++ b/ci/tools/patchwork.sh
@@ -0,0 +1,197 @@
+#!/bin/sh -ex
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Shell script that can be used to communicate with Patchwork via REST API.
+# It has been mainly created for CI purposes, but it can be used in the shell
+# by satisfing minimum requirements.
+#
+# Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
+
+command_exists() {
+        command -v $1 >/dev/null 2>&1
+
+        if [ $? -ne 0 ]; then
+                echo "'$1' must be present in the system"
+                exit 1
+        fi
+}
+
+command_exists "curl"
+command_exists "jq"
+
+if [ -z "$PATCHWORK_URL" ]; then
+        PATCHWORK_URL="https://patchwork.ozlabs.org"
+fi
+
+if [ -z "$PATCHWORK_SINCE" ]; then
+        PATCHWORK_SINCE=3600
+fi
+
+fetch_series() {
+        local current_time=$(date +%s)
+        local since_time=$(expr $current_time - $PATCHWORK_SINCE)
+        local date=$(date -u -d @$since_time +"%Y-%m-%dT%H:%M:%SZ")
+
+        curl -k -G "$PATCHWORK_URL/api/events/" \
+                --data "category=series-completed" \
+                --data "project=ltp" \
+                --data "state=new" \
+                --data "since=$date" \
+                --data "archive=no" |
+                jq -r '.[] | "\(.payload.series.id):\(.payload.series.mbox)"'
+
+        if [ $? -ne 0 ]; then
+                exit 1
+        fi
+}
+
+get_patches() {
+        local series_id="$1"
+
+        curl -k -G "$PATCHWORK_URL/api/patches/" \
+                --data "project=ltp" \
+                --data "series=$series_id" |
+                jq -r '.[] | "\(.id)"'
+
+        if [ $? -ne 0 ]; then
+                exit 1
+        fi
+}
+
+verify_token_exists() {
+        if [ -z "$PATCHWORK_TOKEN" ]; then
+                echo "For this feature you need \$PATCHWORK_TOKEN"
+                exit 1
+        fi
+}
+
+set_patch_state() {
+        local patch_id="$1"
+        local state="$2"
+
+        verify_token_exists
+
+        curl -k -X PATCH \
+                -H "Authorization: Token $PATCHWORK_TOKEN" \
+                -F "state=$state" \
+                "$PATCHWORK_URL/api/patches/$patch_id/"
+
+        if [ $? -ne 0 ]; then
+                exit 1
+        fi
+}
+
+set_series_state() {
+        local series_id="$1"
+        local state="$2"
+
+        get_patches "$series_id" | while IFS= read -r patch_id; do
+                if [ -n "$patch_id" ]; then
+                        set_patch_state "$patch_id" "$state"
+                fi
+        done
+}
+
+get_checks() {
+        local patch_id="$1"
+
+        curl -k -G "$PATCHWORK_URL/api/patches/$patch_id/checks/" |
+                jq -r '.[] | "\(.id)"'
+
+        if [ $? -ne 0 ]; then
+                exit 1
+        fi
+}
+
+already_tested() {
+        local series_id="$1"
+
+        get_patches "$series_id" | while IFS= read -r patch_id; do
+                if [ ! -n "$patch_id" ]; then
+                        continue
+                fi
+
+                get_checks "$patch_id" | while IFS= read -r check_id; do
+                        if [ -n "$check_id" ]; then
+                                echo "$check_id"
+                                return
+                        fi
+                done
+        done
+}
+
+verify_new_patches() {
+        local output="series_ids.txt"
+
+        echo -n '' >"$output"
+
+        fetch_series | while IFS=: read -r series_id series_mbox; do
+                if [ ! -n "$series_id" ]; then
+                        continue
+                fi
+
+                tested=$(already_tested "$series_id")
+                if [ -n "$tested" ]; then
+                        continue
+                fi
+
+                echo "$series_id|$series_mbox" >>"$output"
+        done
+
+        cat "$output"
+}
+
+send_results() {
+        local series_id="$1"
+        local target_url="$2"
+
+        verify_token_exists
+
+        local context=$(echo "$3" | sed 's/:/_/g; s/\//-/g; s/\./-/g')
+        if [ -n "$CC" ]; then
+                context="${context}_${CC}"
+        fi
+
+        if [ -n "$ARCH" ]; then
+                context="${context}_${ARCH}"
+        fi
+
+        local result="$4"
+        if [ "$result" = "cancelled" ]; then
+                return
+        fi
+
+        local state="fail"
+        if [ "$result" = "success" ]; then
+                state="success"
+        fi
+
+        get_patches "$series_id" | while IFS= read -r patch_id; do
+                if [ -n "$patch_id" ]; then
+                        curl -k -X POST \
+                                -H "Authorization: Token $PATCHWORK_TOKEN" \
+                                -F "state=$state" \
+                                -F "context=$context" \
+                                -F "target_url=$target_url" \
+                                -F "description=$result" \
+                                "$PATCHWORK_URL/api/patches/$patch_id/checks/"
+
+                        if [ $? -ne 0 ]; then
+                                exit 1
+                        fi
+                fi
+        done
+}
+
+run="$1"
+
+if [ -z "$run" -o "$run" = "state" ]; then
+        set_series_state "$2" "$3"
+elif [ -z "$run" -o "$run" = "check" ]; then
+        send_results "$2" "$3" "$4" "$5"
+elif [ -z "$run" -o "$run" = "verify" ]; then
+        verify_new_patches
+else
+        echo "Available commands: state, check, verify"
+        exit 1
+fi
-- 
2.43.0


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v5 4/4] ci: apply patchwork series in ci-docker-build workflow
  2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
  2025-04-11 11:43 ` [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script Andrea Cervesato
  2025-04-11 11:43 ` [LTP] [PATCH v5 2/4] ci: add patchwork communication script Andrea Cervesato
@ 2025-04-11 11:43 ` Andrea Cervesato
  2025-04-11 12:07 ` [LTP] [PATCH v5 0/4] Support for Patchwork CI Petr Vorel
  2025-04-14 15:41 ` Petr Vorel
  4 siblings, 0 replies; 17+ messages in thread
From: Andrea Cervesato @ 2025-04-11 11:43 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Modify ci-docker-build workflow in order to apply untested new
patchwork patch-series inside the current branch and to send back
results in the patchwork instance.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 .github/workflows/ci-docker-build.yml | 39 ++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/ci-docker-build.yml b/.github/workflows/ci-docker-build.yml
index 44dcca055..b476d993c 100644
--- a/.github/workflows/ci-docker-build.yml
+++ b/.github/workflows/ci-docker-build.yml
@@ -1,7 +1,19 @@
 # Copyright (c) 2021-2024 Petr Vorel <pvorel@suse.cz>
 
 name: "Test building in various distros in Docker"
-on: [push, pull_request]
+on:
+  push:
+  pull_request:
+  workflow_dispatch:
+    inputs:
+      SERIES_ID:
+        description: LTP patch series ID
+        required: false
+        default: ''
+      SERIES_MBOX:
+        description: LTP patch series URL
+        required: false
+        default: ''
 
 permissions:
   contents: read # to fetch code (actions/checkout)
@@ -125,6 +137,20 @@ jobs:
     - name: Compiler version
       run: $CC --version
 
+    - name: Apply Patchwork series
+      if: inputs.SERIES_ID != '' && inputs.SERIES_MBOX != ''
+      env:
+        PATCHWORK_TOKEN: ${{ secrets.PATCHWORK_TOKEN }}
+      run: |
+        git config --global user.name 'GitHub CI'
+        git config --global user.email 'github@example.com'
+        git config --global --add safe.directory "$GITHUB_WORKSPACE"
+
+        git checkout -b review_patch_series_"${{ inputs.SERIES_ID }}"
+        curl -k "${{ inputs.SERIES_MBOX }}" | git am
+
+        ./ci/tools/patchwork.sh state "${{ inputs.SERIES_ID }}" "needs-review-ack"
+
     - name: ver_linux
       run: ./ver_linux
 
@@ -158,3 +184,14 @@ jobs:
       run: |
         if [ "$MAKE_INSTALL" = 1 ]; then INSTALL_OPT="-i"; fi
         ./build.sh -r install -o ${TREE:-in} $INSTALL_OPT
+
+    - name: Send results to Patchwork
+      if: always() && inputs.SERIES_ID != '' && inputs.SERIES_MBOX != ''
+      env:
+        PATCHWORK_TOKEN: ${{ secrets.PATCHWORK_TOKEN }}
+      run: |
+        ./ci/tools/patchwork.sh check \
+          "${{ inputs.SERIES_ID }}" \
+          "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" \
+          "${{ matrix.container }}" \
+          "${{ job.status }}"
-- 
2.43.0


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
                   ` (2 preceding siblings ...)
  2025-04-11 11:43 ` [LTP] [PATCH v5 4/4] ci: apply patchwork series in ci-docker-build workflow Andrea Cervesato
@ 2025-04-11 12:07 ` Petr Vorel
  2025-04-11 12:10   ` Cyril Hrubis
  2025-04-14 15:41 ` Petr Vorel
  4 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-11 12:07 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi all,

> 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
>   ci: add ci-patchwork-trigger workflow

Andrea did a great effort. Unfortunately we deal with problem probably caused by
our mailing list because 3rd commit "ci: add ci-patchwork-trigger workflow" did
not arrive to mailing list.

I suppose the branch is visible on Andrea's fork:

https://github.com/acerv/ltp/tree/refs/heads/b4/patchwork_ci

Mail is not in "ending moderator requests" on https://lists.linux.it/,
I guess we need to contact ML administrators, I'll do it and Cc you.

Kind regards,
Petr

>   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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-11 12:07 ` [LTP] [PATCH v5 0/4] Support for Patchwork CI Petr Vorel
@ 2025-04-11 12:10   ` Cyril Hrubis
  2025-04-11 12:59     ` Petr Vorel
  0 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2025-04-11 12:10 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> Andrea did a great effort. Unfortunately we deal with problem probably caused by
> our mailing list because 3rd commit "ci: add ci-patchwork-trigger workflow" did
> not arrive to mailing list.
> 
> I suppose the branch is visible on Andrea's fork:
> 
> https://github.com/acerv/ltp/tree/refs/heads/b4/patchwork_ci
> 
> Mail is not in "ending moderator requests" on https://lists.linux.it/,
> I guess we need to contact ML administrators, I'll do it and Cc you.

Wait a bit I'm helping Andrea to debug his email setup.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-11 12:10   ` Cyril Hrubis
@ 2025-04-11 12:59     ` Petr Vorel
  2025-04-11 13:06       ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-11 12:59 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

> Hi!
> > Andrea did a great effort. Unfortunately we deal with problem probably caused by
> > our mailing list because 3rd commit "ci: add ci-patchwork-trigger workflow" did
> > not arrive to mailing list.

> > I suppose the branch is visible on Andrea's fork:

> > https://github.com/acerv/ltp/tree/refs/heads/b4/patchwork_ci

> > Mail is not in "ending moderator requests" on https://lists.linux.it/,
> > I guess we need to contact ML administrators, I'll do it and Cc you.

> Wait a bit I'm helping Andrea to debug his email setup.

I'm sorry, I was too quick to send the question.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-11 12:59     ` Petr Vorel
@ 2025-04-11 13:06       ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 17+ messages in thread
From: Andrea Cervesato via ltp @ 2025-04-11 13:06 UTC (permalink / raw)
  To: Petr Vorel, Cyril Hrubis; +Cc: ltp


On 4/11/25 14:59, Petr Vorel wrote:
>> Hi!
>>> Andrea did a great effort. Unfortunately we deal with problem probably caused by
>>> our mailing list because 3rd commit "ci: add ci-patchwork-trigger workflow" did
>>> not arrive to mailing list.
>>> I suppose the branch is visible on Andrea's fork:
>>> https://github.com/acerv/ltp/tree/refs/heads/b4/patchwork_ci
>>> Mail is not in "ending moderator requests" on https://lists.linux.it/,
>>> I guess we need to contact ML administrators, I'll do it and Cc you.
>> Wait a bit I'm helping Andrea to debug his email setup.
> I'm sorry, I was too quick to send the question.
>
> Kind regards,
> Petr
I tried everything, even changing email client for git send-email and to 
generate separate patches without b4. Same issues.
I can receive and send the patches directly via --cc or --to , but ML 
keeps failing to track 3/4 patch.

I hope we will find a solution.

- Andrea


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script
  2025-04-11 11:43 ` [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script Andrea Cervesato
@ 2025-04-14 12:51   ` Petr Vorel
  2025-04-14 13:00     ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-14 12:51 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi Andrea,

> +++ b/ci/debian.sh
> @@ -4,7 +4,7 @@

>  # workaround for missing oldstable-updates repository
>  # W: Failed to fetch http://deb.debian.org/debian/dists/oldstable-updates/main/binary-amd64/Packages
> -grep -v oldstable-updates /etc/apt/sources.list > /tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list
> +grep -v oldstable-updates /etc/apt/sources.list >/tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list

Could you please drop (before merge) this unrelated change? It's not necessary
nor related to the change.

>  apt update

> @@ -23,6 +23,8 @@ pkg_minimal="
>  	debhelper
>  	devscripts
>  	clang
> +	curl
> +	jq
>  	gcc
>  	git
>  	iproute2
> @@ -47,18 +49,18 @@ pkg_nonessential="
>  "

>  case "$ACTION" in
> -	minimal)
> -		echo "=== Installing only minimal dependencies ==="
> -		$install $pkg_minimal
> -		;;
> -	remove-nonessential)
> -		echo "=== Make sure devel libraries are removed ==="
> -		$remove $pkg_nonessential
> -		;;
> -	*)
> -		echo "=== Installing dependencies ==="
> -		$install $pkg_minimal $pkg_nonessential
> -		;;
> +minimal)
> +	echo "=== Installing only minimal dependencies ==="
> +	$install $pkg_minimal
> +	;;
> +remove-nonessential)
> +	echo "=== Make sure devel libraries are removed ==="
> +	$remove $pkg_nonessential
> +	;;
> +*)
> +	echo "=== Installing dependencies ==="
> +	$install $pkg_minimal $pkg_nonessential
> +	;;
Also this whitespace cleanup should not be part of the change.

With this fixed:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script
  2025-04-14 12:51   ` Petr Vorel
@ 2025-04-14 13:00     ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 17+ messages in thread
From: Andrea Cervesato via ltp @ 2025-04-14 13:00 UTC (permalink / raw)
  To: Petr Vorel, Andrea Cervesato; +Cc: ltp

Hi Petr,

On 4/14/25 14:51, Petr Vorel wrote:
> Hi Andrea,
>
>> +++ b/ci/debian.sh
>> @@ -4,7 +4,7 @@
>>   # workaround for missing oldstable-updates repository
>>   # W: Failed to fetch http://deb.debian.org/debian/dists/oldstable-updates/main/binary-amd64/Packages
>> -grep -v oldstable-updates /etc/apt/sources.list > /tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list
>> +grep -v oldstable-updates /etc/apt/sources.list >/tmp/sources.list && mv /tmp/sources.list /etc/apt/sources.list
> Could you please drop (before merge) this unrelated change? It's not necessary
> nor related to the change.
>
>>   apt update
>> @@ -23,6 +23,8 @@ pkg_minimal="
>>   	debhelper
>>   	devscripts
>>   	clang
>> +	curl
>> +	jq
>>   	gcc
>>   	git
>>   	iproute2
>> @@ -47,18 +49,18 @@ pkg_nonessential="
>>   "
>>   case "$ACTION" in
>> -	minimal)
>> -		echo "=== Installing only minimal dependencies ==="
>> -		$install $pkg_minimal
>> -		;;
>> -	remove-nonessential)
>> -		echo "=== Make sure devel libraries are removed ==="
>> -		$remove $pkg_nonessential
>> -		;;
>> -	*)
>> -		echo "=== Installing dependencies ==="
>> -		$install $pkg_minimal $pkg_nonessential
>> -		;;
>> +minimal)
>> +	echo "=== Installing only minimal dependencies ==="
>> +	$install $pkg_minimal
>> +	;;
>> +remove-nonessential)
>> +	echo "=== Make sure devel libraries are removed ==="
>> +	$remove $pkg_nonessential
>> +	;;
>> +*)
>> +	echo "=== Installing dependencies ==="
>> +	$install $pkg_minimal $pkg_nonessential
>> +	;;
> Also this whitespace cleanup should not be part of the change.
>
> With this fixed:
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr

Yeah, it's done automatically by the bash format tool. I'm going to 
revert it.

Thanks,
Andrea


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 2/4] ci: add patchwork communication script
  2025-04-11 11:43 ` [LTP] [PATCH v5 2/4] ci: add patchwork communication script Andrea Cervesato
@ 2025-04-14 14:02   ` Petr Vorel
  2025-04-15  8:50     ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-14 14:02 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi Andrea,

Generally LGTM, few notes below.
Feel free to ignore them, note at '-e' and '-z "$run" -o' (at the end) are bugs
worth to be fixed.

> Add a script to communicate with patchwork. Available commands are:

> - state: change patch-series state
> - check: send a tests report to patchwork
> - verify: will print a list of new patch-series which has not been
>   tested in the past hour (by default)

> The script can be configured defining:

> - PATCHWORK_URL: patchwork url to communicate with
> - PATCHWORK_TOKEN: patchwork authentication token
> - PATCHWORK_SINCE: timespan in seconds where we want to fetch
>   patch-series

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  ci/tools/patchwork.sh | 197 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 197 insertions(+)
>  create mode 100755 ci/tools/patchwork.sh

> diff --git a/ci/tools/patchwork.sh b/ci/tools/patchwork.sh
> new file mode 100755
> index 000000000..d43b7fd61
> --- /dev/null
> +++ b/ci/tools/patchwork.sh

+1 for having the script in tools directory (the ci/ directory is kept for
distro installation scripts, which can be reused by the others).

> @@ -0,0 +1,197 @@
> +#!/bin/sh -ex
-e in shebang (i.e. 'set -e') means quit after error.
This will lead to some unreachable code. If you have proper error handling, '-e'
should not be needed. Or if you want really to '-e' cause to quit early, please
drop whole error handling (it's useless). '-x' is quite verbose, it will be
visible when the code quit (just without your explanations).

see "Argument List Processing"
https://man7.org/linux/man-pages/man1/dash.1.html

or
https://www.gnu.org/software/bash/manual/bash.html#Bourne-Shell-Builtins#The-Set-Builtin-1


> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Shell script that can be used to communicate with Patchwork via REST API.
> +# It has been mainly created for CI purposes, but it can be used in the shell
> +# by satisfing minimum requirements.
typo: s/satisfing/satisfying/

> +#
> +# Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
> +
> +command_exists() {
> +        command -v $1 >/dev/null 2>&1
Here script quit if $1 will not be found...
> +
> +        if [ $? -ne 0 ]; then
> +                echo "'$1' must be present in the system"
nit: maybe redirect error to stderr?
                   echo "'$1' must be present in the system" >&2

> +                exit 1
> +        fi
... Therefore the above code will not be shown when shebang contains '-e'.
> +}
> +
> +command_exists "curl"
> +command_exists "jq"
very minor: I would make a loop in command_exists (as done in tst_require_cmds) and call it:
command_exists curl jq

> +
> +if [ -z "$PATCHWORK_URL" ]; then
> +        PATCHWORK_URL="https://patchwork.ozlabs.org"
> +fi
> +
> +if [ -z "$PATCHWORK_SINCE" ]; then
> +        PATCHWORK_SINCE=3600
> +fi

IMHO it's more readable if global variables are defined above the functions
(i.e. before command_exists(), not in the middle of function setup.

Also all shells support :- with default parameter:

PATCHWORK_URL="${PATCHWORK_URL:-https://patchwork.ozlabs.org}"
PATCHWORK_SINCE="${PATCHWORK_SINCE:-3600}"

> +
> +fetch_series() {
> +        local current_time=$(date +%s)
> +        local since_time=$(expr $current_time - $PATCHWORK_SINCE)
> +        local date=$(date -u -d @$since_time +"%Y-%m-%dT%H:%M:%SZ")
> +
> +        curl -k -G "$PATCHWORK_URL/api/events/" \
> +                --data "category=series-completed" \
> +                --data "project=ltp" \
> +                --data "state=new" \
> +                --data "since=$date" \
> +                --data "archive=no" |
> +                jq -r '.[] | "\(.payload.series.id):\(.payload.series.mbox)"'
> +
> +        if [ $? -ne 0 ]; then
> +                exit 1
Again, '-e' means quit immediately after curl exit with non-zero code.

Only if you decide to remove '-e': curl has various exit codes. I was thinking
if exit the real curl exit code would be useful. OTOH the error message will be
visible, thus probably not needed.

> +        fi
> +}
> +
> +get_patches() {
> +        local series_id="$1"
> +
> +        curl -k -G "$PATCHWORK_URL/api/patches/" \
> +                --data "project=ltp" \
> +                --data "series=$series_id" |
> +                jq -r '.[] | "\(.id)"'
> +
> +        if [ $? -ne 0 ]; then
> +                exit 1
> +        fi
> +}
> +
> +verify_token_exists() {
> +        if [ -z "$PATCHWORK_TOKEN" ]; then
> +                echo "For this feature you need \$PATCHWORK_TOKEN"
> +                exit 1
> +        fi
> +}
> +
> +set_patch_state() {
> +        local patch_id="$1"
> +        local state="$2"
> +
> +        verify_token_exists
> +
> +        curl -k -X PATCH \
> +                -H "Authorization: Token $PATCHWORK_TOKEN" \
> +                -F "state=$state" \
> +                "$PATCHWORK_URL/api/patches/$patch_id/"
> +
> +        if [ $? -ne 0 ]; then
> +                exit 1
> +        fi
> +}
> +
> +set_series_state() {
> +        local series_id="$1"
> +        local state="$2"
> +
> +        get_patches "$series_id" | while IFS= read -r patch_id; do
> +                if [ -n "$patch_id" ]; then
> +                        set_patch_state "$patch_id" "$state"
> +                fi
> +        done
> +}
> +
> +get_checks() {
> +        local patch_id="$1"
> +
> +        curl -k -G "$PATCHWORK_URL/api/patches/$patch_id/checks/" |
> +                jq -r '.[] | "\(.id)"'
> +
> +        if [ $? -ne 0 ]; then
> +                exit 1
> +        fi
> +}
> +
> +already_tested() {
> +        local series_id="$1"
> +
> +        get_patches "$series_id" | while IFS= read -r patch_id; do
> +                if [ ! -n "$patch_id" ]; then
> +                        continue
> +                fi
FYI: common shell shorter (works everywhere):
                   [ -n "$patch_id" ] || continue

Or even:
                   [ "$patch_id" ] || continue

> +
> +                get_checks "$patch_id" | while IFS= read -r check_id; do
> +                        if [ -n "$check_id" ]; then
> +                                echo "$check_id"
> +                                return
> +                        fi
> +                done
> +        done
> +}
> +
> +verify_new_patches() {
> +        local output="series_ids.txt"
> +
> +        echo -n '' >"$output"

series_ids.txt is stored in current working directory, right? Clearer it'd be to
use mktemp (part of coreutils and busybox, probably as common as date, which we
also expect to be available, at least it's on Alpine).

That way git clone is not polluted.

> +
> +        fetch_series | while IFS=: read -r series_id series_mbox; do
> +                if [ ! -n "$series_id" ]; then
> +                        continue
> +                fi
> +
> +                tested=$(already_tested "$series_id")
> +                if [ -n "$tested" ]; then
> +                        continue
> +                fi
> +
> +                echo "$series_id|$series_mbox" >>"$output"
> +        done
> +
> +        cat "$output"
> +}
> +
> +send_results() {
> +        local series_id="$1"
> +        local target_url="$2"
> +
> +        verify_token_exists
> +
> +        local context=$(echo "$3" | sed 's/:/_/g; s/\//-/g; s/\./-/g')
> +        if [ -n "$CC" ]; then
> +                context="${context}_${CC}"
> +        fi
> +
> +        if [ -n "$ARCH" ]; then
> +                context="${context}_${ARCH}"
> +        fi
> +
> +        local result="$4"
> +        if [ "$result" = "cancelled" ]; then
> +                return
> +        fi
> +
> +        local state="fail"
> +        if [ "$result" = "success" ]; then
> +                state="success"
> +        fi
> +
> +        get_patches "$series_id" | while IFS= read -r patch_id; do
> +                if [ -n "$patch_id" ]; then
> +                        curl -k -X POST \
> +                                -H "Authorization: Token $PATCHWORK_TOKEN" \
> +                                -F "state=$state" \
> +                                -F "context=$context" \
> +                                -F "target_url=$target_url" \
> +                                -F "description=$result" \
> +                                "$PATCHWORK_URL/api/patches/$patch_id/checks/"
This will help you to save one indent:
                   [ -z "$patch_id" ] || continue
				   curl -k -X POST \

> +
> +                        if [ $? -ne 0 ]; then
> +                                exit 1
> +                        fi
> +                fi
> +        done
> +}
> +
> +run="$1"
> +
> +if [ -z "$run" -o "$run" = "state" ]; then
> +        set_series_state "$2" "$3"
> +elif [ -z "$run" -o "$run" = "check" ]; then

'-z "$run"' is unreachable code (if $run is empty, it's run in the above if and
both elif and else blocks are skipped).

You could have something like:

case "$1" in
	state|check|verify|"") run="$1";;
	*) echo "Available commands: state, check, verify"; exit 1;;
esac

if [ -z "$run" -o "$run" = "state" ]; then
        set_series_state "$2" "$3"
fi

if [ -z "$run" -o "$run" = "check" ]; then
        send_results "$2" "$3" "$4" "$5"
fi

if [ -z "$run" -o "$run" = "verify" ]; then
        verify_new_patches
fi

Kind regards,
Petr

> +        send_results "$2" "$3" "$4" "$5"
> +elif [ -z "$run" -o "$run" = "verify" ]; then
> +        verify_new_patches
> +else
> +        echo "Available commands: state, check, verify"
> +        exit 1
> +fi

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
                   ` (3 preceding siblings ...)
  2025-04-11 12:07 ` [LTP] [PATCH v5 0/4] Support for Patchwork CI Petr Vorel
@ 2025-04-14 15:41 ` Petr Vorel
  2025-04-15  7:53   ` Andrea Cervesato via ltp
  4 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-14 15:41 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

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.

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").

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'.

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.

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

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-14 15:41 ` Petr Vorel
@ 2025-04-15  7:53   ` Andrea Cervesato via ltp
  2025-04-15  9:49     ` Petr Vorel
  0 siblings, 1 reply; 17+ messages in thread
From: Andrea Cervesato via ltp @ 2025-04-15  7:53 UTC (permalink / raw)
  To: Petr Vorel, Andrea Cervesato; +Cc: ltp

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.
>
> 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.
>
> 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.
>
> 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.

>
> 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. There are still some things which are 
more important, like showing linting warnings in Patchwork due to "make 
check" command.
>
> 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........

- Andrea


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 2/4] ci: add patchwork communication script
  2025-04-14 14:02   ` Petr Vorel
@ 2025-04-15  8:50     ` Andrea Cervesato via ltp
  2025-04-15  9:04       ` Petr Vorel
  0 siblings, 1 reply; 17+ messages in thread
From: Andrea Cervesato via ltp @ 2025-04-15  8:50 UTC (permalink / raw)
  To: Petr Vorel, Andrea Cervesato; +Cc: ltp

Hi Petr,

On 4/14/25 16:02, Petr Vorel wrote:
> Generally LGTM, few notes below.
> Feel free to ignore them, note at '-e' and '-z "$run" -o' (at the end) are bugs
> worth to be fixed.
We can remove -e, but what about -x ? It's verbose, but it's quite 
helping in the CI.
Maybe I can just set -x before running the script in the workflow.

The rest looks ok.

- Andrea


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 2/4] ci: add patchwork communication script
  2025-04-15  8:50     ` Andrea Cervesato via ltp
@ 2025-04-15  9:04       ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2025-04-15  9:04 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi Andrea,

> Hi Petr,

> On 4/14/25 16:02, Petr Vorel wrote:
> > Generally LGTM, few notes below.
> > Feel free to ignore them, note at '-e' and '-z "$run" -o' (at the end) are bugs
> > worth to be fixed.
> We can remove -e, but what about -x ? It's verbose, but it's quite helping
> in the CI.
> Maybe I can just set -x before running the script in the workflow.

Sure, I'd definitely keep -x.

-e is up to you. You can keep it, but have to wrap commands with if:

if ! curl ...; then
	echo ... >&2
	exit 1
fi

Kind regards,
Petr

> The rest looks ok.

> - Andrea


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-15  7:53   ` Andrea Cervesato via ltp
@ 2025-04-15  9:49     ` Petr Vorel
  2025-04-15  9:54       ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2025-04-15  9:49 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v5 0/4] Support for Patchwork CI
  2025-04-15  9:49     ` Petr Vorel
@ 2025-04-15  9:54       ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 17+ messages in thread
From: Andrea Cervesato via ltp @ 2025-04-15  9:54 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On 4/15/25 11:49, Petr Vorel wrote:
> 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).
We can do it later once we have everything working for basics build tests.
>
>>> 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.

I still don't know, we will need an implementation later on.

- Andrea

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-04-15  9:54 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-11 11:43 [LTP] [PATCH v5 0/4] Support for Patchwork CI Andrea Cervesato
2025-04-11 11:43 ` [LTP] [PATCH v5 1/4] ci: install dependences for patchwork-ci script Andrea Cervesato
2025-04-14 12:51   ` Petr Vorel
2025-04-14 13:00     ` Andrea Cervesato via ltp
2025-04-11 11:43 ` [LTP] [PATCH v5 2/4] ci: add patchwork communication script Andrea Cervesato
2025-04-14 14:02   ` Petr Vorel
2025-04-15  8:50     ` Andrea Cervesato via ltp
2025-04-15  9:04       ` Petr Vorel
2025-04-11 11:43 ` [LTP] [PATCH v5 4/4] ci: apply patchwork series in ci-docker-build workflow Andrea Cervesato
2025-04-11 12:07 ` [LTP] [PATCH v5 0/4] Support for Patchwork CI Petr Vorel
2025-04-11 12:10   ` Cyril Hrubis
2025-04-11 12:59     ` Petr Vorel
2025-04-11 13:06       ` Andrea Cervesato via ltp
2025-04-14 15:41 ` Petr Vorel
2025-04-15  7:53   ` Andrea Cervesato via ltp
2025-04-15  9:49     ` Petr Vorel
2025-04-15  9:54       ` Andrea Cervesato via ltp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox