[Buildroot] [PATCH v2 0/3] test-pkg: by default only test a subset of toolchains

[Buildroot] [PATCH v2 0/3] test-pkg: by default only test a subset of toolchains

[Thomas Petazzoni]

During the Buildroot Developers meeting at ELCE 2017, we discussed how
to make test-pkg more usable for our contributors. Our idea is that
test-pkg should not test all toolchains by default, but only a smaller
subset of "interesting" toolchains, that exhibit the most common
problems we encounter with new packages.

This series does that by:

 - Adjust the toolchain CSV file to have the interesting toolchains
   first, adding comments and empty lines along the way to clarify
   what is going on.

 - Update test-pkg to test only the first 7 toolchains, add a --all
   option to test all toolchains, and a --number option to test only
   the first N toolchains.

 - Update the Buildroot manual accordingly.

Thanks!

Thomas

Changes since v1:

 - Drop the first two patches, since they have been merged.

 - In the CSV list of toolchains, add better comments to explain the
   two "sections" of the toolchain list, as suggested by Yann
   E. Morin.

 - In the test-pkg script changes themselves:

    - Keep alphabetic sorting in the definition of the 'o' variable that
      contains the list of short options. Suggested by Yann E. Morin. 'h'
      for help remains a special case and therefore remains at the first
      position.
    - Keep alphabetic sorting when handling options. Here as well, --help
      remains first, as it already was. Suggested by Yann E. Morin.
    - Use spaces and not tabs for indentation. Suggested by Yann E. Morin.

Thomas Petazzoni (3):
  toolchain-configs.csv: re-organize for test-pkg
  test-pkg: test a subset of toolchains by default, add -a and -n
    options
  docs/manual: update the documentation about test-pkg

 docs/manual/adding-packages-tips.txt               | 11 +++--
 .../autobuild/toolchain-configs.csv                | 32 ++++++++++---
 utils/test-pkg                                     | 56 +++++++++++++++++++---
 3 files changed, 83 insertions(+), 16 deletions(-)

-- 
2.14.3

[Buildroot] [PATCH v2 1/3] toolchain-configs.csv: re-organize for test-pkg

[Thomas Petazzoni]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

This commit reorganizes the toolchain-configs.csv so that the first
toolchains are a subset of "useful" toolchains to be tested by
contributors to validate a package. This subset is the one that will
be used by default by test-pkg.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes since v1:
- Add better comments to explain the two "sections" of the toolchain
  list, as suggested by Yann E. Morin.
---
 .../autobuild/toolchain-configs.csv                | 32 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/support/config-fragments/autobuild/toolchain-configs.csv b/support/config-fragments/autobuild/toolchain-configs.csv
index 2010113f44..1bce7ac577 100644
--- a/support/config-fragments/autobuild/toolchain-configs.csv
+++ b/support/config-fragments/autobuild/toolchain-configs.csv
@@ -1,20 +1,41 @@
+# This file is sorted by "importance" of toolchains, so that by
+# default test-pkg tests a useful subset of toolchains
+
+# Toolchains used by default by test-pkg:
+
+# Test a regular uClibc toolchain
+support/config-fragments/autobuild/br-arm-full.config,x86_64
+
+# Test a toolchain with glibc and a very recent gcc version
+support/config-fragments/autobuild/br-arm-cortex-a9-glibc.config,x86_64
+
+# Test a noMMU toolchain with no dynamic library support
+support/config-fragments/autobuild/br-arm-cortex-m4-full.config,x86_64
+
+# Test a musl toolchain
+support/config-fragments/autobuild/br-x86-64-musl.config,x86_64
+
+# Test a noMMU toolchain with dynamic library support
+support/config-fragments/autobuild/br-bfin-full.config,x86_64
+
+# Test a MMU toolchain without dynamic library support
+support/config-fragments/autobuild/br-arm-full-static.config,x86_64
+
+# Test a toolchain with an old gcc version (gcc 4.8)
 support/config-fragments/autobuild/armv5-ctng-linux-gnueabi.config,x86
+
+# Toolchains used by test-pkg only when the '-a' option is passed:
 support/config-fragments/autobuild/armv7-ctng-linux-gnueabihf.config,x86
 support/config-fragments/autobuild/br-aarch64-glibc.config,x86_64
 support/config-fragments/autobuild/br-arc-full-internal.config,any
 support/config-fragments/autobuild/br-arc-internal-glibc.config,any
 support/config-fragments/autobuild/br-arcle-hs38.config,x86_64
 support/config-fragments/autobuild/br-arm-basic.config,x86_64
-support/config-fragments/autobuild/br-arm-cortex-a9-glibc.config,x86_64
 support/config-fragments/autobuild/br-arm-cortex-a9-musl.config,x86_64
-support/config-fragments/autobuild/br-arm-cortex-m4-full.config,x86_64
-support/config-fragments/autobuild/br-arm-full.config,x86_64
 support/config-fragments/autobuild/br-arm-full-nothread.config,x86_64
-support/config-fragments/autobuild/br-arm-full-static.config,x86_64
 support/config-fragments/autobuild/br-arm-internal-full.config,any
 support/config-fragments/autobuild/br-arm-internal-glibc.config,any
 support/config-fragments/autobuild/br-arm-internal-musl.config,any
-support/config-fragments/autobuild/br-bfin-full.config,x86_64
 support/config-fragments/autobuild/br-i386-pentium4-full.config,x86_64
 support/config-fragments/autobuild/br-i386-pentium-mmx-musl.config,x86_64
 support/config-fragments/autobuild/br-m68k-5208-full.config,x86_64
@@ -36,7 +57,6 @@ support/config-fragments/autobuild/br-sh4-full.config,x86_64
 support/config-fragments/autobuild/br-sparc-uclibc.config,x86_64
 support/config-fragments/autobuild/br-sparc64-glibc.config,x86_64
 support/config-fragments/autobuild/br-x86-64-core2-full.config,x86_64
-support/config-fragments/autobuild/br-x86-64-musl.config,x86_64
 support/config-fragments/autobuild/br-xtensa-full.config,x86_64
 support/config-fragments/autobuild/br-xtensa-full-internal.config,any
 support/config-fragments/autobuild/i686-ctng-linux-gnu.config,x86
-- 
2.14.3

[Buildroot] [PATCH v2 2/3] test-pkg: test a subset of toolchains by default, add -a and -n options

[Thomas Petazzoni]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

During the latest Buildroot Developers meeting, we discussed that
test-pkg would perhaps be more widely used if it tested a smaller
subset of toolchains. Indeed, it currently tests 47 toolchains, which
takes very long to build. Several of the toolchain configurations are
quite similar, and it is perhaps not necessary for contributors to
test them all before submitting a package.

Therefore, this commit changes the test-pkg script to only test a
subset of the toolchain configurations by default. The N first
configurations of the CSV files are tested, where N is hard-coded in
the script. The CSV file has therefore been re-organized to have the
first N toolchains be the most important ones.

A -a/--all option is added to test with all toolchains, while a
-n/--number option is added to test with the first N toolchains, N
being passed on the command line.

Note that the list of toolchains (built in the "toolchains" shell
variable) is no longer sorted. Indeed, when the first N toolchains are
tested, we want them to be tested in the same order as they are listed
in the CSV file, as we are careful to order them in an interesting
order. We only sort when all toolchains are tested.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v1:
- Keep alphabetic sorting in the definition of the 'o' variable that
  contains the list of short options. Suggested by Yann E. Morin. 'h'
  for help remains a special case and therefore remains at the first
  position.
- Keep alphabetic sorting when handling options. Here as well, --help
  remains first, as it already was. Suggested by Yann E. Morin.
- Use spaces and not tabs for indentation. Suggested by Yann E. Morin.
---
 utils/test-pkg | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/utils/test-pkg b/utils/test-pkg
