[LTP] [PATCH v7 0/4] Support for Patchwork CI

[LTP] [PATCH v7 0/4] Support for Patchwork CI

[Andrea Cervesato]

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 v7:
- patchwork.sh: fix bashism ==
- patchwork.sh: fix return code check for curl
- patchwork.sh: remove IFS= from loops
- patchwork.sh: local cmd in command_exists
- fix message for trigger scripts commit
- Link to v6: https://lore.kernel.org/r/20250415-patchwork_ci-v6-0-81e6d4184af5@suse.com

Changes in v6:
- patchwork.sh: use more "[ $stuff ] || do_something" syntax
- patchwork.sh: remove set -ex
- patchwork.sh: variables default setup
- patchwork.sh: move file creation in tmp folder
- patchwork.sh: check multiple commands exist in once
- trigger workflow now triggers only on linux-test-project/ltp repo
- Link to v5: https://patchwork.ozlabs.org/project/ltp/list/?series=452320

Changes in v5:
- patchwork is now a utility that can be used separately
- Link to v4: https://lore.kernel.org/r/20250411-patchwork_ci-v4-0-7f3c5ba298a1@suse.com

Changes in v4:
- make use of secrets to apply patches in order to set state to
  needs-review-ack
- check for curl failures and eventually exit 1
- use a different format for stored file in 'patchwork-ci.sh verify'
  command
- define PATCHWORK_CI_FILE
- some symbols like . are not supported by patchwork checks, so slurmify
  the string before sending checks context
- Link to v3: https://lore.kernel.org/r/20250411-patchwork_ci-v3-0-c9bb90c6961b@suse.com

Changes in v3:
- secrets are not supported by if: statement, so we remove it
- Link to v2: https://lore.kernel.org/r/20250410-patchwork_ci-v2-0-288f4be835ff@suse.com

Changes in v2:
- check for secrets.PATCHWORK_TOKEN before sending tests results
- v2 to re-trigger lore after missing 3/4 patch
- Link to v1: https://lore.kernel.org/r/20250410-patchwork_ci-v1-0-def85825f46a@suse.com

---
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 |  65 +++++++++++
 ci/alpine-runtime.sh                       |   2 +
 ci/alpine.sh                               |   2 +
 ci/debian.i386.sh                          |   2 +
 ci/debian.sh                               |   2 +
 ci/fedora.sh                               |   2 +
 ci/tools/patchwork.sh                      | 173 +++++++++++++++++++++++++++++
 ci/tumbleweed.sh                           |   2 +
 9 files changed, 288 insertions(+), 1 deletion(-)
---
base-commit: 40c559787d15b2167ba657b3ba431e3299969d98
change-id: 20250410-patchwork_ci-7dc4ae02c40d

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


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

[LTP] [PATCH v7 1/4] ci: install dependences for patchwork-ci script

[Andrea Cervesato]

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

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
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         | 2 ++
 ci/fedora.sh         | 2 ++
 ci/tumbleweed.sh     | 2 ++
 6 files changed, 12 insertions(+)

diff --git a/ci/alpine-runtime.sh b/ci/alpine-runtime.sh
index 3bff42770bd8ae7da3e6e571a843e44c7a56181b..d0e1990d2c25c9f300dd47fb979af4d38e3ad3f9 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 5a44a6687b9eb7743c0fbbb288a133366b17da75..254f4aaece66ef177bc5b4a92cf5160443552378 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 2846053098033dd0215aa724738a90baff290430..44c7ddf2ff6f049ab847b374213c6aee74fbc130 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 f590b4b9a4f910c7e4229f4e2a818cf5b1e818bb..0445c92ec40819e49c0435b1881cf6b202d14087 100755
--- a/ci/debian.sh
+++ b/ci/debian.sh
@@ -23,6 +23,8 @@ pkg_minimal="
 	debhelper
 	devscripts
 	clang
+	curl
+	jq
 	gcc
 	git
 	iproute2
diff --git a/ci/fedora.sh b/ci/fedora.sh
index bef5bcd2b519f4a32533a2d52f182c6dd5e1b7d0..494de928f7434f3310979e595e18162c2042f1e8 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 33937ec63a372ed92e8d4f05ecd435ce902fa5bb..d0607eed25f06fec3064ca75ec7875ce833f9d96 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

[LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Andrea Cervesato]

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

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 ci/tools/patchwork.sh | 173 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 173 insertions(+)

diff --git a/ci/tools/patchwork.sh b/ci/tools/patchwork.sh
new file mode 100755
index 0000000000000000000000000000000000000000..176bd7737aacfc6178cd69a324d74f13bdc120e9
--- /dev/null
+++ b/ci/tools/patchwork.sh
@@ -0,0 +1,173 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Shell script to communicate with Patchwork via REST API.
+# It has been mainly created for CI purposes, but it can be used in the shell
+# by satisfying minimum requirements.
+#
+# Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
+
+PATCHWORK_URL="${PATCHWORK_URL:-https://patchwork.ozlabs.org}"
+PATCHWORK_SINCE="${PATCHWORK_SINCE:-3600}"
+
+command_exists() {
+        local cmd=""
+
+        for cmd in "$@"; do
+                if ! command -v "$cmd" >/dev/null 2>&1; then
+                        echo "'$1' must be present in the system" >&2
+                        exit 1
+                fi
+        done
+}
+
+command_exists "curl" "jq"
+
+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")
+        local stdout=$(curl -k -G "$PATCHWORK_URL/api/events/" \
+                --data "category=series-completed" \
+                --data "project=ltp" \
+                --data "state=new" \
+                --data "since=$date" \
+                --data "archive=no")
+
+        [ $? -eq 0 ] || exit 1
+
+        echo "$stdout" | jq -r '.[] | "\(.payload.series.id) \(.payload.series.mbox)"'
+}
+
+get_patches() {
+        local series_id="$1"
+        local stdout="$(curl -k -G $PATCHWORK_URL/api/patches/ \
+                --data project=ltp \
+                --data series=$series_id)"
+
+        [ $? -eq 0 ] || exit 1
+
+        echo "$stdout" | jq -r '.[] | "\(.id)"'
+}
+
+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/"
+
+        [ $? -eq 0 ] || exit 1
+}
+
+set_series_state() {
+        local series_id="$1"
+        local state="$2"
+
+        get_patches "$series_id" | while read -r patch_id; do
+                if [ "$patch_id" ]; then
+                        set_patch_state "$patch_id" "$state"
+                fi
+        done
+}
+
+get_checks() {
+        local patch_id="$1"
+        local stdout="$(curl -k -G $PATCHWORK_URL/api/patches/$patch_id/checks/)"
+
+        [ $? -eq 0 ] || exit 1
+
+        echo "$stdout" | jq -r '.[] | "\(.id)"'
+}
+
+already_tested() {
+        local series_id="$1"
+
+        get_patches "$series_id" | while read -r patch_id; do
+                [ "$patch_id" ] || continue
+
+                get_checks "$patch_id" | while read -r check_id; do
+                        if [ -n "$check_id" ]; then
+                                echo "$check_id"
+                                return
+                        fi
+                done
+        done
+}
+
+verify_new_patches() {
+        local tmp=$(mktemp -d)
+        local output="$tmp/series_ids.txt"
+
+        touch "$output"
+
+        fetch_series | while read -r series_id series_mbox; do
+                [ "$series_id" ] || continue
+
+                tested=$(already_tested "$series_id")
+                [ "$tested" ] && continue
+
+                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')
+
+        [ "$CC" ] && context="${context}_${CC}"
+        [ "$ARCH" ] && context="${context}_${ARCH}"
+
+        local result="$4"
+        [ "$result" = "cancelled" ] && return
+
+        local state="fail"
+        [ "$result" = "success" ] && state="success"
+
+        get_patches "$series_id" | while read -r patch_id; do
+                [ "$patch_id" ] || continue
+
+                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/"
+
+                [ $? -eq 0 ] && exit 1
+        done
+}
+
+case "$1" in
+state)
+        set_series_state "$2" "$3"
+        ;;
+check)
+        send_results "$2" "$3" "$4" "$5"
+        ;;
+verify)
+        verify_new_patches
+        ;;
+*)
+        echo "Available commands: state, check, verify" >&2
+        exit 1
+        ;;
+esac

-- 
2.43.0


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

[LTP] [PATCH v7 3/4] ci: add ci-patchwork-trigger workflow

