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

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

[Thomas Petazzoni]

Hello,

During the Buildroot Developers meeting, 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:

 - Fixing genrandconfig and test-pkg to ignore comments and empty
   lines in the toolchain CSV file.

 - 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

Thomas Petazzoni (5):
  utils/genrandconfig: filter empty lines and comments in CSV file
  test-pkg: filter empty lines and comments in CSV file
  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                | 30 ++++++++---
 utils/genrandconfig                                |  5 +-
 utils/test-pkg                                     | 58 +++++++++++++++++++---
 4 files changed, 86 insertions(+), 18 deletions(-)

-- 
2.13.6

[Buildroot] [PATCH 1/5] utils/genrandconfig: filter empty lines and comments in CSV file

[Thomas Petazzoni]

In preparation for the addition of comments in the CSV file listing
toolchain configurations, we filter out such lines when reading the
CSV file in utils/genrandconfig.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 utils/genrandconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/utils/genrandconfig b/utils/genrandconfig
index a67d46fad9..e963349d70 100755
--- a/utils/genrandconfig
+++ b/utils/genrandconfig
@@ -126,7 +126,9 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
     """
 
     with open(toolchains_csv) as r:
-        toolchains = decode_byte_list(r.readlines())
+        # filter empty lines and comments
+        lines = [ t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#' ]
+        toolchains = decode_byte_list(lines)
     configs = []
 
     (_, _, _, _, hostarch) = os.uname()
@@ -164,6 +166,7 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
         with open(configfile) as r:
             config = r.readlines()
         configs.append(config)
+
     return configs
 
 
-- 
2.13.6

[Buildroot] [PATCH 2/5] test-pkg: filter empty lines and comments in CSV file

[Thomas Petazzoni]

In preparation for the addition of comments in the CSV file listing
toolchain configurations, we filter out such lines when reading the
CSV file in utils/test-pkg.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 utils/test-pkg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/test-pkg b/utils/test-pkg
index 6899d44f7c..1b7046eac4 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -54,7 +54,7 @@ main() {
     # 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;' "${toolchains_csv}" \
+    toolchains=($(sed -r -e 's/,.*//; /internal/d; /^#/d; /^$/d;' "${toolchains_csv}" \
                   |if [ ${random} -gt 0 ]; then \
                       sort -R |head -n ${random}
                    else
-- 
2.13.6

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

[Thomas Petazzoni]

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@free-electrons.com>
---
 .../autobuild/toolchain-configs.csv                | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/support/config-fragments/autobuild/toolchain-configs.csv b/support/config-fragments/autobuild/toolchain-configs.csv
index efb78eec4f..53d1d7234a 100644
--- a/support/config-fragments/autobuild/toolchain-configs.csv
+++ b/support/config-fragments/autobuild/toolchain-configs.csv
@@ -1,18 +1,37 @@
+# This file is sorted by "importance" of toolchains, so that by
+# default test-pkg tests a useful subset of toolchains
+
+# 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
+
+# All other toolchains
 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-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
@@ -34,7 +53,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.13.6

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

[Thomas Petazzoni]

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>
---
 utils/test-pkg | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/utils/test-pkg b/utils/test-pkg
index 1b7046eac4..54e5ad8bbb 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -5,16 +5,19 @@ 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='han:c:d: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
@@ -33,6 +36,12 @@ main() {
         (-r|--random)
             random="${2}"; shift 2
             ;;
+	(-a|--all)
+            all=1; shift 1
+            ;;
+	(-n|--number)
+            number="${2}"; shift 2
+            ;;
         (-t|--toolchains-csv)
             toolchains_csv="${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
                  )
                )
 
@@ -156,6 +187,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
@@ -172,9 +207,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.13.6

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

[Thomas Petazzoni]

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 824f07547d..113172d0ab 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.13.6

[Buildroot] [PATCH 1/5] utils/genrandconfig: filter empty lines and comments in CSV file

[Yann E. MORIN]

Thomas, All,

On 2017-10-29 18:14 +0100, Thomas Petazzoni spake thusly:
> In preparation for the addition of comments in the CSV file listing
> toolchain configurations, we filter out such lines when reading the
> CSV file in utils/genrandconfig.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

Small nit, below...

> ---
>  utils/genrandconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/genrandconfig b/utils/genrandconfig
> index a67d46fad9..e963349d70 100755
> --- a/utils/genrandconfig
> +++ b/utils/genrandconfig
> @@ -126,7 +126,9 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
>      """
>  
>      with open(toolchains_csv) as r:
> -        toolchains = decode_byte_list(r.readlines())
> +        # filter empty lines and comments
> +        lines = [ t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#' ]
> +        toolchains = decode_byte_list(lines)
>      configs = []
>  
>      (_, _, _, _, hostarch) = os.uname()
> @@ -164,6 +166,7 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
>          with open(configfile) as r:
>              config = r.readlines()
>          configs.append(config)
> +

Spurious hunk.

Regards,
Yann E. MORIN.

>      return configs
>  
>  
> -- 
> 2.13.6
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 2/5] test-pkg: filter empty lines and comments in CSV file

[Yann E. MORIN]

Thomas, All,

On 2017-10-29 18:14 +0100, Thomas Petazzoni spake thusly:
> In preparation for the addition of comments in the CSV file listing
> toolchain configurations, we filter out such lines when reading the
> CSV file in utils/test-pkg.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

Regards,
Yann E. MORIN.

