diff mbox

[1/6] support/scripts: add script to test a package

Message ID 085941ea3e9e9f34405ac10780d5c8aa2a6317aa.1486584734.git.yann.morin.1998@free.fr
State Accepted
Headers show

Commit Message

Yann E. MORIN Feb. 8, 2017, 8:15 p.m. UTC
This script helps in testing that a package builds fine on a wide range
of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[yann.morin.1998@free.fr:
 - completely rewrite the script from Thomas, with help from Luca
]
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

---
Changes v2 -> v3:
  - grammar  (Luca)
---
 support/scripts/test-pkg | 168 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100755 support/scripts/test-pkg

Comments

Cam Hutchison Feb. 8, 2017, 10 p.m. UTC | #1
"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

>This script helps in testing that a package builds fine on a wide range
>of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...

>Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>[yann.morin.1998@free.fr:
> - completely rewrite the script from Thomas, with help from Luca
>]
>Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>Cc: Luca Ceresoli <luca@lucaceresoli.net>
>Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
>Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
>Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

>---
>Changes v2 -> v3:
>  - grammar  (Luca)
>---
> support/scripts/test-pkg | 168 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 168 insertions(+)
> create mode 100755 support/scripts/test-pkg

>diff --git a/support/scripts/test-pkg b/support/scripts/test-pkg
>new file mode 100755
>index 0000000..008b32c
>--- /dev/null
>+++ b/support/scripts/test-pkg
>@@ -0,0 +1,168 @@
>+#!/bin/bash
>+set -e
>+
>+TOOLCHAINS_URL='http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv'
>+
>+main() {
>+    local o O opts
>+    local cfg dir pkg toolchain
>+    local -a toolchains
>+
>+    o='hc:d:p:'
>+    O='help,config-snippet:build-dir:package:'
>+    opts="$( getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}"  )"
>+    eval set -- "${opts}"
>+
>+    while [ ${#} -gt 0 ]; do
>+        case "${1}" in
>+        (-h|--help)
>+            help; exit 0
>+            ;;
>+        (-c|--config-snippet)
>+            cfg="${2}"; shift 2
>+            ;;
>+        (-d|--build-dir)
>+            dir="${2}"; shift 2
>+            ;;
>+        (-p|--package)
>+            pkg="${2}"; shift 2
>+            ;;
>+        (--)
>+            shift; break
>+            ;;
>+        esac
>+    done
>+    if [ -z "${cfg}" ]; then
>+        printf "error: no config snippet specified\n" >&2; exit 1
>+    fi
>+    if [ -z "${dir}" ]; then
>+        dir="${HOME}/br-test-pkg"
>+    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=( $( curl -s "${TOOLCHAINS_URL}" \
>+                    |sed -r -e 's/,.*//; /internal/d;'
>+                  )
>+               )
>+
>+    if [ ${#toolchains[@]} -eq 0 ]; then
>+        printf "error: no toolchain found (networking issue?)\n" >&2; exit 1

The format string should be in single quotes as it contains a glob char.

I usually put all printf format strings in single quotes since printf is
doing the substitutions, not the shell.

>+    fi
>+
>+    for toolchain in "${toolchains[@]}"; do
>+        build_one "${dir}" "${toolchain}" "${cfg}" "${pkg}"
>+    done
>+}
>+
>+build_one() {
>+    local dir="${1}"
>+    local url="${2}"
>+    local cfg="${3}"
>+    local pkg="${4}"
>+    local toolchain line
>+
>+    # Using basename(1) on a URL works nicely
>+    toolchain="$( basename "${url}" .config )"
>+
>+    printf "%40s: " "${toolchain}"
>+
>+    dir="${dir}/${toolchain}"
>+    mkdir -p "${dir}"
>+
>+    printf "download config"
>+    if ! curl -s "${url}" >"${dir}/.config"; then
>+        printf ": FAILED\n"
>+        return
>+    fi
>+
>+    cat >>"${dir}/.config" <<-_EOF_
>+	BR2_INIT_NONE=y
>+	BR2_SYSTEM_BIN_SH_NONE=y
>+	# BR2_PACKAGE_BUSYBOX is not set
>+	# BR2_TARGET_ROOTFS_TAR is not set
>+	_EOF_
>+    cat "${cfg}" >>"${dir}/.config"
>+
>+    printf ", olddefconfig"
>+    if ! make O="${dir}" olddefconfig >/dev/null 2>&1; then
>+        printf ": FAILED\n"
>+        return
>+    fi
>+    # We want all the options from the snippet to be present as-is (set
>+    # or not set) in the actual .config; if one of them is not, it means
>+    # some dependency from the toolchain or arch is not available, in
>+    # which case this config is untestable and we skip it.
>+    while read line; do
>+        if ! grep "^${line}\$" "${dir}/.config" >/dev/null 2>&1; then

You should use grep -Fx here since ${line} might contains regex chars:

        if ! grep -Fx "${line}" "${dir}/.config" >/dev/null 2>&1; then

....

>+            printf ", SKIPPED\n"
>+            return
>+        fi
>+    done <"${cfg}"

.... but I'd get rid of the loop altogether and use comm(1); something like:

    if [ -n "$( comm -23 <(sort "${cfg}") <(sort "${dir/.config}") )" ]; then
        printf ", SKIPPED\n"
	return
    fi

>+
>+    if [ -n "${pkg}" ]; then
>+        printf ", dirclean"
>+        if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
>+            printf ": FAILED\n"
>+            return
>+        fi
>+    fi
>+
>+    printf ", build"
>+    # shellcheck disable=SC2086
>+    if ! make O="${dir}" ${pkg} >> "${dir}/logfile" 2>&1; then
>+        printf ": FAILED\n"
>+        return
>+    fi
>+
>+    printf ": OK\n"
>+}
>+
>+help() {
>+    cat <<_EOF_
>+test-pkg: test-build a package against various toolchains and architectures
>+
>+The supplied config snippet is appended to each toolchain config, the
>+resulting configuration is checked to ensure it still contains all options
>+specified in the snippet; if any is missing, the build is skipped, on the
>+assumption that the package under test requires a toolchain or architecture
>+feature that is missing.
>+
>+In case failures are noticed, you can fix the package and just re-run the
>+same command again; it will re-run the test where it failed. If you did
>+specify a package (with -p), the package build dir will be removed first.
>+
>+The list of toolchains is retrieved from the Buildroot autobuilders, available
>+at ${TOOLCHAINS_URL}.
>+
>+Options:
>+
>+    -h, --help
>+        Print this help.
>+
>+    -c CFG, --config-snippet CFG
>+        Use the CFG file as the source for the config snippet. This file
>+        should contain all the config options required to build a package.
>+
>+    -d DIR, --build-dir DIR
>+        Do the builds in directory DIR, one sub-dir per toolchain.
>+
>+    -p PKG, --package PKG
>+        Test-build the package PKG, by running 'make PKG'; if not specified,
>+        just runs 'make'.
>+
>+Example:
>+
>+    Testing libcec would require a config snippet that contains:
>+        BR2_PACKAGE_LIBCEC=y
>+
>+    Testing libcurl with openSSL support would require a snippet such as:
>+        BR2_PACKAGE_OPENSSL=y
>+        BR2_PACKAGE_LIBCURL=y
>+
>+_EOF_
>+}
>+
>+my_name="${0##*/}"
>+main "${@}"
>-- 
>2.7.4

>_______________________________________________
>buildroot mailing list
>buildroot@busybox.net
>http://lists.busybox.net/mailman/listinfo/buildroot
Cam Hutchison Feb. 8, 2017, 10:39 p.m. UTC | #2
On 9 February 2017 at 09:00, Cam Hutchison <camh@xdna.net> wrote:

> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
>
> >This script helps in testing that a package builds fine on a wide range
> >of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
>
> >Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >[yann.morin.1998@free.fr:
> > - completely rewrite the script from Thomas, with help from Luca
> >]
> >Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >Cc: Luca Ceresoli <luca@lucaceresoli.net>
> >Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> >Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> >Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> >---
> >Changes v2 -> v3:
> >  - grammar  (Luca)
> >---
> > support/scripts/test-pkg | 168 ++++++++++++++++++++++++++++++
> +++++++++++++++++
> > 1 file changed, 168 insertions(+)
> > create mode 100755 support/scripts/test-pkg
>
> >diff --git a/support/scripts/test-pkg b/support/scripts/test-pkg
> >new file mode 100755
> >index 0000000..008b32c
> >--- /dev/null
> >+++ b/support/scripts/test-pkg
> >@@ -0,0 +1,168 @@
> >+#!/bin/bash
> >+set -e
> >+
> >+TOOLCHAINS_URL='http://autobuild.buildroot.org/
> toolchains/configs/toolchain-configs.csv'
> >+
> >+main() {
> >+    local o O opts
> >+    local cfg dir pkg toolchain
> >+    local -a toolchains
> >+
> >+    o='hc:d:p:'
> >+    O='help,config-snippet:build-dir:package:'
> >+    opts="$( getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}"  )"
> >+    eval set -- "${opts}"
> >+
> >+    while [ ${#} -gt 0 ]; do
> >+        case "${1}" in
> >+        (-h|--help)
> >+            help; exit 0
> >+            ;;
> >+        (-c|--config-snippet)
> >+            cfg="${2}"; shift 2
> >+            ;;
> >+        (-d|--build-dir)
> >+            dir="${2}"; shift 2
> >+            ;;
> >+        (-p|--package)
> >+            pkg="${2}"; shift 2
> >+            ;;
> >+        (--)
> >+            shift; break
> >+            ;;
> >+        esac
> >+    done
> >+    if [ -z "${cfg}" ]; then
> >+        printf "error: no config snippet specified\n" >&2; exit 1
> >+    fi
> >+    if [ -z "${dir}" ]; then
> >+        dir="${HOME}/br-test-pkg"
> >+    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=( $( curl -s "${TOOLCHAINS_URL}" \
> >+                    |sed -r -e 's/,.*//; /internal/d;'
> >+                  )
> >+               )
> >+
> >+    if [ ${#toolchains[@]} -eq 0 ]; then
> >+        printf "error: no toolchain found (networking issue?)\n" >&2;
> exit 1
>
> The format string should be in single quotes as it contains a glob char.
>

I'm wrong here. Globs don't expand in double-quotes.


> I usually put all printf format strings in single quotes since printf is
> doing the substitutions, not the shell.
>
> >+    fi
> >+
> >+    for toolchain in "${toolchains[@]}"; do
> >+        build_one "${dir}" "${toolchain}" "${cfg}" "${pkg}"
> >+    done
> >+}
> >+
> >+build_one() {
> >+    local dir="${1}"
> >+    local url="${2}"
> >+    local cfg="${3}"
> >+    local pkg="${4}"
> >+    local toolchain line
> >+
> >+    # Using basename(1) on a URL works nicely
> >+    toolchain="$( basename "${url}" .config )"
> >+
> >+    printf "%40s: " "${toolchain}"
> >+
> >+    dir="${dir}/${toolchain}"
> >+    mkdir -p "${dir}"
> >+
> >+    printf "download config"
> >+    if ! curl -s "${url}" >"${dir}/.config"; then
> >+        printf ": FAILED\n"
> >+        return
> >+    fi
> >+
> >+    cat >>"${dir}/.config" <<-_EOF_
> >+      BR2_INIT_NONE=y
> >+      BR2_SYSTEM_BIN_SH_NONE=y
> >+      # BR2_PACKAGE_BUSYBOX is not set
> >+      # BR2_TARGET_ROOTFS_TAR is not set
> >+      _EOF_
> >+    cat "${cfg}" >>"${dir}/.config"
> >+
> >+    printf ", olddefconfig"
> >+    if ! make O="${dir}" olddefconfig >/dev/null 2>&1; then
> >+        printf ": FAILED\n"
> >+        return
> >+    fi
> >+    # We want all the options from the snippet to be present as-is (set
> >+    # or not set) in the actual .config; if one of them is not, it means
> >+    # some dependency from the toolchain or arch is not available, in
> >+    # which case this config is untestable and we skip it.
> >+    while read line; do
> >+        if ! grep "^${line}\$" "${dir}/.config" >/dev/null 2>&1; then
>
> You should use grep -Fx here since ${line} might contains regex chars:
>
>         if ! grep -Fx "${line}" "${dir}/.config" >/dev/null 2>&1; then
>
> ....
>
> >+            printf ", SKIPPED\n"
> >+            return
> >+        fi
> >+    done <"${cfg}"
>
> .... but I'd get rid of the loop altogether and use comm(1); something
> like:
>
>     if [ -n "$( comm -23 <(sort "${cfg}") <(sort "${dir/.config}") )" ];
> then
>         printf ", SKIPPED\n"
>         return
>     fi
>
> >+
> >+    if [ -n "${pkg}" ]; then
> >+        printf ", dirclean"
> >+        if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1;
> then
> >+            printf ": FAILED\n"
> >+            return
> >+        fi
> >+    fi
> >+
> >+    printf ", build"
> >+    # shellcheck disable=SC2086
> >+    if ! make O="${dir}" ${pkg} >> "${dir}/logfile" 2>&1; then
> >+        printf ": FAILED\n"
> >+        return
> >+    fi
> >+
> >+    printf ": OK\n"
> >+}
> >+
> >+help() {
> >+    cat <<_EOF_
> >+test-pkg: test-build a package against various toolchains and
> architectures
> >+
> >+The supplied config snippet is appended to each toolchain config, the
> >+resulting configuration is checked to ensure it still contains all
> options
> >+specified in the snippet; if any is missing, the build is skipped, on the
> >+assumption that the package under test requires a toolchain or
> architecture
> >+feature that is missing.
> >+
> >+In case failures are noticed, you can fix the package and just re-run the
> >+same command again; it will re-run the test where it failed. If you did
> >+specify a package (with -p), the package build dir will be removed first.
> >+
> >+The list of toolchains is retrieved from the Buildroot autobuilders,
> available
> >+at ${TOOLCHAINS_URL}.
> >+
> >+Options:
> >+
> >+    -h, --help
> >+        Print this help.
> >+
> >+    -c CFG, --config-snippet CFG
> >+        Use the CFG file as the source for the config snippet. This file
> >+        should contain all the config options required to build a
> package.
> >+
> >+    -d DIR, --build-dir DIR
> >+        Do the builds in directory DIR, one sub-dir per toolchain.
> >+
> >+    -p PKG, --package PKG
> >+        Test-build the package PKG, by running 'make PKG'; if not
> specified,
> >+        just runs 'make'.
> >+
> >+Example:
> >+
> >+    Testing libcec would require a config snippet that contains:
> >+        BR2_PACKAGE_LIBCEC=y
> >+
> >+    Testing libcurl with openSSL support would require a snippet such as:
> >+        BR2_PACKAGE_OPENSSL=y
> >+        BR2_PACKAGE_LIBCURL=y
> >+
> >+_EOF_
> >+}
> >+
> >+my_name="${0##*/}"
> >+main "${@}"
> >--
> >2.7.4
>
> >_______________________________________________
> >buildroot mailing list
> >buildroot@busybox.net
> >http://lists.busybox.net/mailman/listinfo/buildroot
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Luca Ceresoli Feb. 9, 2017, 1:41 p.m. UTC | #3
Hi,

On 08/02/2017 21:15, Yann E. MORIN wrote:
> This script helps in testing that a package builds fine on a wide range
> of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> [yann.morin.1998@free.fr:
>  - completely rewrite the script from Thomas, with help from Luca
> ]
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> ---
> Changes v2 -> v3:
>   - grammar  (Luca)

There are more changes in reality, not only grammar fixes. So the Acked
and Reviewed-by tags should be removed IMO.

But I'm OK with the new version as well, so:

[tested the exim package with all toolchains, with and without -p,
introducing build, URL and defconfig errors]
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Thomas Petazzoni Feb. 9, 2017, 9:50 p.m. UTC | #4
Hello,

For some reason only your cover letter had PATCHv3, not the patches
themselves. Could you check what happened?

On Wed,  8 Feb 2017 21:15:24 +0100, Yann E. MORIN wrote:
> This script helps in testing that a package builds fine on a wide range
> of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> [yann.morin.1998@free.fr:
>  - completely rewrite the script from Thomas, with help from Luca
> ]
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

First of all, thanks for taking over my initial crappy submission and
making it something production ready. I've applied your patch, but I
nonetheless have a few comments about it. See below.

> +    opts="$( getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}"  )"

I continue to terribly dislike this whole concept of putting a space
after an opening parenthesis and before a closing parenthesis.

I know this is your coding style, but it is *not* the Buildroot coding
style. So I'd appreciate if the shell scripts could also comply with
the Buildroot coding style on this.

> +    printf "download config"

I find all these prints to be really useless, and they clutter the
output for nothing. My original script had a very lean and readable
output:

	<toolchain>:	<status>

and now it's the much more verbose (and useless):

                armv5-ctng-linux-gnueabi: download config, olddefconfig, build: OK
              armv7-ctng-linux-gnueabihf: download config, olddefconfig, build: OK

(which would get even more verbose if I had passed a -p option, since
dirclean would have been added).

I'd really prefer if we got rid of those additional prints, they are
not useful, and make the whole thing less readable.

Thanks!

Thomas
Thomas Petazzoni Feb. 9, 2017, 9:51 p.m. UTC | #5
Hello,

On Wed,  8 Feb 2017 21:15:24 +0100, Yann E. MORIN wrote:
> This script helps in testing that a package builds fine on a wide range
> of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> [yann.morin.1998@free.fr:
>  - completely rewrite the script from Thomas, with help from Luca
> ]
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Another comment: now that the script itself has been merged, adding a
quick note in the manual about it would be nice, so that developers
adding new packages (or bumping versions) can quickly test by using
this script.

Thanks!

Thomas
Yann E. MORIN Feb. 9, 2017, 10 p.m. UTC | #6
Thomas, All,

On 2017-02-09 22:50 +0100, Thomas Petazzoni spake thusly:
> For some reason only your cover letter had PATCHv3, not the patches
> themselves. Could you check what happened?

Yup, I'll check...

> On Wed,  8 Feb 2017 21:15:24 +0100, Yann E. MORIN wrote:
> > This script helps in testing that a package builds fine on a wide range
> > of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > [yann.morin.1998@free.fr:
> >  - completely rewrite the script from Thomas, with help from Luca
> > ]
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Luca Ceresoli <luca@lucaceresoli.net>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> First of all, thanks for taking over my initial crappy submission and
> making it something production ready. I've applied your patch, but I
> nonetheless have a few comments about it. See below.
> 
> > +    opts="$( getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}"  )"
> 
> I continue to terribly dislike this whole concept of putting a space
> after an opening parenthesis and before a closing parenthesis.
> 
> I know this is your coding style, but it is *not* the Buildroot coding
> style. So I'd appreciate if the shell scripts could also comply with
> the Buildroot coding style on this.

Fair enough. I'll post a patch fixing this.

> > +    printf "download config"
> 
> I find all these prints to be really useless, and they clutter the
> output for nothing. My original script had a very lean and readable
> output:
> 
> 	<toolchain>:	<status>
> 
> and now it's the much more verbose (and useless):
> 
>                 armv5-ctng-linux-gnueabi: download config, olddefconfig, build: OK
>               armv7-ctng-linux-gnueabihf: download config, olddefconfig, build: OK
> 
> (which would get even more verbose if I had passed a -p option, since
> dirclean would have been added).
> 
> I'd really prefer if we got rid of those additional prints, they are
> not useful, and make the whole thing less readable.

Well, I like that there is so feedback of what's going on under the
hood...

But I'll send updates to 'fix' that.

Thanks! :-)

Regards,
Yann E. MORIN.

> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Yann E. MORIN Feb. 11, 2017, 4:08 p.m. UTC | #7
Cam, All,

Since you did not Cc me on that mail, I only noticed it now...

On 2017-02-08 22:00 -0000, Cam Hutchison spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
> >This script helps in testing that a package builds fine on a wide range
> >of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
[--SNIP--]
> >+    if [ ${#toolchains[@]} -eq 0 ]; then
> >+        printf "error: no toolchain found (networking issue?)\n" >&2; exit 1
> 
> The format string should be in single quotes as it contains a glob char.

Nope, wildcards are not expanded in double quotes (as you then noticed).

> I usually put all printf format strings in single quotes since printf is
> doing the substitutions, not the shell.

I tend to agree on the principle: format strings should be
single-quoted. But then we have disparate quoting styles, and I don;t
like it. I prefer a single quoting style...

But your point is valid as well.

[--SNIP--]
> >+    while read line; do
> >+        if ! grep "^${line}\$" "${dir}/.config" >/dev/null 2>&1; then
> 
> You should use grep -Fx here since ${line} might contains regex chars:
> 
>         if ! grep -Fx "${line}" "${dir}/.config" >/dev/null 2>&1; then
> 
> ....
> 
> >+            printf ", SKIPPED\n"
> >+            return
> >+        fi
> >+    done <"${cfg}"
> 
> .... but I'd get rid of the loop altogether and use comm(1); something like:
> 
>     if [ -n "$( comm -23 <(sort "${cfg}") <(sort "${dir/.config}") )" ]; then
>         printf ", SKIPPED\n"
> 	return
>     fi

Yup, I'll take a look at using comm instead. Thanks for the tip! :-)

Regards,
Yann E. MORIN.
Thomas De Schampheleire Feb. 11, 2017, 9:19 p.m. UTC | #8
On Sat, Feb 11, 2017 at 5:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Cam, All,
>
> Since you did not Cc me on that mail, I only noticed it now...
>
> On 2017-02-08 22:00 -0000, Cam Hutchison spake thusly:
>> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
>> >This script helps in testing that a package builds fine on a wide range
>> >of architectures and toolchains: BE/LE, 32/64-bit, musl/glibc/uclibc...
> [--SNIP--]
>> >+    if [ ${#toolchains[@]} -eq 0 ]; then
>> >+        printf "error: no toolchain found (networking issue?)\n" >&2; exit 1
>>
>> The format string should be in single quotes as it contains a glob char.
>
> Nope, wildcards are not expanded in double quotes (as you then noticed).
>
>> I usually put all printf format strings in single quotes since printf is
>> doing the substitutions, not the shell.
>
> I tend to agree on the principle: format strings should be
> single-quoted. But then we have disparate quoting styles, and I don;t
> like it. I prefer a single quoting style...
>
> But your point is valid as well.
>
> [--SNIP--]
>> >+    while read line; do
>> >+        if ! grep "^${line}\$" "${dir}/.config" >/dev/null 2>&1; then
>>
>> You should use grep -Fx here since ${line} might contains regex chars:
>>
>>         if ! grep -Fx "${line}" "${dir}/.config" >/dev/null 2>&1; then
>>
>> ....
>>
>> >+            printf ", SKIPPED\n"
>> >+            return
>> >+        fi
>> >+    done <"${cfg}"
>>
>> .... but I'd get rid of the loop altogether and use comm(1); something like:
>>
>>     if [ -n "$( comm -23 <(sort "${cfg}") <(sort "${dir/.config}") )" ]; then
>>         printf ", SKIPPED\n"
>>       return
>>     fi
>
> Yup, I'll take a look at using comm instead. Thanks for the tip! :-)
>

Here is a feature request, but only if it does not add much
complexity: I was expecting the script to accept this:

echo "BR2_PACKAGE_BUSYBOX=y" | support/scripts/test-pkg -p busybox -c -

i.e. let '-' indicate that the config snippet is to be taken from
standard input, a principle supported by many unix tools already.
The 'echo' could be more complex in someone else's use case, for
example a concatenation of several pre-existing snippets.

Best regards,
Thomas
Yann E. MORIN Feb. 11, 2017, 10:28 p.m. UTC | #9
Thomas DS, All,

On 2017-02-11 22:19 +0100, Thomas De Schampheleire spake thusly:
> On Sat, Feb 11, 2017 at 5:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> Here is a feature request, but only if it does not add much
> complexity: I was expecting the script to accept this:
> 
> echo "BR2_PACKAGE_BUSYBOX=y" | support/scripts/test-pkg -p busybox -c -
> 
> i.e. let '-' indicate that the config snippet is to be taken from
> standard input, a principle supported by many unix tools already.
> The 'echo' could be more complex in someone else's use case, for
> example a concatenation of several pre-existing snippets.

Which means that we'd have to manage a temporary file internally. Well,
we can do without it, at the cost of some complexity in the script.

What would be the use-case that would prevent on to do:

    $ printf "Bla\nBle\n" >foo.cfg
    $ ./support/scripts/test-pkg -p busybox -c foo.cfg
    $ rm foo.cfg

instead?

But yes, that's doable. Lt's just post-pone that "enhancement" for
later...

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/support/scripts/test-pkg b/support/scripts/test-pkg
new file mode 100755
index 0000000..008b32c
--- /dev/null
+++ b/support/scripts/test-pkg
@@ -0,0 +1,168 @@ 
+#!/bin/bash
+set -e
+
+TOOLCHAINS_URL='http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv'
+
+main() {
+    local o O opts
+    local cfg dir pkg toolchain
+    local -a toolchains
+
+    o='hc:d:p:'
+    O='help,config-snippet:build-dir:package:'
+    opts="$( getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}"  )"
+    eval set -- "${opts}"
+
+    while [ ${#} -gt 0 ]; do
+        case "${1}" in
+        (-h|--help)
+            help; exit 0
+            ;;
+        (-c|--config-snippet)
+            cfg="${2}"; shift 2
+            ;;
+        (-d|--build-dir)
+            dir="${2}"; shift 2
+            ;;
+        (-p|--package)
+            pkg="${2}"; shift 2
+            ;;
+        (--)
+            shift; break
+            ;;
+        esac
+    done
+    if [ -z "${cfg}" ]; then
+        printf "error: no config snippet specified\n" >&2; exit 1
+    fi
+    if [ -z "${dir}" ]; then
+        dir="${HOME}/br-test-pkg"
+    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=( $( curl -s "${TOOLCHAINS_URL}" \
+                    |sed -r -e 's/,.*//; /internal/d;'
+                  )
+               )
+
+    if [ ${#toolchains[@]} -eq 0 ]; then
+        printf "error: no toolchain found (networking issue?)\n" >&2; exit 1
+    fi
+
+    for toolchain in "${toolchains[@]}"; do
+        build_one "${dir}" "${toolchain}" "${cfg}" "${pkg}"
+    done
+}
+
+build_one() {
+    local dir="${1}"
+    local url="${2}"
+    local cfg="${3}"
+    local pkg="${4}"
+    local toolchain line
+
+    # Using basename(1) on a URL works nicely
+    toolchain="$( basename "${url}" .config )"
+
+    printf "%40s: " "${toolchain}"
+
+    dir="${dir}/${toolchain}"
+    mkdir -p "${dir}"
+
+    printf "download config"
+    if ! curl -s "${url}" >"${dir}/.config"; then
+        printf ": FAILED\n"
+        return
+    fi
+
+    cat >>"${dir}/.config" <<-_EOF_
+	BR2_INIT_NONE=y
+	BR2_SYSTEM_BIN_SH_NONE=y
+	# BR2_PACKAGE_BUSYBOX is not set
+	# BR2_TARGET_ROOTFS_TAR is not set
+	_EOF_
+    cat "${cfg}" >>"${dir}/.config"
+
+    printf ", olddefconfig"
+    if ! make O="${dir}" olddefconfig >/dev/null 2>&1; then
+        printf ": FAILED\n"
+        return
+    fi
+    # We want all the options from the snippet to be present as-is (set
+    # or not set) in the actual .config; if one of them is not, it means
+    # some dependency from the toolchain or arch is not available, in
+    # which case this config is untestable and we skip it.
+    while read line; do
+        if ! grep "^${line}\$" "${dir}/.config" >/dev/null 2>&1; then
+            printf ", SKIPPED\n"
+            return
+        fi
+    done <"${cfg}"
+
+    if [ -n "${pkg}" ]; then
+        printf ", dirclean"
+        if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
+            printf ": FAILED\n"
+            return
+        fi
+    fi
+
+    printf ", build"
+    # shellcheck disable=SC2086
+    if ! make O="${dir}" ${pkg} >> "${dir}/logfile" 2>&1; then
+        printf ": FAILED\n"
+        return
+    fi
+
+    printf ": OK\n"
+}
+
+help() {
+    cat <<_EOF_
+test-pkg: test-build a package against various toolchains and architectures
+
+The supplied config snippet is appended to each toolchain config, the
+resulting configuration is checked to ensure it still contains all options
+specified in the snippet; if any is missing, the build is skipped, on the
+assumption that the package under test requires a toolchain or architecture
+feature that is missing.
+
+In case failures are noticed, you can fix the package and just re-run the
+same command again; it will re-run the test where it failed. If you did
+specify a package (with -p), the package build dir will be removed first.
+
+The list of toolchains is retrieved from the Buildroot autobuilders, available
+at ${TOOLCHAINS_URL}.
+
+Options:
+
+    -h, --help
+        Print this help.
+
+    -c CFG, --config-snippet CFG
+        Use the CFG file as the source for the config snippet. This file
+        should contain all the config options required to build a package.
+
+    -d DIR, --build-dir DIR
+        Do the builds in directory DIR, one sub-dir per toolchain.
+
+    -p PKG, --package PKG
+        Test-build the package PKG, by running 'make PKG'; if not specified,
+        just runs 'make'.
+
+Example:
+
+    Testing libcec would require a config snippet that contains:
+        BR2_PACKAGE_LIBCEC=y
+
+    Testing libcurl with openSSL support would require a snippet such as:
+        BR2_PACKAGE_OPENSSL=y
+        BR2_PACKAGE_LIBCURL=y
+
+_EOF_
+}
+
+my_name="${0##*/}"
+main "${@}"