index cd7b9dad7b..2e069b6005 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -5,28 +5,37 @@ TOOLCHAINS_CSV='support/config-fragments/autobuild/toolchain-configs.csv'
 
 main() {
     local o O opts
-    local cfg dir pkg random toolchains_dir toolchain
+    local cfg dir pkg random toolchains_dir toolchain all number mode
     local ret nb nb_skip nb_fail nb_legal nb_tc build_dir
     local -a toolchains
 
-    o='hc:d:p:r:t:'
+    o='hac:d:n:p:r:t:'
     O='help,config-snippet:build-dir:package:,random:,toolchains-dir:'
     opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")"
     eval set -- "${opts}"
 
     random=0
+    all=0
+    number=0
+    mode=0
     toolchains_csv="${TOOLCHAINS_CSV}"
     while [ ${#} -gt 0 ]; do
         case "${1}" in
         (-h|--help)
             help; exit 0
             ;;
+        (-a|--all)
+            all=1; shift 1
+            ;;
         (-c|--config-snippet)
             cfg="${2}"; shift 2
             ;;
         (-d|--build-dir)
             dir="${2}"; shift 2
             ;;
+        (-n|--number)
+            number="${2}"; shift 2
+            ;;
         (-p|--package)
             pkg="${2}"; shift 2
             ;;
@@ -51,15 +60,37 @@ main() {
         dir="${HOME}/br-test-pkg"
     fi
 
+    if [ ${random} -gt 0 ]; then
+        mode=$((mode+1))
+    fi
+
+    if [ ${number} -gt 0 ]; then
+        mode=$((mode+1))
+    fi
+
+    if [ ${all} -eq 1 ]; then
+        mode=$((mode+1))
+    fi
+
+    # Default mode is to test the N first toolchains, which have been
+    # chosen to be a good selection of toolchains.
+    if [ ${mode} -eq 0 ] ; then
+        number=7
+    elif [ ${mode} -gt 1 ] ; then
+        printf "error: --all, --number and --random are mutually exclusive\n" >&2; exit 1
+    fi
+
     # Extract the URLs of the toolchains; drop internal toolchains
     # E.g.: http://server/path/to/name.config,arch,libc
     #  -->  http://server/path/to/name.config
     toolchains=($(sed -r -e 's/,.*//; /internal/d; /^#/d; /^$/d;' "${toolchains_csv}" \
                   |if [ ${random} -gt 0 ]; then \
                       sort -R |head -n ${random}
-                   else
-                      cat
-                   fi |sort
+                  elif [ ${number} -gt 0 ]; then \
+                      head -n ${number}
+                  else
+                      sort
+                  fi
                  )
                )
 
@@ -154,6 +185,10 @@ toolchain config fragment and the required host architecture, separated by a
 comma. The config fragments should contain only the toolchain and architecture
 settings.
 
+By default, a useful subset of toolchains is tested. If needed, all
+toolchains can be tested (-a), an arbitrary number of toolchains (-n
+in order, -r for random).
+
 Options:
 
     -h, --help
@@ -170,9 +205,16 @@ Options:
         Test-build the package PKG, by running 'make PKG'; if not specified,
         just runs 'make'.
 
+    -a, --all
+        Test all toolchains, instead of the default subset defined by
+        Buildroot developers.
+
+    -n N, --number N
+        Test N toolchains, in the order defined in the toolchain CSV
+        file.
+
     -r N, --random N
-        Limit the tests to the N randomly selected toolchains, instead of
-        building with all toolchains.
+        Limit the tests to the N randomly selected toolchains.
 
     -t CSVFILE, --toolchains-csv CSVFILE
         CSV file containing the paths to config fragments of toolchains to
-- 
2.14.3

[Buildroot] [PATCH v2 3/3] docs/manual: update the documentation about test-pkg

[Thomas Petazzoni]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 docs/manual/adding-packages-tips.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/docs/manual/adding-packages-tips.txt b/docs/manual/adding-packages-tips.txt
index 19577fa821..f37f9874e7 100644
--- a/docs/manual/adding-packages-tips.txt
+++ b/docs/manual/adding-packages-tips.txt
@@ -77,9 +77,14 @@ and what package to test:
 $ ./utils/test-pkg -c libcurl.config -p libcurl
 ----
 
-This will try to build your package against all the toolchains used
-by the autobuilders (except for the internal toolchains, because it takes
-too long to do so). The output lists all toolchains and the corresponding
+By default, +test-pkg+ will build your package against a subset of the
+toolchains used by the autobuilders, which has been selected by the
+Buildroot developers as being the most useful and representative
+subset. If you want to test all toolchains, pass the +-a+ option. Note
+that in any case, internal toolchains are excluded as they take too
+long to build.
+
+The output lists all toolchains that are tested and the corresponding
 result (excerpt, results are fake):
 
 ----
-- 
2.14.3

[Buildroot] [PATCH v2 2/3] test-pkg: test a subset of toolchains by default, add -a and -n options

[Matthew Weber]

Thomas,

On Fri, Mar 23, 2018 at 4:48 PM, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> During the latest Buildroot Developers meeting, we discussed that
> test-pkg would perhaps be more widely used if it tested a smaller
> subset of toolchains. Indeed, it currently tests 47 toolchains, which
> takes very long to build. Several of the toolchain configurations are
> quite similar, and it is perhaps not necessary for contributors to
> test them all before submitting a package.
>
> Therefore, this commit changes the test-pkg script to only test a
> subset of the toolchain configurations by default. The N first
> configurations of the CSV files are tested, where N is hard-coded in
> the script. The CSV file has therefore been re-organized to have the
> first N toolchains be the most important ones.
>
> A -a/--all option is added to test with all toolchains, while a
> -n/--number option is added to test with the first N toolchains, N
> being passed on the command line.
>
> Note that the list of toolchains (built in the "toolchains" shell
> variable) is no longer sorted. Indeed, when the first N toolchains are
> tested, we want them to be tested in the same order as they are listed
> in the CSV file, as we are careful to order them in an interesting
> order. We only sort when all toolchains are tested.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

I tested a basic one package build with -a, -n 2 and also the no arg
(default) number of toolchains.

Tested-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH v2 1/3] toolchain-configs.csv: re-organize for test-pkg

[Yann E. MORIN]

Thomas, All,

On 2018-03-23 22:48 +0100, Thomas Petazzoni spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> This commit reorganizes the toolchain-configs.csv so that the first
> toolchains are a subset of "useful" toolchains to be tested by
> contributors to validate a package. This subset is the one that will
> be used by default by test-pkg.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
> Changes since v1:
> - Add better comments to explain the two "sections" of the toolchain
>   list, as suggested by Yann E. Morin.
> ---
>  .../autobuild/toolchain-configs.csv                | 32 ++++++++++++++++++----
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/support/config-fragments/autobuild/toolchain-configs.csv b/support/config-fragments/autobuild/toolchain-configs.csv
> index 2010113f44..1bce7ac577 100644
> --- a/support/config-fragments/autobuild/toolchain-configs.csv
> +++ b/support/config-fragments/autobuild/toolchain-configs.csv
> @@ -1,20 +1,41 @@
> +# This file is sorted by "importance" of toolchains, so that by
> +# default test-pkg tests a useful subset of toolchains
> +
> +# Toolchains used by default by test-pkg:
> +
> +# Test a regular uClibc toolchain
> +support/config-fragments/autobuild/br-arm-full.config,x86_64
> +
> +# Test a toolchain with glibc and a very recent gcc version
> +support/config-fragments/autobuild/br-arm-cortex-a9-glibc.config,x86_64
> +
> +# Test a noMMU toolchain with no dynamic library support
> +support/config-fragments/autobuild/br-arm-cortex-m4-full.config,x86_64
> +
> +# Test a musl toolchain
> +support/config-fragments/autobuild/br-x86-64-musl.config,x86_64
> +
> +# Test a noMMU toolchain with dynamic library support
> +support/config-fragments/autobuild/br-bfin-full.config,x86_64
> +
> +# Test a MMU toolchain without dynamic library support
> +support/config-fragments/autobuild/br-arm-full-static.config,x86_64
> +
> +# Test a toolchain with an old gcc version (gcc 4.8)
>  support/config-fragments/autobuild/armv5-ctng-linux-gnueabi.config,x86
> +
> +# Toolchains used by test-pkg only when the '-a' option is passed:
>  support/config-fragments/autobuild/armv7-ctng-linux-gnueabihf.config,x86
>  support/config-fragments/autobuild/br-aarch64-glibc.config,x86_64
>  support/config-fragments/autobuild/br-arc-full-internal.config,any
>  support/config-fragments/autobuild/br-arc-internal-glibc.config,any
>  support/config-fragments/autobuild/br-arcle-hs38.config,x86_64
>  support/config-fragments/autobuild/br-arm-basic.config,x86_64
> -support/config-fragments/autobuild/br-arm-cortex-a9-glibc.config,x86_64
>  support/config-fragments/autobuild/br-arm-cortex-a9-musl.config,x86_64
> -support/config-fragments/autobuild/br-arm-cortex-m4-full.config,x86_64
> -support/config-fragments/autobuild/br-arm-full.config,x86_64
>  support/config-fragments/autobuild/br-arm-full-nothread.config,x86_64
> -support/config-fragments/autobuild/br-arm-full-static.config,x86_64
>  support/config-fragments/autobuild/br-arm-internal-full.config,any
>  support/config-fragments/autobuild/br-arm-internal-glibc.config,any
>  support/config-fragments/autobuild/br-arm-internal-musl.config,any
> -support/config-fragments/autobuild/br-bfin-full.config,x86_64
>  support/config-fragments/autobuild/br-i386-pentium4-full.config,x86_64
>  support/config-fragments/autobuild/br-i386-pentium-mmx-musl.config,x86_64
>  support/config-fragments/autobuild/br-m68k-5208-full.config,x86_64
> @@ -36,7 +57,6 @@ support/config-fragments/autobuild/br-sh4-full.config,x86_64
>  support/config-fragments/autobuild/br-sparc-uclibc.config,x86_64
>  support/config-fragments/autobuild/br-sparc64-glibc.config,x86_64
>  support/config-fragments/autobuild/br-x86-64-core2-full.config,x86_64
> -support/config-fragments/autobuild/br-x86-64-musl.config,x86_64
>  support/config-fragments/autobuild/br-xtensa-full.config,x86_64
>  support/config-fragments/autobuild/br-xtensa-full-internal.config,any
>  support/config-fragments/autobuild/i686-ctng-linux-gnu.config,x86
> -- 
> 2.14.3
> 

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

[Buildroot] [PATCH v2 3/3] docs/manual: update the documentation about test-pkg

[Yann E. MORIN]

Thomas, All,

On 2018-03-23 22:48 +0100, Thomas Petazzoni spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  docs/manual/adding-packages-tips.txt | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-tips.txt b/docs/manual/adding-packages-tips.txt
> index 19577fa821..f37f9874e7 100644
> --- a/docs/manual/adding-packages-tips.txt
> +++ b/docs/manual/adding-packages-tips.txt
> @@ -77,9 +77,14 @@ and what package to test:
>  $ ./utils/test-pkg -c libcurl.config -p libcurl
>  ----
>  
> -This will try to build your package against all the toolchains used
> -by the autobuilders (except for the internal toolchains, because it takes
> -too long to do so). The output lists all toolchains and the corresponding
> +By default, +test-pkg+ will build your package against a subset of the
> +toolchains used by the autobuilders, which has been selected by the
> +Buildroot developers as being the most useful and representative
> +subset. If you want to test all toolchains, pass the +-a+ option. Note
> +that in any case, internal toolchains are excluded as they take too
> +long to build.
> +
> +The output lists all toolchains that are tested and the corresponding
>  result (excerpt, results are fake):
>  
>  ----
> -- 
> 2.14.3
> 

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

[Buildroot] [PATCH v2 2/3] test-pkg: test a subset of toolchains by default, add -a and -n options

[Yann E. MORIN]

Thomas, All,

On 2018-03-23 22:48 +0100, Thomas Petazzoni spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> During the latest Buildroot Developers meeting, we discussed that
> test-pkg would perhaps be more widely used if it tested a smaller
> subset of toolchains. Indeed, it currently tests 47 toolchains, which
> takes very long to build. Several of the toolchain configurations are
> quite similar, and it is perhaps not necessary for contributors to
> test them all before submitting a package.
> 
> Therefore, this commit changes the test-pkg script to only test a
> subset of the toolchain configurations by default. The N first
> configurations of the CSV files are tested, where N is hard-coded in
> the script. The CSV file has therefore been re-organized to have the
> first N toolchains be the most important ones.
> 
> A -a/--all option is added to test with all toolchains, while a
> -n/--number option is added to test with the first N toolchains, N
> being passed on the command line.
> 
> Note that the list of toolchains (built in the "toolchains" shell
> variable) is no longer sorted. Indeed, when the first N toolchains are
> tested, we want them to be tested in the same order as they are listed
> in the CSV file, as we are careful to order them in an interesting
> order. We only sort when all toolchains are tested.

I don't understand why you want to test them in the order they are
listed... I understand that you want them to be _selected_ in the order
they appear, yes. But tested? See below...

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Except for a small nit [0]:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[--SNIP--]
>      # Extract the URLs of the toolchains; drop internal toolchains
>      # E.g.: http://server/path/to/name.config,arch,libc
>      #  -->  http://server/path/to/name.config
>      toolchains=($(sed -r -e 's/,.*//; /internal/d; /^#/d; /^$/d;' "${toolchains_csv}" \
>                    |if [ ${random} -gt 0 ]; then \
>                        sort -R |head -n ${random}
> -                   else
> -                      cat
> -                   fi |sort
> +                  elif [ ${number} -gt 0 ]; then \

[0] BTW, no need for a trailing '\' here...

> +                      head -n ${number}
> +                  else
> +                      sort
> +                  fi

We could always keep the previous trick to sort the selected toolchains;

    |if [ ${random} -gt 0 ]; then \
        sort -R |head -n ${random} 
     elif [ ${number} -gt 0 ]; then
        head -n ${number}
     else
        cat
     fi |sort

This way, the toolchains are selected in the order they appear in the
file, they are always tested in alphabetical order, like it was before
that patch.

But I won't let this prevent the merge, so my Reviewed-by stands.

Regards,
Yann E. MORIN.

>  
> @@ -154,6 +185,10 @@ toolchain config fragment and the required host architecture, separated by a
>  comma. The config fragments should contain only the toolchain and architecture
>  settings.
>  
> +By default, a useful subset of toolchains is tested. If needed, all
> +toolchains can be tested (-a), an arbitrary number of toolchains (-n
> +in order, -r for random).
> +
>  Options:
>  
>      -h, --help
> @@ -170,9 +205,16 @@ Options:
>          Test-build the package PKG, by running 'make PKG'; if not specified,
>          just runs 'make'.
>  
> +    -a, --all
> +        Test all toolchains, instead of the default subset defined by
> +        Buildroot developers.
> +
> +    -n N, --number N
> +        Test N toolchains, in the order defined in the toolchain CSV
> +        file.
> +
>      -r N, --random N
> -        Limit the tests to the N randomly selected toolchains, instead of
> -        building with all toolchains.
> +        Limit the tests to the N randomly selected toolchains.
>  
>      -t CSVFILE, --toolchains-csv CSVFILE
>          CSV file containing the paths to config fragments of toolchains to
> -- 
> 2.14.3
> 

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

[Buildroot] [PATCH v2 2/3] test-pkg: test a subset of toolchains by default, add -a and -n options

[Thomas Petazzoni]

Hello,

On Sun, 25 Mar 2018 10:50:18 +0200, Yann E. MORIN wrote:

> > Note that the list of toolchains (built in the "toolchains" shell
> > variable) is no longer sorted. Indeed, when the first N toolchains are
> > tested, we want them to be tested in the same order as they are listed
> > in the CSV file, as we are careful to order them in an interesting
> > order. We only sort when all toolchains are tested.  
> 
> I don't understand why you want to test them in the order they are
> listed... I understand that you want them to be _selected_ in the order
> they appear, yes. But tested? See below...

Well, the first 7 toolchains are kind of listed in the order of
importance, so it makes more sense to *test* the most important
toolchain first, so if it fails, you can abort there, investigate, and
then restart the test.

But that's not a strong opinion, so if you disagree with this, I don't
care that much if the toolchains are tested in alphabetic ordering.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 2/3] test-pkg: test a subset of toolchains by default, add -a and -n options

[Yann E. MORIN]

Thomas, All,

On 2018-03-25 21:06 +0200, Thomas Petazzoni spake thusly:
> On Sun, 25 Mar 2018 10:50:18 +0200, Yann E. MORIN wrote:
> > > Note that the list of toolchains (built in the "toolchains" shell
> > > variable) is no longer sorted. Indeed, when the first N toolchains are
> > > tested, we want them to be tested in the same order as they are listed
> > > in the CSV file, as we are careful to order them in an interesting
> > > order. We only sort when all toolchains are tested.  
> > 
> > I don't understand why you want to test them in the order they are
> > listed... I understand that you want them to be _selected_ in the order
> > they appear, yes. But tested? See below...
> 
> Well, the first 7 toolchains are kind of listed in the order of
> importance, so it makes more sense to *test* the most important
> toolchain first, so if it fails, you can abort there, investigate, and
> then restart the test.

Well, usually I would expect people to fire-and-forget the build, and
check later what is going on. This is not typically something you gaze
at while it is running...

Maybe we should be using that time to plot a Julia like the cool kids?  ;-]

> But that's not a strong opinion, so if you disagree with this, I don't
> care that much if the toolchains are tested in alphabetic ordering.

Well, as I said, not a big deal for me either.

I just like that things are consistent, and that, to the user, it looks
nicer to have something that is always sorted, because it is "nice".

With a very personal definition of "nice", that is! ;-]

So, go ahead: if there is no other feedback that requires a respin, just
apply it as-is.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v2 1/3] toolchain-configs.csv: re-organize for test-pkg

[Thomas Petazzoni]

Hello,

On Fri, 23 Mar 2018 22:48:13 +0100, Thomas Petazzoni wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> This commit reorganizes the toolchain-configs.csv so that the first
> toolchains are a subset of "useful" toolchains to be tested by
> contributors to validate a package. This subset is the one that will
> be used by default by test-pkg.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> Changes since v1:
> - Add better comments to explain the two "sections" of the toolchain
>   list, as suggested by Yann E. Morin.
> ---
>  .../autobuild/toolchain-configs.csv                | 32 ++++++++++++++++++----
>  1 file changed, 26 insertions(+), 6 deletions(-)

Series applied to master.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com