[Andrea Cervesato]

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

Add ci-patchwork-trigger workflow that is meant to run every 15 minutes
during the week and every 45 minutes during the weekend, checking for
new untested LTP patches in the Mailing List and running the
ci-docker-build workflow for them.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Acked-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 .github/workflows/ci-patchwork-trigger.yml | 65 ++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/.github/workflows/ci-patchwork-trigger.yml b/.github/workflows/ci-patchwork-trigger.yml
new file mode 100644
index 0000000000000000000000000000000000000000..0938aed5ad0df8af8e2e575283b56475ff219ff3
--- /dev/null
+++ b/.github/workflows/ci-patchwork-trigger.yml
@@ -0,0 +1,65 @@
+# Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
+
+name: "Patchwork checker"
+on:
+  push:
+  schedule:
+    - cron: '*/15 * * * 1-5'
+    - cron: '*/45 * * * 6,0'
+
+env:
+  PATCHWORK_CI_FILE: patchwork-ci-output.txt
+
+jobs:
+  checker:
+    if: ${{ github.repository == 'linux-test-project/ltp' }}
+    runs-on: ubuntu-latest
+    steps:
+      - name: Git checkout
+        uses: actions/checkout@v1
+
+      - name: Verify new patches
+        id: verify
+        run: |
+          ./ci/tools/patchwork.sh verify > "$PATCHWORK_CI_FILE"
+          cat "$PATCHWORK_CI_FILE"
+
+      - name: Run tests
+        if: success()
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const fs = require('fs');
+
+            const output = fs.readFileSync(process.env.PATCHWORK_CI_FILE, 'utf8');
+            if (output.length === 0) {
+              console.log("'patchwork-ci.sh verify' output is empty");
+              return;
+            }
+
+            const lines = output.split('\n');
+            if (lines.length === 0) {
+              console.log("No new patch-series found");
+              return;
+            }
+
+            for (const data of lines) {
+              const [series_id, series_mbox] = data.split('|');
+              if (series_id.length === 0 || series_mbox.length === 0) {
+                console.log(`Malformed data: ${data}`);
+                continue;
+              }
+
+              const response = await github.rest.actions.createWorkflowDispatch({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                ref: context.ref,
+                workflow_id: 'ci-docker-build.yml',
+                inputs: {
+                  SERIES_ID: series_id,
+                  SERIES_MBOX: series_mbox,
+                }
+              });
+
+              console.log(response);
+            }

-- 
2.43.0


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

[LTP] [PATCH v7 4/4] ci: apply patchwork series in ci-docker-build workflow

[Andrea Cervesato]

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.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
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 44dcca055018a1fb06541a788e3152239aea2e66..b476d993c50a3369efdaa9a72787a7f006cfe0b9 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

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

[Andrea Cervesato via ltp]

Hi,

I temporarily disabled the schedule on my personal master branch, since 
this patch-set is going to be merged soon. FYI:

- we need a patchwork user to test new ML patches, so we can use its token
- we need to setup this token into the LTP settings, so we are able to 
communicate with patchwork

Kind regards,
Andrea Cervesato