> ---
>  utils/test-pkg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/test-pkg b/utils/test-pkg
> index 6899d44f7c..1b7046eac4 100755
> --- a/utils/test-pkg
> +++ b/utils/test-pkg
> @@ -54,7 +54,7 @@ main() {
>      # 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;' "${toolchains_csv}" \
> +    toolchains=($(sed -r -e 's/,.*//; /internal/d; /^#/d; /^$/d;' "${toolchains_csv}" \
>                    |if [ ${random} -gt 0 ]; then \
>                        sort -R |head -n ${random}
>                     else
> -- 
> 2.13.6
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 3/5] toolchain-configs.csv: re-organize for test-pkg

[Yann E. MORIN]

Thomas, All,

On 2017-10-29 18:14 +0100, Thomas Petazzoni spake thusly:
> 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.

I would have done something else:

    N:path:arch

where:
  - N is an integer that is the importance of the toolchain, in the
    range [0..9], with 0 the most important and 9 the least important.

 - path and arch as they are today.

So we'd tag the toolchains between 0 and 9, and default to test those <3
for example.

Which allows us to keep the ordering of the toolchain, which is nice.

But I'm also OK with your solution, just suggesting an alternative.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  .../autobuild/toolchain-configs.csv                | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/support/config-fragments/autobuild/toolchain-configs.csv b/support/config-fragments/autobuild/toolchain-configs.csv
> index efb78eec4f..53d1d7234a 100644
> --- a/support/config-fragments/autobuild/toolchain-configs.csv
> +++ b/support/config-fragments/autobuild/toolchain-configs.csv
> @@ -1,18 +1,37 @@
> +# This file is sorted by "importance" of toolchains, so that by
> +# default test-pkg tests a useful subset of toolchains

    # Toolchains used by default:

> +# 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
> +
> +# All other toolchains

    # All other toolchains, not used by default:

Do we want to reorder the remaining list by relevance, too, so that a
-n12 would pick the 12 most relevant toolchains?

Or are we just happy that the 7 first are ordered, and we don't care
about the rest?

I'm fine in either case.

Regards,
Yann E. MORIN.

>  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-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
> @@ -34,7 +53,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.13.6
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 4/5] test-pkg: test a subset of toolchains by default, add -a and -n options

[Yann E. MORIN]

Thomas, All,

On 2017-10-29 18:14 +0100, Thomas Petazzoni spake thusly:
> 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>
> ---
>  utils/test-pkg | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/utils/test-pkg b/utils/test-pkg
> index 1b7046eac4..54e5ad8bbb 100755
> --- a/utils/test-pkg
> +++ b/utils/test-pkg
> @@ -5,16 +5,19 @@ 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='han:c:d:p:r:t:'

Except for 'h' which is in a leading position, all other options where
in alphabetical order.

>      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
> @@ -33,6 +36,12 @@ main() {
>          (-r|--random)
>              random="${2}"; shift 2
>              ;;
> +	(-a|--all)

This script uses leading spaces, not TABs.

And keep options in aphabetical order. ;-)

> +            all=1; shift 1
> +            ;;
> +	(-n|--number)

Ditto.

Regards,
Yann E. MORIN.

> +            number="${2}"; shift 2
> +            ;;
>          (-t|--toolchains-csv)
>              toolchains_csv="${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
>                   )
>                 )
>  
> @@ -156,6 +187,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
> @@ -172,9 +207,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.13.6
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 1/5] utils/genrandconfig: filter empty lines and comments in CSV file

[Thomas Petazzoni]

Hello,

On Sun, 29 Oct 2017 18:14:36 +0100, Thomas Petazzoni wrote:
> In preparation for the addition of comments in the CSV file listing
> toolchain configurations, we filter out such lines when reading the
> CSV file in utils/genrandconfig.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  utils/genrandconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied to next after dropping the spurious change noticed by Yann.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/5] test-pkg: filter empty lines and comments in CSV file

[Thomas Petazzoni]

Hello,

On Sun, 29 Oct 2017 18:14:37 +0100, Thomas Petazzoni wrote:
> In preparation for the addition of comments in the CSV file listing
> toolchain configurations, we filter out such lines when reading the
> CSV file in utils/test-pkg.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  utils/test-pkg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to next, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Thomas Petazzoni]

Hello,

Thanks for the review, finally getting back to this old patch series!

On Sun, 29 Oct 2017 18:35:36 +0100, Yann E. MORIN wrote:

> I would have done something else:
> 
>     N:path:arch
> 
> where:
>   - N is an integer that is the importance of the toolchain, in the
>     range [0..9], with 0 the most important and 9 the least important.
> 
>  - path and arch as they are today.
> 
> So we'd tag the toolchains between 0 and 9, and default to test those <3
> for example.
> 
> Which allows us to keep the ordering of the toolchain, which is nice.
> 
> But I'm also OK with your solution, just suggesting an alternative.

I don't find adding another field really necessary. After all, all we
need is to distinguish the "important" ones from the "other" ones.

>     # Toolchains used by default:

Fixed.

> > +# All other toolchains  
> 
>     # All other toolchains, not used by default:

Fixed.

> Do we want to reorder the remaining list by relevance, too, so that a
> -n12 would pick the 12 most relevant toolchains?
> 
> Or are we just happy that the 7 first are ordered, and we don't care
> about the rest?

I think it will be difficult to define a "relevance order", nobody will
agree on what is the correct order for all those toolchains, and it is
not super important either. I'm not even sure my "-n" argument to
test-pkg makes a lot of sense. Really the "default" or "all" case are
what I really wanted to add.

Best regards,

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