[Buildroot] [PATCH 0/4 v5] gitlab-ci: allow running test-pkg (branch yem/test-pkg-in-gitlab-ci)

[Buildroot] [PATCH 0/4 v5] gitlab-ci: allow running test-pkg (branch yem/test-pkg-in-gitlab-ci)

[Yann E. MORIN]

Hello All!

This series is a respin of the previous work by Romain:
    http://lists.busybox.net/pipermail/buildroot/2021-June/thread.html#313369

Yann has done quite some extensive rework, so much so that it is not
sane to apply without further review.

So, the big changes from Romain's work, are:
  - introduce new mode for utils/test-pkg, to only prepare the
    configuration but niot build them,
  - support/scripts/generate-gitlab-ci-yml generates the yaml blurb from
    the valid configurations
  - an empty config fragment is now an error
  - a config fragment that yields no test is also an error
  - some eye-candy and code-styles in the shell scripts...

See a pipeline generated with this branch:

    https://gitlab.com/ymorin/buildroot/-/pipelines/328074297

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Romain Naour (2):
      support/misc/gitlab-ci.yml.in: templatise the make command
      utils/test-pkg: add gitlab-ci support

Yann E. MORIN (2):
      utils/test-pkg: remove configurations that are skipped
      utils/test-pkg: add mode to only prepare .config files

 .gitlab-ci.yml                         |  5 +++++
 support/misc/gitlab-ci.yml.in          | 38 ++++++++++++++++++++++++++++------
 support/scripts/generate-gitlab-ci-yml | 27 +++++++++++++++++++++++-
 utils/test-pkg                         | 26 +++++++++++++++++++----
 4 files changed, 85 insertions(+), 11 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/4 v5] support/misc/gitlab-ci.yml.in: templatise the make command

[Yann E. MORIN]

From: Romain Naour <romain.naour@gmail.com>

In a followup commit, the make command used to log and display the last
lines on error will be used in another job.
Factorize it by introducing .run_make template.

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/misc/gitlab-ci.yml.in | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
index fcfff5c6aa..1ee3772154 100644
--- a/support/misc/gitlab-ci.yml.in
+++ b/support/misc/gitlab-ci.yml.in
@@ -26,20 +26,24 @@
         paths:
             - .config
 
+.run_make: &run_make
+    |
+      make O=${OUTPUT_DIR} > >(tee build.log |grep '>>>') 2>&1 || {
+          echo 'Failed build last output'
+          tail -200 build.log
+          exit 1
+      }
+
 .defconfig_base:
     before_script:
         - DEFCONFIG_NAME=${CI_JOB_NAME}
+        - OUTPUT_DIR=output
     script:
         - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
         - make ${DEFCONFIG_NAME}
         - ./support/scripts/check-dotconfig.py .config ./configs/${DEFCONFIG_NAME}
         - echo 'Build buildroot'