On 4/15/25 18:39, Andrea Cervesato wrote:
> 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 v7:
> - patchwork.sh: fix bashism ==
> - patchwork.sh: fix return code check for curl
> - patchwork.sh: remove IFS= from loops
> - patchwork.sh: local cmd in command_exists
> - fix message for trigger scripts commit
> - Link to v6: https://lore.kernel.org/r/20250415-patchwork_ci-v6-0-81e6d4184af5@suse.com
>
> Changes in v6:
> - patchwork.sh: use more "[ $stuff ] || do_something" syntax
> - patchwork.sh: remove set -ex
> - patchwork.sh: variables default setup
> - patchwork.sh: move file creation in tmp folder
> - patchwork.sh: check multiple commands exist in once
> - trigger workflow now triggers only on linux-test-project/ltp repo
> - Link to v5: https://patchwork.ozlabs.org/project/ltp/list/?series=452320
>
> Changes in v5:
> - patchwork is now a utility that can be used separately
> - Link to v4: https://lore.kernel.org/r/20250411-patchwork_ci-v4-0-7f3c5ba298a1@suse.com
>
> Changes in v4:
> - make use of secrets to apply patches in order to set state to
>    needs-review-ack
> - check for curl failures and eventually exit 1
> - use a different format for stored file in 'patchwork-ci.sh verify'
>    command
> - define PATCHWORK_CI_FILE
> - some symbols like . are not supported by patchwork checks, so slurmify
>    the string before sending checks context
> - Link to v3: https://lore.kernel.org/r/20250411-patchwork_ci-v3-0-c9bb90c6961b@suse.com
>
> Changes in v3:
> - secrets are not supported by if: statement, so we remove it
> - Link to v2: https://lore.kernel.org/r/20250410-patchwork_ci-v2-0-288f4be835ff@suse.com
>
> Changes in v2:
> - check for secrets.PATCHWORK_TOKEN before sending tests results
> - v2 to re-trigger lore after missing 3/4 patch
> - Link to v1: https://lore.kernel.org/r/20250410-patchwork_ci-v1-0-def85825f46a@suse.com
>
> ---
> 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 |  65 +++++++++++
>   ci/alpine-runtime.sh                       |   2 +
>   ci/alpine.sh                               |   2 +
>   ci/debian.i386.sh                          |   2 +
>   ci/debian.sh                               |   2 +
>   ci/fedora.sh                               |   2 +
>   ci/tools/patchwork.sh                      | 173 +++++++++++++++++++++++++++++
>   ci/tumbleweed.sh                           |   2 +
>   9 files changed, 288 insertions(+), 1 deletion(-)
> ---
> base-commit: 40c559787d15b2167ba657b3ba431e3299969d98
> change-id: 20250410-patchwork_ci-7dc4ae02c40d
>
> Best regards,

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Petr Vorel]

Hi Andrea,

...
> +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")
> +        local stdout=$(curl -k -G "$PATCHWORK_URL/api/events/" \
> +                --data "category=series-completed" \
> +                --data "project=ltp" \
> +                --data "state=new" \
> +                --data "since=$date" \
> +                --data "archive=no")
> +
> +        [ $? -eq 0 ] || exit 1

You may have noticed in tst_test.sh, that local variable never uses $(...).
It assign single value, but never call $(...). This is for a reason.

Try:

$ cat foo.sh
#!/bin/sh

foo()
{
    local foo=$(aasfd_command_which_fails)

    [ $? -eq 0 ] || exit 1
    echo "run after"
}

bar()
{
    local foo

    foo=$(aasfd_command_which_fails)

    [ $? -eq 0 ] || exit 1
    echo "will not run after"
}

foo
bar

---
$ bash ./foo.sh
./foo.sh: line 5: aasfd_command_which_fails: command not found
run after
./foo.sh: line 15: aasfd_command_which_fails: command not found

What happen? $? is assigned from result of local keyword,
it overwrite previous result from $(...). Note even '#!/bin/sh -e'
would not cause it to fail early.

(Deliberately test with bash to demonstrate local behaves oddly not even in dash
or 'busybox sh' but even with bash. And yes, given how many errors we caught
with this script and generate_arch.sh and generate_syscalls.sh due shell strange
syntax and behavior makes me wonder if we really want to use shell scripts for
anything longer than 5 lines.)

Kind regards,
Petr

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Dennis Brendel via ltp]

On 4/15/25 7:35 PM, Petr Vorel wrote:
> Hi Andrea,
> You may have noticed in tst_test.sh, that local variable never uses $(...).
> It assign single value, but never call $(...). This is for a reason.
> [...]
> What happen? $? is assigned from result of local keyword,
> it overwrite previous result from $(...). Note even '#!/bin/sh -e'
> would not cause it to fail early.

Here is the corresponding documentation:
https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#index-local

> The return status is zero unless local is used outside a function, an
invalid name is supplied, or name is a readonly variable

So:
If you are interested in return statuses then don't *mask them* with local.

Use this pattern instead:

local my_var
my_var=$(my_expr)

Kind regards,
Dennis

> (Deliberately test with bash to demonstrate local behaves oddly not even in dash
> or 'busybox sh' but even with bash. And yes, given how many errors we caught
> with this script and generate_arch.sh and generate_syscalls.sh due shell strange
> syntax and behavior makes me wonder if we really want to use shell scripts for
> anything longer than 5 lines.)
> 
> Kind regards,
> Petr
> 


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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Li Wang via ltp]

On Wed, Apr 16, 2025 at 12:40 AM Andrea Cervesato <andrea.cervesato@suse.de>
wrote:

> 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
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  ci/tools/patchwork.sh | 173
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>

Maybe better to move this independent tool to the script/ dir?
This ci/tools/ is so hidden that it can easily be overlooked.


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Andrea Cervesato via ltp]

Hi Petr,

this is exactly the reason why I prefer "set -e" (I'm not a shell script 
expert anyway...). But I was actually thinking that the whole pipe 
command will fail with !=0 ret code anyway, since any curl output which 
is not a REST API reply (JSON string in particular) and having the right 
format, will cause jq to fail. So the first implementation is 
technically working. I would like to keep that simple and check $? later 
on after the piped command as it was before. Let me know what you think.

- Andrea

On 4/15/25 19:35, Petr Vorel wrote:
> Hi Andrea,
>
> ...
>> +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")
>> +        local stdout=$(curl -k -G "$PATCHWORK_URL/api/events/" \
>> +                --data "category=series-completed" \
>> +                --data "project=ltp" \
>> +                --data "state=new" \
>> +                --data "since=$date" \
>> +                --data "archive=no")
>> +
>> +        [ $? -eq 0 ] || exit 1
> You may have noticed in tst_test.sh, that local variable never uses $(...).
> It assign single value, but never call $(...). This is for a reason.
>
> Try:
>
> $ cat foo.sh
> #!/bin/sh
>
> foo()
> {
>      local foo=$(aasfd_command_which_fails)
>
>      [ $? -eq 0 ] || exit 1
>      echo "run after"
> }
>
> bar()
> {
>      local foo
>
>      foo=$(aasfd_command_which_fails)
>
>      [ $? -eq 0 ] || exit 1
>      echo "will not run after"
> }
>
> foo
> bar
>
> ---
> $ bash ./foo.sh
> ./foo.sh: line 5: aasfd_command_which_fails: command not found
> run after
> ./foo.sh: line 15: aasfd_command_which_fails: command not found
>
> What happen? $? is assigned from result of local keyword,
> it overwrite previous result from $(...). Note even '#!/bin/sh -e'
> would not cause it to fail early.
>
> (Deliberately test with bash to demonstrate local behaves oddly not even in dash
> or 'busybox sh' but even with bash. And yes, given how many errors we caught
> with this script and generate_arch.sh and generate_syscalls.sh due shell strange
> syntax and behavior makes me wonder if we really want to use shell scripts for
> anything longer than 5 lines.)
>
> Kind regards,
> Petr

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Andrea Cervesato via ltp]

Hi Petr,

On 4/15/25 19:35, Petr Vorel wrote:
> (Deliberately test with bash to demonstrate local behaves oddly not even in dash
> or 'busybox sh' but even with bash. And yes, given how many errors we caught
> with this script and generate_arch.sh and generate_syscalls.sh due shell strange
> syntax and behavior makes me wonder if we really want to use shell scripts for
> anything longer than 5 lines.)
>
> Kind regards,
> Petr

Not a big deal, the script is really simple anyway. But I will put on 
TODO list the conversion of this script to python 3.x.
Thanks for helping with shell scripting.

- Andrea


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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Andrea Cervesato via ltp]

Hi Li,

On 4/16/25 09:04, Li Wang wrote:
> Maybe better to move this independent tool to the script/ dir?
> This ci/tools/ is so hidden that it can easily be overlooked.

We thought about it, but since it's mostly a related CI script we 
decided to move it into ci/tools. The scripts/ folder seems to be more 
related to LTP tests development, so I'm a bit puzzled now :-)

I let Petr Vorel the last word about it

- Andrea

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Petr Vorel]

Hi Li, Andrea,

> Hi Li,

> On 4/16/25 09:04, Li Wang wrote:
> > Maybe better to move this independent tool to the script/ dir?
> > This ci/tools/ is so hidden that it can easily be overlooked.

> We thought about it, but since it's mostly a related CI script we decided to
> move it into ci/tools. The scripts/ folder seems to be more related to LTP
> tests development, so I'm a bit puzzled now :-)

> I let Petr Vorel the last word about it

Li, FYI I asked Andrea to keep patchwork.sh script in ci/tools/ because it's
related to CI and probably nobody will use it directly. I would like to keep in
script/ only the scripts which will be used by people.  If you really think that
somebody will be using it, we can have it in script/. WDYT?

(And why in ci/tools/ and not in ci/? I'd like to keep ci/*.sh for LTP users
as an easy way to install the dependencies.)

Kind regards,
Petr

> - Andrea

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Li Wang via ltp]

On Wed, Apr 16, 2025 at 4:38 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li, Andrea,
>
> > Hi Li,
>
> > On 4/16/25 09:04, Li Wang wrote:
> > > Maybe better to move this independent tool to the script/ dir?
> > > This ci/tools/ is so hidden that it can easily be overlooked.
>
> > We thought about it, but since it's mostly a related CI script we
> decided to
> > move it into ci/tools. The scripts/ folder seems to be more related to
> LTP
> > tests development, so I'm a bit puzzled now :-)
>
> > I let Petr Vorel the last word about it
>
> Li, FYI I asked Andrea to keep patchwork.sh script in ci/tools/ because
> it's
> related to CI and probably nobody will use it directly. I would like to
> keep in
> script/ only the scripts which will be used by people.  If you really
> think that
> somebody will be using it, we can have it in script/. WDYT?


> (And why in ci/tools/ and not in ci/? I'd like to keep ci/*.sh for LTP
> users
> as an easy way to install the dependencies.)
>

Sure, I'm OK with putting it in the ci/tools/.


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Petr Vorel]

Hi Dennis,

> On 4/15/25 7:35 PM, Petr Vorel wrote:
> > Hi Andrea,
> > You may have noticed in tst_test.sh, that local variable never uses $(...).
> > It assign single value, but never call $(...). This is for a reason.
> > [...]
> > What happen? $? is assigned from result of local keyword,
> > it overwrite previous result from $(...). Note even '#!/bin/sh -e'
> > would not cause it to fail early.

> Here is the corresponding documentation:
> https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#index-local

> > The return status is zero unless local is used outside a function, an
> invalid name is supplied, or name is a readonly variable

> So:
> If you are interested in return statuses then don't *mask them* with local.

> Use this pattern instead:

> local my_var
> my_var=$(my_expr)

Thanks for much better explanation than I provided + link to the doc. I'll Cc
you on my shell related patches :).

I remember this since 87a82a62ce ("lib/tst_test.sh: fix ROD_SILENT command
return status check") [1], another related is e267a022cd ("tst_test.sh: Fix $@
usage on older dash releases") [2].

Because shell has quirks (both POSIX portable syntax or bash or whatever shell
extension) we've had many fixes for shell code, e.g. 0c0076fbaf ("tst_test.sh:
Fix ROD_SILENT() quoting") [3], although some of them are non-bash specific,
e.g. 0bb01e67b3 ("shell: Fix handling default parameters in tst_mkfs()") [4]
(just to list very few).

The biggest problem is that some things aren't easily done with plain shell,
that's why we need to use C helpers in testcases/lib (some of them are just
reusing C API to avoid reimplementing code, but e.g.  testcases/lib/tst_timeout_kill.c
was really needed after few attempts to write code in bash). Also cgroup tests
aren't reliable when written in shell. Therefore we're trying to get rid of
shell API (rewritten tests into C, or make them to use testcases/lib/tst_run_shell.c,
which makes the layer really thin).

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/commit/87a82a62ce3fcdaf174d6e4529e0324742e13684
[2] https://github.com/linux-test-project/ltp/commit/e267a022cd2deb0d5e7280570c711420927ad817
[3] https://github.com/linux-test-project/ltp/commit/0c0076fbaf6e0059b470fadff6240fc56952c218
[4] https://github.com/linux-test-project/ltp/commit/0bb01e67b3ba079f8f069a99422d783fe6da4287

> Kind regards,
> Dennis

> > (Deliberately test with bash to demonstrate local behaves oddly not even in dash
> > or 'busybox sh' but even with bash. And yes, given how many errors we caught
> > with this script and generate_arch.sh and generate_syscalls.sh due shell strange
> > syntax and behavior makes me wonder if we really want to use shell scripts for
> > anything longer than 5 lines.)

> > Kind regards,
> > Petr

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