-        - |
-            make > >(tee build.log |grep '>>>') 2>&1 || {
-                echo 'Failed build last output'
-                tail -200 build.log
-                exit 1
-            }
+        - *run_make
         - |
             ./support/scripts/boot-qemu-image.py "${DEFCONFIG_NAME}" > >(tee runtime-test.log) 2>&1 || {
                 echo 'Failed runtime test last output'
-- 
2.25.1

[Buildroot] [PATCH 2/4 v5] utils/test-pkg: remove configurations that are skipped

[Yann E. MORIN]

When the config fragment provided by the user is not usable with a
specific toolchain configuration, the resulting .config file was kept
around.

In a follow up commit, we'll need to know, from outside test-pkg, if a
specific configuration was indeed usable or not.

So, unless if the user actually requested to keep the build directories,
remove the .config file when it contains a configration that would be
skipped.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
v5: new patch
---
 utils/test-pkg | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/utils/test-pkg b/utils/test-pkg
index a317d8c17a..54c6c5e8fe 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -161,6 +161,10 @@ build_one() {
     # done in the same locale.
     comm -23 <(sort "${cfg}") <(sort "${dir}/.config") >"${dir}/missing.config"
     if [ -s "${dir}/missing.config" ]; then
+        if [ ${keep} -ne 1 ]; then
+            # Invalid configuration, drop it
+            rm -f "${dir}/.config"
+        fi
         return 1
     fi
     # Remove file, it's empty anyway.
-- 
2.25.1

[Buildroot] [PATCH 3/4 v5] utils/test-pkg: add mode to only prepare .config files

[Yann E. MORIN]

Currently, running test-pkg is only done locally on the developpers
machine.

In a follow up commit, we'll add the possibility to run test-pkg in a
gitlab-ci pipeline and, to speed up things, with one job per buildable
configuration.

As such, we will need that test-pkg only ever prepares the
configuration, and that it does not build them.

Add such a mode, with a new option, --prepare-only

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
Note: naming is hard; naming options is harder; naming options with a
terse term is even harder; naming options with a terse term that is
still meaningful and explains what the option does, is even harder yet.

---
v5: split off from the next patch into this patch
---
 utils/test-pkg | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/utils/test-pkg b/utils/test-pkg
index 54c6c5e8fe..4a20cab57f 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -12,13 +12,13 @@ do_clean() {
 
 main() {
     local o O opts
-    local cfg dir pkg random toolchains_csv toolchain all number mode
+    local cfg dir pkg random toolchains_csv toolchain all number mode prepare_only
     local ret nb nb_skip nb_fail nb_legal nb_tc build_dir keep
     local -a toolchains
     local pkg_br_name
 
-    o='hakc:d:n:p:r:t:'
-    O='help,all,keep,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:'
+    o='hakgc:d:n:p:r:t:'
+    O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:'
     opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")"
     eval set -- "${opts}"
 
@@ -27,6 +27,7 @@ main() {
     keep=0
     number=0
     mode=0
+    prepare_only=0
     toolchains_csv="${TOOLCHAINS_CSV}"
     while [ ${#} -gt 0 ]; do
         case "${1}" in
@@ -39,6 +40,9 @@ main() {
         (-k|--keep)
             keep=1; shift 1
             ;;
+        (-l|--prepare-only)
+            prepare_only=1; shift 1
+            ;;
         (-c|--config-snippet)
             cfg="${2}"; shift 2
             ;;
@@ -127,7 +131,7 @@ main() {
         toolchain="$(basename "${toolchainconfig}" .config)"
         build_dir="${dir}/${toolchain}"
         printf "%40s [%*d/%d]: " "${toolchain}" ${#nb_tc} ${nb} ${nb_tc}
-        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" && ret=0 || ret=${?}
+        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" "${prepare_only}" && ret=0 || ret=${?}
         case ${ret} in
         (0) printf "OK\n";;
         (1) : $((nb_skip++)); printf "SKIPPED\n";;
@@ -147,6 +151,7 @@ build_one() {
     local toolchainconfig="${2}"
     local cfg="${3}"
     local pkg="${4}"
+    local defer="${5}"
 
     mkdir -p "${dir}"
 
@@ -170,6 +175,11 @@ build_one() {
     # Remove file, it's empty anyway.
     rm -f "${dir}/missing.config"
 
+    # Defer building the job to the caller (e.g. a gitlab pipeline)
+    if [ ${defer} -eq 1 ]; then
+        return 0
+    fi
+
     if [ -n "${pkg}" ]; then
         if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
             return 2
@@ -257,6 +267,10 @@ Options:
         Note: the logfile and configuration is always retained, even without
         this option.
 
+    --prepare-only
+        Only prepare the .config files, but do not build them. Output the
+        list of build directories to stdout, and the status on stderr.
+
 Example:
 
     Testing libcec would require a config snippet that contains:
-- 
2.25.1

[Buildroot] [PATCH 4/4 v5] utils/test-pkg: add gitlab-ci support

[Yann E. MORIN]

From: Romain Naour <romain.naour@gmail.com>

The gitlab-ci support in test-pkg allows to parallelize the test-pkg
work into several gitlab jobs. It's much faster than local serialized
testing.

To triger this, a developper will have to add, to the latest commit of
their branch, a token on its own line, followd by a configuration
fragment, e.g.:

    test-pkg config:
    SOME_OPTION=y
    # OTHER_OPTION is not set
    SOME_VARIABLE="some value"

This configuration fragment is used as input to test-pkg.

To be able to generate one job per test to run, we need the list of
tests in the parent pipeline, and the individual .config files (one per
test) in the child pipeline. We use the newly-introduce --prepare-only
mode to test-pkg, and collect all the generated .config files as
artefacts; those are inherited in the child pipeline via the
"needs::pipeline" and "needs::job" directives. This is a bit tricky,
and is best described by the Gitlab-CI documentation [0].

We also list those .config files to generate the actual list of jobs to
run in the child pipeline.

Notes:
  - if the user provides an empty fragment, this is considered an error:
    indeed, without a fragment (and the package name), there is no way
    to know what to test;
  - if that fragment yields an empty list of tests, then there is
    nothing to test either, so that is also considered an error.

[0] https://docs.gitlab.com/ee/ci/yaml/README.html#artifact-downloads-to-child-pipelines

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
[yann.morin.1998 at free.fr:
  - split the change to test-pkg to its own patch
  - generate the actual yml snippet in support/scripts/generate-gitlab-ci-yml,
    listing the .config files created by test-pkg
  - some code-style-candies...
]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
v5: Split the change to test-pkg to its own patch
    List the .config files
    Also store the missing.config files as artefacts

v4: reworked by Yann:
    tst-pkg only lists, does not generate the yml code
    The yml code is generated by support/scripts/generate-gitlab-ci-yml
    Empty fragment is an error
    Fragment that yierlds no test is also an error

v3: Implement Arnout's review: http://lists.busybox.net/pipermail/buildroot/2021-May/310656.html
    Enable artifacts download from child-pipeline

v2: Rework this patch following Arnout review
    use CI_COMMIT_DESCRIPTION
    remove .config from artifacts but keep images directory since
    it can be useful for further issue investigation
    use the "br-test-pkg" prefix for test-pkg jobs
---
 .gitlab-ci.yml                         |  5 +++++
 support/misc/gitlab-ci.yml.in          | 22 +++++++++++++++++++++
 support/scripts/generate-gitlab-ci-yml | 27 +++++++++++++++++++++++++-
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e85ac32033..bf9f2dca6c 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -11,8 +11,11 @@ generate-gitlab-ci-yml:
   stage: generate-gitlab-ci
   script: ./support/scripts/generate-gitlab-ci-yml support/misc/gitlab-ci.yml.in > generated-gitlab-ci.yml
   artifacts:
+    when: always
     paths:
       - generated-gitlab-ci.yml
+      - br-test-pkg/*/.config
+      - br-test-pkg/*/missing.config
 
 buildroot-pipeline:
   stage: build
@@ -21,3 +24,5 @@ buildroot-pipeline:
       - artifact: generated-gitlab-ci.yml
         job: generate-gitlab-ci-yml
     strategy: depend
+  variables:
+    PARENT_PIPELINE_ID: $CI_PIPELINE_ID
diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
index 1ee3772154..be7951b3d2 100644
--- a/support/misc/gitlab-ci.yml.in
+++ b/support/misc/gitlab-ci.yml.in
@@ -80,3 +80,25 @@
             - test-output/*/.config
             - test-output/*/images/*
 
+.test_pkg:
+    stage: build
+    before_script:
+        - OUTPUT_DIR=${CI_JOB_NAME}
+    script:
+        - echo "Configure Buildroot for ${OUTPUT_DIR}"
+        - make O=${OUTPUT_DIR} syncconfig
+        - make O=${OUTPUT_DIR} savedefconfig
+        - echo 'Build buildroot'
+        - *run_make
+    needs:
+        - pipeline: $PARENT_PIPELINE_ID
+          job: generate-gitlab-ci-yml
+    artifacts:
+        when: always
+        expire_in: 2 weeks
+        paths:
+            - build.log
+            - br-test-pkg/*/.config
+            - br-test-pkg/*/defconfig
+            - br-test-pkg/*/build/build-time.log
+            - br-test-pkg/*/build/packages-file-list*.txt
diff --git a/support/scripts/generate-gitlab-ci-yml b/support/scripts/generate-gitlab-ci-yml
index 3f498e08fd..063c5081da 100755
--- a/support/scripts/generate-gitlab-ci-yml
+++ b/support/scripts/generate-gitlab-ci-yml
@@ -23,7 +23,7 @@ _EOF_
 
 gen_tests() {
     local -a basics defconfigs runtimes
-    local do_basics do_defconfigs do_runtime
+    local do_basics do_defconfigs do_runtime do_testpkg
     local defconfigs_ext cfg tst
 
     basics=( DEVELOPERS flake8 package )
@@ -77,9 +77,30 @@ gen_tests() {
         esac
     fi
 
+    # Retrieve defconfig for test-pkg from the git commit message (if any)
+    if grep -q -E '^test-pkg config:$' <<<"${CI_COMMIT_DESCRIPTION}"; then
+        sed -r -n -e '/^test-pkg config:$/{:a;n;p;ba;}' \
+            <<<"${CI_COMMIT_DESCRIPTION}" \
+            >defconfig.frag
+        if [ ! -s defconfig.frag ]; then
+            printf "Empty configuration fragment.\n" >&2; exit 1
+        fi
+        # Use --all since we expect the user having already pre-tested the
+        # new package with the default subset of toolchains.
+        ./utils/test-pkg \
+            --all --prepare-only \
+            --config-snippet defconfig.frag \
+            --build-dir br-test-pkg >&2
+        do_testpkg=( $(ls -1 br-test-pkg/*/.config 2>/dev/null |xargs -r dirname ) )
+        if [ "${#do_testpkg[@]}" -eq 0 ]; then
+            printf "Configuration fragment enables no test.\n" >&2; exit 1
+        fi
+    fi
+
     # If nothing else, at least do the basics to generate a valid pipeline
     if [    -z "${do_defconfigs}" \
          -a -z "${do_runtime}" \
+         -a -z "${do_testpkg}" \
        ]
     then
         do_basics=true
@@ -101,6 +122,10 @@ gen_tests() {
     if ${do_runtime:-false}; then
         printf '%s: { extends: .runtime_test_base }\n' "${runtimes[@]}"
     fi
+
+    if [ -n "${do_testpkg}" ]; then
+        printf '%s: { extends: .test_pkg }\n' "${do_testpkg[@]}"
+    fi
 }
 
 main "${@}"
-- 
2.25.1

[Buildroot] [PATCH 1/4 v5] support/misc/gitlab-ci.yml.in: templatise the make command

[Thomas Petazzoni]

On Mon, 28 Jun 2021 22:15:12 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> From: Romain Naour <romain.naour@gmail.com>
> 
> In a followup commit, the make command used to log and display the last
> lines on error will be used in another job.
> Factorize it by introducing .run_make template.
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>


-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 4/4 v5] utils/test-pkg: add gitlab-ci support

[Yegor Yefremov]

Hi Yann, all,

some typo fixes.

On Mon, Jun 28, 2021 at 10:16 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> From: Romain Naour <romain.naour@gmail.com>
>
> The gitlab-ci support in test-pkg allows to parallelize the test-pkg
> work into several gitlab jobs. It's much faster than local serialized
> testing.
>
> To triger this, a developper will have to add, to the latest commit of

s/triger/trigger and s/developper/developer

> their branch, a token on its own line, followd by a configuration

s/followd/followed

> fragment, e.g.:
>
>     test-pkg config:
>     SOME_OPTION=y
>     # OTHER_OPTION is not set
>     SOME_VARIABLE="some value"
>
> This configuration fragment is used as input to test-pkg.
>
> To be able to generate one job per test to run, we need the list of
> tests in the parent pipeline, and the individual .config files (one per

remove a comma after "pipeline"

> test) in the child pipeline. We use the newly-introduce --prepare-only

s/newly-introduce/newly-introduced

> mode to test-pkg, and collect all the generated .config files as
> artefacts; those are inherited in the child pipeline via the
> "needs::pipeline" and "needs::job" directives. This is a bit tricky,

remove a comma after tricky

Best regards,
Yegor

> and is best described by the Gitlab-CI documentation [0].
>
> We also list those .config files to generate the actual list of jobs to
> run in the child pipeline.
>
> Notes:
>   - if the user provides an empty fragment, this is considered an error:
>     indeed, without a fragment (and the package name), there is no way
>     to know what to test;
>   - if that fragment yields an empty list of tests, then there is
>     nothing to test either, so that is also considered an error.
>
> [0] https://docs.gitlab.com/ee/ci/yaml/README.html#artifact-downloads-to-child-pipelines
>
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> [yann.morin.1998 at free.fr:
>   - split the change to test-pkg to its own patch
>   - generate the actual yml snippet in support/scripts/generate-gitlab-ci-yml,
>     listing the .config files created by test-pkg
>   - some code-style-candies...
> ]
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>
> ---
> v5: Split the change to test-pkg to its own patch
>     List the .config files
>     Also store the missing.config files as artefacts
>
> v4: reworked by Yann:
>     tst-pkg only lists, does not generate the yml code
>     The yml code is generated by support/scripts/generate-gitlab-ci-yml
>     Empty fragment is an error
>     Fragment that yierlds no test is also an error
>
> v3: Implement Arnout's review: http://lists.busybox.net/pipermail/buildroot/2021-May/310656.html
>     Enable artifacts download from child-pipeline
>
> v2: Rework this patch following Arnout review
>     use CI_COMMIT_DESCRIPTION
>     remove .config from artifacts but keep images directory since
>     it can be useful for further issue investigation
>     use the "br-test-pkg" prefix for test-pkg jobs
> ---
>  .gitlab-ci.yml                         |  5 +++++
>  support/misc/gitlab-ci.yml.in          | 22 +++++++++++++++++++++
>  support/scripts/generate-gitlab-ci-yml | 27 +++++++++++++++++++++++++-
>  3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index e85ac32033..bf9f2dca6c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -11,8 +11,11 @@ generate-gitlab-ci-yml:
>    stage: generate-gitlab-ci
>    script: ./support/scripts/generate-gitlab-ci-yml support/misc/gitlab-ci.yml.in > generated-gitlab-ci.yml
>    artifacts:
> +    when: always
>      paths:
>        - generated-gitlab-ci.yml
> +      - br-test-pkg/*/.config
> +      - br-test-pkg/*/missing.config
>
>  buildroot-pipeline:
>    stage: build
> @@ -21,3 +24,5 @@ buildroot-pipeline:
>        - artifact: generated-gitlab-ci.yml
>          job: generate-gitlab-ci-yml
>      strategy: depend
> +  variables:
> +    PARENT_PIPELINE_ID: $CI_PIPELINE_ID
> diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
> index 1ee3772154..be7951b3d2 100644
> --- a/support/misc/gitlab-ci.yml.in
> +++ b/support/misc/gitlab-ci.yml.in
> @@ -80,3 +80,25 @@
>              - test-output/*/.config
>              - test-output/*/images/*
>
> +.test_pkg:
> +    stage: build
> +    before_script:
> +        - OUTPUT_DIR=${CI_JOB_NAME}
> +    script:
> +        - echo "Configure Buildroot for ${OUTPUT_DIR}"
> +        - make O=${OUTPUT_DIR} syncconfig
> +        - make O=${OUTPUT_DIR} savedefconfig
> +        - echo 'Build buildroot'
> +        - *run_make
> +    needs:
> +        - pipeline: $PARENT_PIPELINE_ID
> +          job: generate-gitlab-ci-yml
> +    artifacts:
> +        when: always
> +        expire_in: 2 weeks
> +        paths:
> +            - build.log
> +            - br-test-pkg/*/.config
> +            - br-test-pkg/*/defconfig
> +            - br-test-pkg/*/build/build-time.log
> +            - br-test-pkg/*/build/packages-file-list*.txt
> diff --git a/support/scripts/generate-gitlab-ci-yml b/support/scripts/generate-gitlab-ci-yml
> index 3f498e08fd..063c5081da 100755
> --- a/support/scripts/generate-gitlab-ci-yml
> +++ b/support/scripts/generate-gitlab-ci-yml
> @@ -23,7 +23,7 @@ _EOF_
>
>  gen_tests() {
>      local -a basics defconfigs runtimes
> -    local do_basics do_defconfigs do_runtime
> +    local do_basics do_defconfigs do_runtime do_testpkg
>      local defconfigs_ext cfg tst
>
>      basics=( DEVELOPERS flake8 package )
> @@ -77,9 +77,30 @@ gen_tests() {
>          esac
>      fi
>
> +    # Retrieve defconfig for test-pkg from the git commit message (if any)
> +    if grep -q -E '^test-pkg config:$' <<<"${CI_COMMIT_DESCRIPTION}"; then
> +        sed -r -n -e '/^test-pkg config:$/{:a;n;p;ba;}' \
> +            <<<"${CI_COMMIT_DESCRIPTION}" \
> +            >defconfig.frag
> +        if [ ! -s defconfig.frag ]; then
> +            printf "Empty configuration fragment.\n" >&2; exit 1
> +        fi
> +        # Use --all since we expect the user having already pre-tested the
> +        # new package with the default subset of toolchains.
> +        ./utils/test-pkg \
> +            --all --prepare-only \
> +            --config-snippet defconfig.frag \
> +            --build-dir br-test-pkg >&2
> +        do_testpkg=( $(ls -1 br-test-pkg/*/.config 2>/dev/null |xargs -r dirname ) )
> +        if [ "${#do_testpkg[@]}" -eq 0 ]; then
> +            printf "Configuration fragment enables no test.\n" >&2; exit 1
> +        fi
> +    fi
> +
>      # If nothing else, at least do the basics to generate a valid pipeline
>      if [    -z "${do_defconfigs}" \
>           -a -z "${do_runtime}" \
> +         -a -z "${do_testpkg}" \
>         ]
>      then
>          do_basics=true
> @@ -101,6 +122,10 @@ gen_tests() {
>      if ${do_runtime:-false}; then
>          printf '%s: { extends: .runtime_test_base }\n' "${runtimes[@]}"
>      fi
> +
> +    if [ -n "${do_testpkg}" ]; then
> +        printf '%s: { extends: .test_pkg }\n' "${do_testpkg[@]}"
> +    fi
>  }
>
>  main "${@}"
> --
> 2.25.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/4 v5] utils/test-pkg: add mode to only prepare .config files

[Arnout Vandecappelle]

On 28/06/2021 22:15, Yann E. MORIN wrote:
> Currently, running test-pkg is only done locally on the developpers
> machine.
> 
> In a follow up commit, we'll add the possibility to run test-pkg in a
> gitlab-ci pipeline and, to speed up things, with one job per buildable
> configuration.
> 
> As such, we will need that test-pkg only ever prepares the
> configuration, and that it does not build them.
> 
> Add such a mode, with a new option, --prepare-only
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> ---
> Note: naming is hard; naming options is harder; naming options with a
> terse term is even harder; naming options with a terse term that is
> still meaningful and explains what the option does, is even harder yet.

 But doing it inconsistently is easy (see below). :-)

 Anyway, there's an easy solution to that (which I believe we should apply
here): don't define a terse option.

 I think terse options should only be defined for stuff that a human has to
type. In scripts, terse options shouldn't be used, because it makes it harder
for the programmer to understand what the command does.

 Since prepare-only is meant ot be used by script, I don't think a terse option
is needed.


> 
> ---
> v5: split off from the next patch into this patch
> ---
>  utils/test-pkg | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/test-pkg b/utils/test-pkg
> index 54c6c5e8fe..4a20cab57f 100755
> --- a/utils/test-pkg
> +++ b/utils/test-pkg
> @@ -12,13 +12,13 @@ do_clean() {
>  
>  main() {
>      local o O opts
> -    local cfg dir pkg random toolchains_csv toolchain all number mode
> +    local cfg dir pkg random toolchains_csv toolchain all number mode prepare_only
>      local ret nb nb_skip nb_fail nb_legal nb_tc build_dir keep
>      local -a toolchains
>      local pkg_br_name
>  
> -    o='hakc:d:n:p:r:t:'
> -    O='help,all,keep,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:> +    o='hakgc:d:n:p:r:t:'
             ^ AFAICS this is a 'g'...

> +    O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:'
>      opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")"
>      eval set -- "${opts}"
>  
> @@ -27,6 +27,7 @@ main() {
>      keep=0
>      number=0
>      mode=0
> +    prepare_only=0
>      toolchains_csv="${TOOLCHAINS_CSV}"
>      while [ ${#} -gt 0 ]; do
>          case "${1}" in
> @@ -39,6 +40,9 @@ main() {
>          (-k|--keep)
>              keep=1; shift 1
>              ;;
> +        (-l|--prepare-only)
             ^ ... but this is an 'l'!

 Clearly someone didn't test with the terse option. :-)

> +            prepare_only=1; shift 1
> +            ;;
>          (-c|--config-snippet)
>              cfg="${2}"; shift 2
>              ;;
> @@ -127,7 +131,7 @@ main() {
>          toolchain="$(basename "${toolchainconfig}" .config)"
>          build_dir="${dir}/${toolchain}"
>          printf "%40s [%*d/%d]: " "${toolchain}" ${#nb_tc} ${nb} ${nb_tc}
> -        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" && ret=0 || ret=${?}
> +        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" "${prepare_only}" && ret=0 || ret=${?}
>          case ${ret} in
>          (0) printf "OK\n";;
>          (1) : $((nb_skip++)); printf "SKIPPED\n";;
> @@ -147,6 +151,7 @@ build_one() {
>      local toolchainconfig="${2}"
>      local cfg="${3}"
>      local pkg="${4}"
> +    local defer="${5}"

 I like it if variables that are the same are called the same. I.e., use
prepare_only here as well. Perhaps you can even skip passing it at all since
it's a global variable.

>  
>      mkdir -p "${dir}"
>  
> @@ -170,6 +175,11 @@ build_one() {
>      # Remove file, it's empty anyway.
>      rm -f "${dir}/missing.config"
>  
> +    # Defer building the job to the caller (e.g. a gitlab pipeline)
> +    if [ ${defer} -eq 1 ]; then
> +        return 0
> +    fi
> +
>      if [ -n "${pkg}" ]; then
>          if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
>              return 2
> @@ -257,6 +267,10 @@ Options:
>          Note: the logfile and configuration is always retained, even without
>          this option.
>  
> +    --prepare-only

 And the terse option isn't even in the help! So nobody would use it anyway...


 Regards,
 Arnout

> +        Only prepare the .config files, but do not build them. Output the
> +        list of build directories to stdout, and the status on stderr.
> +
>  Example:
>  
>      Testing libcec would require a config snippet that contains:
> 
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/4 v5] support/misc/gitlab-ci.yml.in: templatise the make command

[Arnout Vandecappelle]

On 28/06/2021 22:15, Yann E. MORIN wrote:
> From: Romain Naour <romain.naour@gmail.com>
> 
> In a followup commit, the make command used to log and display the last
> lines on error will be used in another job.
> Factorize it by introducing .run_make template.
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

 Applied to next, thanks, and  the second one as well.

 Regards,
 Arnout

> ---
>  support/misc/gitlab-ci.yml.in | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
> index fcfff5c6aa..1ee3772154 100644
> --- a/support/misc/gitlab-ci.yml.in
> +++ b/support/misc/gitlab-ci.yml.in
> @@ -26,20 +26,24 @@
>          paths:
>              - .config
>  
> +.run_make: &run_make
> +    |
> +      make O=${OUTPUT_DIR} > >(tee build.log |grep '>>>') 2>&1 || {
> +          echo 'Failed build last output'
> +          tail -200 build.log
> +          exit 1
> +      }
> +
>  .defconfig_base:
>      before_script:
>          - DEFCONFIG_NAME=${CI_JOB_NAME}
> +        - OUTPUT_DIR=output
>      script:
>          - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
>          - make ${DEFCONFIG_NAME}
>          - ./support/scripts/check-dotconfig.py .config ./configs/${DEFCONFIG_NAME}
>          - echo 'Build buildroot'
> -        - |
> -            make > >(tee build.log |grep '>>>') 2>&1 || {
> -                echo 'Failed build last output'
> -                tail -200 build.log
> -                exit 1
> -            }
> +        - *run_make
>          - |
>              ./support/scripts/boot-qemu-image.py "${DEFCONFIG_NAME}" > >(tee runtime-test.log) 2>&1 || {
>                  echo 'Failed runtime test last output'
> 
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/4 v5] utils/test-pkg: add mode to only prepare .config files

[Romain Naour]

Hello Arnout, Yann,

Le 05/08/2021 à 22:45, Arnout Vandecappelle a écrit :
> 
> 
> On 28/06/2021 22:15, Yann E. MORIN wrote:
>> Currently, running test-pkg is only done locally on the developpers
>> machine.
>>
>> In a follow up commit, we'll add the possibility to run test-pkg in a
>> gitlab-ci pipeline and, to speed up things, with one job per buildable
>> configuration.
>>
>> As such, we will need that test-pkg only ever prepares the
>> configuration, and that it does not build them.
>>
>> Add such a mode, with a new option, --prepare-only
>>
>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>> Cc: Romain Naour <romain.naour@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>> ---
>> Note: naming is hard; naming options is harder; naming options with a
>> terse term is even harder; naming options with a terse term that is
>> still meaningful and explains what the option does, is even harder yet.
> 
>  But doing it inconsistently is easy (see below). :-)
> 
>  Anyway, there's an easy solution to that (which I believe we should apply
> here): don't define a terse option.
> 
>  I think terse options should only be defined for stuff that a human has to
> type. In scripts, terse options shouldn't be used, because it makes it harder
> for the programmer to understand what the command does.
> 
>  Since prepare-only is meant ot be used by script, I don't think a terse option
> is needed.

Thanks for your advice. Indeed I don't think we need a terse option.

Best regards,
Romain

> 
> 
>>
>> ---
>> v5: split off from the next patch into this patch
>> ---
>>  utils/test-pkg | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/utils/test-pkg b/utils/test-pkg
>> index 54c6c5e8fe..4a20cab57f 100755
>> --- a/utils/test-pkg
>> +++ b/utils/test-pkg
>> @@ -12,13 +12,13 @@ do_clean() {
>>  
>>  main() {
>>      local o O opts
>> -    local cfg dir pkg random toolchains_csv toolchain all number mode
>> +    local cfg dir pkg random toolchains_csv toolchain all number mode prepare_only
>>      local ret nb nb_skip nb_fail nb_legal nb_tc build_dir keep
>>      local -a toolchains
>>      local pkg_br_name
>>  
>> -    o='hakc:d:n:p:r:t:'
>> -    O='help,all,keep,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:> +    o='hakgc:d:n:p:r:t:'
>              ^ AFAICS this is a 'g'...
> 
>> +    O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:'
>>      opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")"
>>      eval set -- "${opts}"
>>  
>> @@ -27,6 +27,7 @@ main() {
>>      keep=0
>>      number=0
>>      mode=0
>> +    prepare_only=0
>>      toolchains_csv="${TOOLCHAINS_CSV}"
>>      while [ ${#} -gt 0 ]; do
>>          case "${1}" in
>> @@ -39,6 +40,9 @@ main() {
>>          (-k|--keep)
>>              keep=1; shift 1
>>              ;;
>> +        (-l|--prepare-only)
>              ^ ... but this is an 'l'!
> 
>  Clearly someone didn't test with the terse option. :-)
> 
>> +            prepare_only=1; shift 1
>> +            ;;
>>          (-c|--config-snippet)
>>              cfg="${2}"; shift 2
>>              ;;
>> @@ -127,7 +131,7 @@ main() {
>>          toolchain="$(basename "${toolchainconfig}" .config)"
>>          build_dir="${dir}/${toolchain}"
>>          printf "%40s [%*d/%d]: " "${toolchain}" ${#nb_tc} ${nb} ${nb_tc}
>> -        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" && ret=0 || ret=${?}
>> +        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" "${prepare_only}" && ret=0 || ret=${?}
>>          case ${ret} in
>>          (0) printf "OK\n";;
>>          (1) : $((nb_skip++)); printf "SKIPPED\n";;
>> @@ -147,6 +151,7 @@ build_one() {
>>      local toolchainconfig="${2}"
>>      local cfg="${3}"
>>      local pkg="${4}"
>> +    local defer="${5}"
> 
>  I like it if variables that are the same are called the same. I.e., use
> prepare_only here as well. Perhaps you can even skip passing it at all since
> it's a global variable.
> 
>>  
>>      mkdir -p "${dir}"
>>  
>> @@ -170,6 +175,11 @@ build_one() {
>>      # Remove file, it's empty anyway.
>>      rm -f "${dir}/missing.config"
>>  
>> +    # Defer building the job to the caller (e.g. a gitlab pipeline)
>> +    if [ ${defer} -eq 1 ]; then
>> +        return 0
>> +    fi
>> +
>>      if [ -n "${pkg}" ]; then
>>          if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
>>              return 2
>> @@ -257,6 +267,10 @@ Options:
>>          Note: the logfile and configuration is always retained, even without
>>          this option.
>>  
>> +    --prepare-only
> 
>  And the terse option isn't even in the help! So nobody would use it anyway...
> 
> 
>  Regards,
>  Arnout
> 
>> +        Only prepare the .config files, but do not build them. Output the
>> +        list of build directories to stdout, and the status on stderr.
>> +
>>  Example:
>>  
>>      Testing libcec would require a config snippet that contains:
>>

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/4 v5] utils/test-pkg: add mode to only prepare .config files

[Yann E. MORIN]

Romain, Arnout, All,

On 2021-08-21 15:38 +0200, Romain Naour spake thusly:
> Le 05/08/2021 à 22:45, Arnout Vandecappelle a écrit :
> > On 28/06/2021 22:15, Yann E. MORIN wrote:
> >> Currently, running test-pkg is only done locally on the developpers
> >> machine.
> >>
> >> In a follow up commit, we'll add the possibility to run test-pkg in a
> >> gitlab-ci pipeline and, to speed up things, with one job per buildable
> >> configuration.
> >>
> >> As such, we will need that test-pkg only ever prepares the
> >> configuration, and that it does not build them.
> >>
> >> Add such a mode, with a new option, --prepare-only
> >>
> >> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> >> Cc: Romain Naour <romain.naour@gmail.com>
> >> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> >> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> >>
> >> ---
> >> Note: naming is hard; naming options is harder; naming options with a
> >> terse term is even harder; naming options with a terse term that is
> >> still meaningful and explains what the option does, is even harder yet.
> > 
> >  But doing it inconsistently is easy (see below). :-)
> > 
> >  Anyway, there's an easy solution to that (which I believe we should apply
> > here): don't define a terse option.

Ah, but there was a misunderstanding: I was refering to the "long
option" that I tried to keep terse. I.e. I started off with:
    --just-generate-config-for-later-use-in-gitlab-CI-or-anyother-such-CI

and eventually tried to shorten it as much as possible, while still
keeping the meaning, so I ended up with just:
    --prepare-only

As for the short, one-char option, indeed., we don't really need one.

> >  I think terse options should only be defined for stuff that a human has to
> > type. In scripts, terse options shouldn't be used, because it makes it harder
> > for the programmer to understand what the command does.
> > 
> >  Since prepare-only is meant ot be used by script, I don't think a terse option
> > is needed.
> Thanks for your advice. Indeed I don't think we need a terse option.

We need a terse "long option", but we don;t need a one-char "short
option". ;-)

Yes, this is confusing... ;-)

> >> +        (-l|--prepare-only)
> >              ^ ... but this is an 'l'!
> >  Clearly someone didn't test with the terse option. :-)

Yes, I did test with terse "long option", but not with the one-char
"short option". Indeed. ;-]

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot