[Buildroot] [PATCH 0/3] introduce maintainers (branch yem/maintainers)

[Buildroot] [PATCH 0/3] introduce maintainers (branch yem/maintainers)

[Yann E. MORIN]

Hello All!

This small series introduces the notion of maintainers.

We introduce a top-level MAINTAINERS file, not unlike the Linux kernel,
but sorted by people rather than topics; more than one person may
register as maintainer for a specific topic. Topics are patterns that
refer to any file or directory in the tree; see the first patch and the
content of MAINTAINERS for how it is formatted.

We also introduce a script to help find a maintainer for a pacakge or an
architecutre, which can get its input via:
  - command-line arguments '-p PKG' or '-a ARCH',
  - an autobuild result "-b SHA1" or "-b URL",
  - a patch on stdin.

And I added myself as a maintainer for some stuff.

Regards,
Yann E. MORIN.


The following changes since commit e16796942a6f68391e69961e5045ce20b56063e2:

  attr: backport upstream patches to fix build on musl (2015-03-21 10:13:51 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/maintainers

for you to fetch changes up to cb8b3fc6e8a055d56c8fbedccace7adc5b700f2a:

  maintainers: add myself to a bit more stuff (2015-03-22 18:06:11 +0100)

----------------------------------------------------------------
Yann E. MORIN (3):
      support: add script to find maintainers
      support/maintainers: add support for autobuild results
      maintainers: add myself to a bit more stuff

 MAINTAINERS                      |  49 ++++++++++
 support/scripts/check-maintainer | 191 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+)
 create mode 100644 MAINTAINERS
 create mode 100755 support/scripts/check-maintainer

-- 
.-----------------.--------------------.------------------.--------------------.
|  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/3] support: add script to find maintainers

[Yann E. MORIN]

With the growing number of packages, as well as the many infrastructures
we now have (package, download...), it is time we introduce the notion
of maintainers.

This is done in two ways:

  - first, a file at the top of the repository, that associates a person
    to areas it is interested in;

  - a script that can help find the maintainer(s) for a package or an
    architecture.

The maintainers file is a simple person -> {files} association, and the
script can be fed a patch on stdin or a package or architecture name as
command line options.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 MAINTAINERS                      |  24 ++++++
 support/scripts/check-maintainer | 157 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+)
 create mode 100644 MAINTAINERS
 create mode 100755 support/scripts/check-maintainer

diff --git a/MAINTAINERS b/MAINTAINERS
new file mode 100644
index 0000000..7ae732c
--- /dev/null
+++ b/MAINTAINERS
@@ -0,0 +1,24 @@
+# vim: ft=
+#
+# List of maintainers for Buildroot.
+#
+# Format of this file:
+#
+# Person <mail>
+#   pattern
+#   pattern
+#
+# When a file matches a pattern, the person is considered interested.
+#
+# If a pattern ends with a slash, it is a directory pattern, and
+# matches all files in that directory; otherwise, the pattern is a file
+# pattern, and matches all files that match the glob.
+#
+# Empty lines are ignored; lines starting with a '#' are ignored.
+#
+# Please, keep this file sorted (LC_COLLATE=C), first sorted by committers
+# and then by patterns.
+
+"Yann E. MORIN" <yann.morin.1998@free.fr>
+    MAINTAINERS
+    support/scripts/check-maintainer
diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
new file mode 100755
index 0000000..0ca7136
--- /dev/null
+++ b/support/scripts/check-maintainer
@@ -0,0 +1,157 @@
+#!/usr/bin/env bash
+
+main() {
+    local OPT OPTARGS
+    local -a pkgs archs files rcpt
+    local pattern pkg arch file maintainer
+
+    while getopts :hp:a: OPT; do
+        case "${OPT}" in
+        h)  help; exit 0;;
+        p)  pkgs+=( "${OPTARG}" );;
+        a)  archs+=( "${OPTARG}" );;
+        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
+        \?) error "unknown option '%s'\n" "${OPTARG}";;
+        esac
+    done
+
+    if [ "${#pkgs[@]}" -eq 0 -a "${#archs[@]}" -eq 0 ]; then
+        # When no package and no arch specified, expect a patch from stdin
+        # and extract files from that patch
+        files=( $( diffstat -p1 -l  \
+                   |sort -u         \
+                   |sed -r -e 's/\|[^|]+$//'
+                 )
+              )
+    else
+        for pkg in "${pkgs[@]}"; do
+            files+=( $( find . -type f -name "${pkg}.mk" \
+                        |sed -r -e 's:^\./::'
+                      )
+                   )
+        done
+        for arch in "${archs[@]}"; do
+            files+=( "arch/Config.in.${arch}" )
+        done
+    fi
+
+    # We can't easily scan the pattern->maintainer relations into an
+    # array, because each pattern may be associated with more than one
+    # maintainer, and entries in bash arrays can't be another array
+    # (i.e. arrays are only one-dimensional).
+    # So, we scan the maintainers file once, and for each pattern we
+    # check if there is a matching file, which in practice is the
+    # optimum solution.
+    # Note: filtering-out comments and empty lines in the bash 'case'
+    # is slightly faster (~10%) than it is to 'sed' them out.
+    while read pattern; do
+        case "${pattern}" in
+        "#"*|"")
+            continue
+        ;;
+        *@*)
+            maintainer="${pattern}"
+            continue
+        ;;
+        esac
+        for file in "${files[@]}"; do
+            if filematch "${file}" "${pattern}"; then
+                debug "  --> adding '%s'\n" "${maintainer}"
+                rcpt+=( "${maintainer}" )
+            fi
+        done
+    done <MAINTAINERS
+
+    # Quick exit path in case there's no one interested, to avoid printing
+    # an empty line, below
+    if [ ${#rcpt[@]} -eq 0 ]; then
+        exit 1
+    fi
+
+    # Report all interested maintainers, ranking by number of matches
+    printf "%s\n" "${rcpt[@]}" \
+    |sort \
+    |uniq -c \
+    |sort -nr \
+    |gawk '{ gsub("^[[:space:]]+[[:digit:]]+[[:space:]]",""); print; }'
+}
+
+filematch() {
+    local file="${1}"
+    local pattern="${2}"
+
+    debug "Testing '%s' against '%s'... " "${file}" "${pattern}"
+
+    case "${pattern}" in
+    */) # Pattern is a directory, matches all files inthat directory
+        if [[ "${file}" =~ ^${pattern}[^/]+$ ]]; then
+            debug "OK\n"
+            return 0
+        fi
+    ;;
+    *)  # Any other pattern if a file match
+        case "${file}" in
+        ${pattern})
+            debug "OK\n"
+            return 0
+        ;;
+        esac
+    ;;
+    esac
+
+    debug "KO\n"
+    return 1
+}
+
+help() {
+    cat <<_EOF_
+NAME 
+    ${my_name} - find appropriate maintainers
+
+SYNOPSIS
+    ${my_name} [options]
+    ${my_name} < file.patch
+
+DESCRIPTION
+    In some projects, some persons are considered responsible for one or
+    more specific parts of the projects; they are called "maintainers",
+    like is done in the Linux kernel.
+
+    In Buildroot, however, we do not have such "maintainers"; rather,
+    some people can declare themselves to be "interested" in specific
+    parts of the project. We still call them "maintainers", though.
+
+    ${my_name} helps in finding such persons. It can be used in two ways.
+
+    In the first synopsis, you pass it a package name or an architecture
+    name, and it will tell you the persons that have declared interests
+    in those.
+
+    In the second synopsis, with no options, it will read a patch from
+    stdin, and it will tell you who has declared interest in the areas
+    touched by the patch.
+
+OPTIONS
+    -h  This help text.
+
+    -p PKG
+        Find the persons interested in package PKG.
+
+    -a ARCH
+        Find the persons interested in architecture ARCH.
+
+EXIT STATUS
+    ${my_name} exits with 0 if it could find a maintainer, and non-zero
+    otherwise.
+_EOF_
+}
+
+trace() { printf "${@}"; }
+debug() { :; }
+if [ -n "${DEBUG}" ]; then
+    debug() { trace "${@}" >&2; }
+fi
+error() { trace "${@}" >&2; exit 1; }
+
+my_name="${0##*/}"
+main "${@}"
-- 
1.9.1

[Buildroot] [PATCH 2/3] support/maintainers: add support for autobuild results

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/scripts/check-maintainer | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
index 0ca7136..1dae00b 100755
--- a/support/scripts/check-maintainer
+++ b/support/scripts/check-maintainer
@@ -1,20 +1,54 @@
 #!/usr/bin/env bash
 
+# Base URL for autobuild results
+ABO_BASE="http://autobuild.buildroot.org/results/"
+
 main() {
     local OPT OPTARGS
     local -a pkgs archs files rcpt
-    local pattern pkg arch file maintainer
+    local autobuild pattern pkg arch file maintainer
+    local pkg_url arch_url short_sha
 
-    while getopts :hp:a: OPT; do
+    while getopts :hp:a:b: OPT; do
         case "${OPT}" in
         h)  help; exit 0;;
         p)  pkgs+=( "${OPTARG}" );;
         a)  archs+=( "${OPTARG}" );;
+        b)  autobuild="${OPTARG}";;
         :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
         \?) error "unknown option '%s'\n" "${OPTARG}";;
         esac
     done
 
+    # Sanity check: can't have an autobuild url and either a package or an arch
+    if [ -n "${autobuild}" -a \( ${#pkgs[@]} -ne 0 -o ${#archs[@]} -ne 0 \) ]; then
+        error "Can not use an autobuild failure at the same time as a package or architcture\n"
+    fi
+
+    # For an autobuild failure, get the failing package and arch,
+    # and fake as if they were passed on the command line.
+    if [ -n "${autobuild}" ]; then
+        # If it starts with http:// then we consider this is an URL to
+        # an autobuild result directory, otherwise we consider it is the
+        # sha1 of a failed build.
+        if [[ "${autobuild}" =~ ^http:// ]]; then
+            pkg_url="${autobuild}/build-time.log"
+            arch_url="${autobuild}/config"
+        else
+            short_sha="${autobuild%${autobuild#???}}"
+            pkg_url="${ABO_BASE}/${short_sha}/${autobuild}/build-time.log"
+            arch_url="${ABO_BASE}/${short_sha}/${autobuild}/config"
+        fi
+        pkgs=( "$( curl -s "${pkg_url}" \
+                   |tail -n 1 |awk '{ print $(NF); }'
+                 )"
+             )
+        archs=( "$( curl -s "${arch_url}" \
+                    |sed -r -e '/^BR2_ARCH="(.+)"$/!d; s//\1/'
+                  )"
+              )
+    fi
+
     if [ "${#pkgs[@]}" -eq 0 -a "${#archs[@]}" -eq 0 ]; then
         # When no package and no arch specified, expect a patch from stdin
         # and extract files from that patch
-- 
1.9.1

[Buildroot] [PATCH 3/3] maintainers: add myself to a bit more stuff

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 MAINTAINERS | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7ae732c..beb91b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21,4 +21,29 @@
 
 "Yann E. MORIN" <yann.morin.1998@free.fr>
     MAINTAINERS
+    package/ca-certificates/
+    package/doc-asciidoc.mk
+    package/dtv-scan-tables/
+    package/freerdp/
+    package/kodi*/*
+    package/linux-firmware/
+    package/mke2img/
+    package/nut/
+    package/nvidia-driver/
+    package/opengl/
+    package/opengl*/*
+    package/patchelf/
+    package/qemu/
+    package/rpi-firmware/
+    package/rpi-userland/
+    package/tvheadend/
+    package/tz/
+    package/tzdata/
+    package/tzdump/
+    package/w_scan/
+    package/wayland/
+    package/weston/
+    package/zic/
+    support/download/
     support/scripts/check-maintainer
+    support/scripts/mkusers
-- 
1.9.1

[Buildroot] [PATCH 1/3] support: add script to find maintainers

[André Erdmann]

Hi,

2015/4/24 Yann E. Morin <yann.morin.1998@free.fr>:
> With the growing number of packages, as well as the many infrastructures
> we now have (package, download...), it is time we introduce the notion
> of maintainers.
> 
> This is done in two ways:
> 
>   - first, a file at the top of the repository, that associates a person
>     to areas it is interested in;
> 
>   - a script that can help find the maintainer(s) for a package or an
>     architecture.
> 
> The maintainers file is a simple person -> {files} association, and the
> script can be fed a patch on stdin or a package or architecture name as
> command line options.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  MAINTAINERS                      |  24 ++++++
>  support/scripts/check-maintainer | 157 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 181 insertions(+)
>  create mode 100644 MAINTAINERS
>  create mode 100755 support/scripts/check-maintainer
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> new file mode 100644
> index 0000000..7ae732c
> --- /dev/null
> +++ b/MAINTAINERS
> @@ -0,0 +1,24 @@
> +# vim: ft=
> +#
> +# List of maintainers for Buildroot.
> +#
> +# Format of this file:
> +#
> +# Person <mail>
> +#   pattern
> +#   pattern
> +#
> +# When a file matches a pattern, the person is considered interested.
> +#
> +# If a pattern ends with a slash, it is a directory pattern, and
> +# matches all files in that directory; otherwise, the pattern is a file
> +# pattern, and matches all files that match the glob.
> +#
> +# Empty lines are ignored; lines starting with a '#' are ignored.
> +#
> +# Please, keep this file sorted (LC_COLLATE=C), first sorted by committers
> +# and then by patterns.
> +
> +"Yann E. MORIN" <yann.morin.1998@free.fr>
> +    MAINTAINERS
> +    support/scripts/check-maintainer
> diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
> new file mode 100755
> index 0000000..0ca7136
> --- /dev/null
> +++ b/support/scripts/check-maintainer
> @@ -0,0 +1,157 @@
> +#!/usr/bin/env bash
> +
> +main() {
> +    local OPT OPTARGS

s/OPTARGS/OPTARG?

Not sure if it makes sense to local-scope getopts vars,
there's also OPTIND and maybe OPTERR.


> +    local -a pkgs archs files rcpt
> +    local pattern pkg arch file maintainer
> +
> +    while getopts :hp:a: OPT; do
> +        case "${OPT}" in
> +        h)  help; exit 0;;
> +        p)  pkgs+=( "${OPTARG}" );;
> +        a)  archs+=( "${OPTARG}" );;
> +        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
> +        \?) error "unknown option '%s'\n" "${OPTARG}";;
> +        esac
> +    done

Non-option args will be ignored by this script.
If people don't read the help message first and simply run
"check-maintainer PKG" or do some scripting and end up with
"check-maintainer -p $(command_that_usually_prints_a_single_package_name_but_now_prints_two)",
then the script could hang forever waiting for a patch from stdin.

Might be better to error-exit in this case, e.g.:

    # bail out if any positional args are given
    [ ${#} -lt ${OPTIND} ] || error "bad usage: script does not accept positional args.\n"


> +
> +    if [ "${#pkgs[@]}" -eq 0 -a "${#archs[@]}" -eq 0 ]; then
> +        # When no package and no arch specified, expect a patch from stdin
> +        # and extract files from that patch
> +        files=( $( diffstat -p1 -l  \
> +                   |sort -u         \
> +                   |sed -r -e 's/\|[^|]+$//'
> +                 )
> +              )
> +    else
> +        for pkg in "${pkgs[@]}"; do
> +            files+=( $( find . -type f -name "${pkg}.mk" \
> +                        |sed -r -e 's:^\./::'
> +                      )
> +                   )
> +        done
> +        for arch in "${archs[@]}"; do
> +            files+=( "arch/Config.in.${arch}" )
> +        done
> +    fi
> +
> +    # We can't easily scan the pattern->maintainer relations into an
> +    # array, because each pattern may be associated with more than one
> +    # maintainer, and entries in bash arrays can't be another array
> +    # (i.e. arrays are only one-dimensional).
> +    # So, we scan the maintainers file once, and for each pattern we
> +    # check if there is a matching file, which in practice is the
> +    # optimum solution.
> +    # Note: filtering-out comments and empty lines in the bash 'case'
> +    # is slightly faster (~10%) than it is to 'sed' them out.
> +    while read pattern; do

"read -r" unless backslash escapes should be expanded

> +        case "${pattern}" in
> +        "#"*|"")
> +            continue
> +        ;;
> +        *@*)
> +            maintainer="${pattern}"
> +            continue
> +        ;;
> +        esac


A "have maintainer?" check would slightly help against a malformed
maintainers file (file pattern before the first "Person <mail>" line):

    [ -z "${maintainer}" ] || \
> +        for file in "${files[@]}"; do
> +            if filematch "${file}" "${pattern}"; then
> +                debug "  --> adding '%s'\n" "${maintainer}"
> +                rcpt+=( "${maintainer}" )
> +            fi
> +        done
> +    done <MAINTAINERS
> +

The read-loop basically reads as follows:

   foreach maintainer, pattern loop
      foreach file loop
         if filematch file, pattern then
            rank up maintainer
         end if
      end loop
   end loop

=> Maintainers with more than one pattern matching the same file get ranked up,
   try "check-maintainer -p opengl" ;) (after applying patch 3 of this series)

Is that on purpose or accepted behavior due to otherwise increased script complexity?

It'd be rather easy to fix it in bash 4 /w an associative array (*),
but I don't know of any sane solution for bash 3, if that's a requirement.

(*) as long as all patterns are listed in one block per maintainer
    (no "person <mail>" redefinition), but that's guaranteed by the file format

> +    # Quick exit path in case there's no one interested, to avoid printing
> +    # an empty line, below
> +    if [ ${#rcpt[@]} -eq 0 ]; then
> +        exit 1
> +    fi
> +
> +    # Report all interested maintainers, ranking by number of matches
> +    printf "%s\n" "${rcpt[@]}" \
> +    |sort \
> +    |uniq -c \
> +    |sort -nr \
> +    |gawk '{ gsub("^[[:space:]]+[[:digit:]]+[[:space:]]",""); print; }'
> +}
> +
> +filematch() {
> +    local file="${1}"
> +    local pattern="${2}"
> +
> +    debug "Testing '%s' against '%s'... " "${file}" "${pattern}"
> +
> +    case "${pattern}" in
> +    */) # Pattern is a directory, matches all files inthat directory
> +        if [[ "${file}" =~ ^${pattern}[^/]+$ ]]; then
> +            debug "OK\n"
> +            return 0
> +        fi
> +    ;;
> +    *)  # Any other pattern if a file match
> +        case "${file}" in
> +        ${pattern})
> +            debug "OK\n"
> +            return 0
> +        ;;
> +        esac
> +    ;;
> +    esac
> +
> +    debug "KO\n"
> +    return 1
> +}
> +
> +help() {
> +    cat <<_EOF_
> +NAME 
> +    ${my_name} - find appropriate maintainers
> +
> +SYNOPSIS
> +    ${my_name} [options]
> +    ${my_name} < file.patch
> +
> +DESCRIPTION
> +    In some projects, some persons are considered responsible for one or
> +    more specific parts of the projects; they are called "maintainers",
> +    like is done in the Linux kernel.
> +
> +    In Buildroot, however, we do not have such "maintainers"; rather,
> +    some people can declare themselves to be "interested" in specific
> +    parts of the project. We still call them "maintainers", though.
> +
> +    ${my_name} helps in finding such persons. It can be used in two ways.
> +
> +    In the first synopsis, you pass it a package name or an architecture
> +    name, and it will tell you the persons that have declared interests
> +    in those.
> +
> +    In the second synopsis, with no options, it will read a patch from
> +    stdin, and it will tell you who has declared interest in the areas
> +    touched by the patch.
> +
> +OPTIONS
> +    -h  This help text.
> +
> +    -p PKG
> +        Find the persons interested in package PKG.
> +

Noteworthy: PKG can also be a glob pattern,
            e.g. "usb_modeswitch*" (because of "find -name <PKG>.mk").

Also, "-p" and "-a" can be given more than once,
whereas "-b" introduced in patch 2 behaves differently.

> +    -a ARCH
> +        Find the persons interested in architecture ARCH.
> +
> +EXIT STATUS
> +    ${my_name} exits with 0 if it could find a maintainer, and non-zero
> +    otherwise.
> +_EOF_
> +}
> +
> +trace() { printf "${@}"; }
> +debug() { :; }
> +if [ -n "${DEBUG}" ]; then
> +    debug() { trace "${@}" >&2; }
> +fi
> +error() { trace "${@}" >&2; exit 1; }
> +
> +my_name="${0##*/}"
> +main "${@}"
> 

-- 
Andr?
 

[Buildroot] [PATCH 2/3] support/maintainers: add support for autobuild results

[André Erdmann]

Hi,

2015/4/24 Yann E. Morin <yann.morin.1998@free.fr>:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  support/scripts/check-maintainer | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
> index 0ca7136..1dae00b 100755
> --- a/support/scripts/check-maintainer
> +++ b/support/scripts/check-maintainer
> @@ -1,20 +1,54 @@
>  #!/usr/bin/env bash
>  
> +# Base URL for autobuild results
> +ABO_BASE="http://autobuild.buildroot.org/results/"
> +
>  main() {
>      local OPT OPTARGS
>      local -a pkgs archs files rcpt
> -    local pattern pkg arch file maintainer
> +    local autobuild pattern pkg arch file maintainer
> +    local pkg_url arch_url short_sha
>  
> -    while getopts :hp:a: OPT; do
> +    while getopts :hp:a:b: OPT; do
>          case "${OPT}" in
>          h)  help; exit 0;;
>          p)  pkgs+=( "${OPTARG}" );;
>          a)  archs+=( "${OPTARG}" );;
> +        b)  autobuild="${OPTARG}";;
>          :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
>          \?) error "unknown option '%s'\n" "${OPTARG}";;
>          esac
>      done
>  
> +    # Sanity check: can't have an autobuild url and either a package or an arch
> +    if [ -n "${autobuild}" -a \( ${#pkgs[@]} -ne 0 -o ${#archs[@]} -ne 0 \) ]; then
> +        error "Can not use an autobuild failure at the same time as a package or architcture\n"
> +    fi
> +
> +    # For an autobuild failure, get the failing package and arch,
> +    # and fake as if they were passed on the command line.
> +    if [ -n "${autobuild}" ]; then
> +        # If it starts with http:// then we consider this is an URL to
> +        # an autobuild result directory, otherwise we consider it is the
> +        # sha1 of a failed build.
> +        if [[ "${autobuild}" =~ ^http:// ]]; then
> +            pkg_url="${autobuild}/build-time.log"
> +            arch_url="${autobuild}/config"
> +        else
> +            short_sha="${autobuild%${autobuild#???}}"

More readable:

   short_sha="${autobuild:0:3}"

But behaves differently from your approach when $autobuild is not valid
(<= 3 chars long).

> +            pkg_url="${ABO_BASE}/${short_sha}/${autobuild}/build-time.log"
> +            arch_url="${ABO_BASE}/${short_sha}/${autobuild}/config"
> +        fi
> +        pkgs=( "$( curl -s "${pkg_url}" \
> +                   |tail -n 1 |awk '{ print $(NF); }'
> +                 )"
> +             )
> +        archs=( "$( curl -s "${arch_url}" \
> +                    |sed -r -e '/^BR2_ARCH="(.+)"$/!d; s//\1/'
> +                  )"
> +              )
> +    fi
> +
>      if [ "${#pkgs[@]}" -eq 0 -a "${#archs[@]}" -eq 0 ]; then
>          # When no package and no arch specified, expect a patch from stdin
>          # and extract files from that patch
> 

The help message for "-b" is missing.

-- 
Andr?

 

[Buildroot] [PATCH 1/3] support: add script to find maintainers

[Yann E. MORIN]

Andr?, All,

On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:
> 2015/4/24 Yann E. Morin <yann.morin.1998@free.fr>:
> > With the growing number of packages, as well as the many infrastructures
> > we now have (package, download...), it is time we introduce the notion
> > of maintainers.
[--SNIP--]
> > diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
> > new file mode 100755
> > index 0000000..0ca7136
> > --- /dev/null
> > +++ b/support/scripts/check-maintainer
> > @@ -0,0 +1,157 @@
> > +#!/usr/bin/env bash
> > +
> > +main() {
> > +    local OPT OPTARGS
> 
> s/OPTARGS/OPTARG?

Bummer...

> Not sure if it makes sense to local-scope getopts vars,
> there's also OPTIND and maybe OPTERR.

Well, it does not really matter, even for OPT and OPTARG, since main()
is the sole function in the script.

Still, I have for a while tried to declare all local-scope variables
with 'local', As I find it to be a "good" behaviour.

But you're right, to be consistent, all should be declared local.

And OPTERR is not something set by bash, it is a variable that bash
reads to see how to behave on error.  So, as long as it is not set,
there's no reason to declare it. But of course, there is, since we
do not want to inhrit it from the user's environment.

> > +    local -a pkgs archs files rcpt
> > +    local pattern pkg arch file maintainer
> > +
> > +    while getopts :hp:a: OPT; do
> > +        case "${OPT}" in
> > +        h)  help; exit 0;;
> > +        p)  pkgs+=( "${OPTARG}" );;
> > +        a)  archs+=( "${OPTARG}" );;
> > +        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
> > +        \?) error "unknown option '%s'\n" "${OPTARG}";;
> > +        esac
> > +    done
> 
> Non-option args will be ignored by this script.
> If people don't read the help message first and simply run
> "check-maintainer PKG" or do some scripting and end up with
> "check-maintainer -p $(command_that_usually_prints_a_single_package_name_but_now_prints_two)",
> then the script could hang forever waiting for a patch from stdin.
> 
> Might be better to error-exit in this case, e.g.:
> 
>     # bail out if any positional args are given
>     [ ${#} -lt ${OPTIND} ] || error "bad usage: script does not accept positional args.\n"

Good catch. I'll fix.

> > +    # We can't easily scan the pattern->maintainer relations into an
> > +    # array, because each pattern may be associated with more than one
> > +    # maintainer, and entries in bash arrays can't be another array
> > +    # (i.e. arrays are only one-dimensional).
> > +    # So, we scan the maintainers file once, and for each pattern we
> > +    # check if there is a matching file, which in practice is the
> > +    # optimum solution.
> > +    # Note: filtering-out comments and empty lines in the bash 'case'
> > +    # is slightly faster (~10%) than it is to 'sed' them out.
> > +    while read pattern; do
> 
> "read -r" unless backslash escapes should be expanded

There's no need for backslash escapes, so I'll use 'read -r'.

> > +        case "${pattern}" in
> > +        "#"*|"")
> > +            continue
> > +        ;;
> > +        *@*)
> > +            maintainer="${pattern}"
> > +            continue
> > +        ;;
> > +        esac
> 
> A "have maintainer?" check would slightly help against a malformed
> maintainers file (file pattern before the first "Person <mail>" line):
> 
>     [ -z "${maintainer}" ] || \

Better yet:
    [ -n "${maintainer}" ] || continue

No?

> > +        for file in "${files[@]}"; do
> > +            if filematch "${file}" "${pattern}"; then
> > +                debug "  --> adding '%s'\n" "${maintainer}"
> > +                rcpt+=( "${maintainer}" )
> > +            fi
> > +        done
> > +    done <MAINTAINERS
> > +
> 
> The read-loop basically reads as follows:
> 
>    foreach maintainer, pattern loop
>       foreach file loop
>          if filematch file, pattern then
>             rank up maintainer
>          end if
>       end loop
>    end loop
> 
> => Maintainers with more than one pattern matching the same file get ranked up,
>    try "check-maintainer -p opengl" ;) (after applying patch 3 of this series)
> 
> Is that on purpose or accepted behavior due to otherwise increased script complexity?

Yes, that's on purpose, see the comment a few lines later:
    # Report all interested maintainers, ranking by number of matches

Is that a problem ranking by number of matches?

> It'd be rather easy to fix it in bash 4 /w an associative array (*),
> but I don't know of any sane solution for bash 3, if that's a requirement.
> 
> (*) as long as all patterns are listed in one block per maintainer
>     (no "person <mail>" redefinition), but that's guaranteed by the file format

No, we can not use associative arrays to store { patterns -> maintainer } 
relations, because a single pattern may have more than one maintainer.

For example, I'm interested in linux/* and you're interested in linux/*
too. How can w estore that in an associative array? Remember that arrays
in bash are one-dimensional, so we can have an associative array like;
    { pattern -> { maint-1, maint-2 } }

That's explained in the comment above the read-loop.

> > +DESCRIPTION
> > +    In some projects, some persons are considered responsible for one or
> > +    more specific parts of the projects; they are called "maintainers",
> > +    like is done in the Linux kernel.
> > +
> > +    In Buildroot, however, we do not have such "maintainers"; rather,
> > +    some people can declare themselves to be "interested" in specific
> > +    parts of the project. We still call them "maintainers", though.
> > +
> > +    ${my_name} helps in finding such persons. It can be used in two ways.
> > +
> > +    In the first synopsis, you pass it a package name or an architecture
> > +    name, and it will tell you the persons that have declared interests
> > +    in those.
> > +
> > +    In the second synopsis, with no options, it will read a patch from
> > +    stdin, and it will tell you who has declared interest in the areas
> > +    touched by the patch.
> > +
> > +OPTIONS
> > +    -h  This help text.
> > +
> > +    -p PKG
> > +        Find the persons interested in package PKG.
> > +
> 
> Noteworthy: PKG can also be a glob pattern,
>             e.g. "usb_modeswitch*" (because of "find -name <PKG>.mk").

Well, even if that works, do we want to document it? Hmm... yes, it
might be usefull; consider e.g. 'kodi*' or 'matchbox*', indeed.

OK, will add to the help.

> Also, "-p" and "-a" can be given more than once,

Will make it explicit.

> whereas "-b" introduced in patch 2 behaves differently.

Does it make sense to support more than one autobuild failure?

Thanks!

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 2/3] support/maintainers: add support for autobuild results

[Yann E. MORIN]

Andr?, All,

On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:
> 2015/4/24 Yann E. Morin <yann.morin.1998@free.fr>:
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> >  support/scripts/check-maintainer | 38 ++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> > 
> > diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
> > index 0ca7136..1dae00b 100755
> > --- a/support/scripts/check-maintainer
> > +++ b/support/scripts/check-maintainer
> > @@ -1,20 +1,54 @@
> >  #!/usr/bin/env bash
> >  
> > +# Base URL for autobuild results
> > +ABO_BASE="http://autobuild.buildroot.org/results/"
> > +
> >  main() {
> >      local OPT OPTARGS
> >      local -a pkgs archs files rcpt
> > -    local pattern pkg arch file maintainer
> > +    local autobuild pattern pkg arch file maintainer
> > +    local pkg_url arch_url short_sha
> >  
> > -    while getopts :hp:a: OPT; do
> > +    while getopts :hp:a:b: OPT; do
> >          case "${OPT}" in
> >          h)  help; exit 0;;
> >          p)  pkgs+=( "${OPTARG}" );;
> >          a)  archs+=( "${OPTARG}" );;
> > +        b)  autobuild="${OPTARG}";;
> >          :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
> >          \?) error "unknown option '%s'\n" "${OPTARG}";;
> >          esac
> >      done
> >  
> > +    # Sanity check: can't have an autobuild url and either a package or an arch
> > +    if [ -n "${autobuild}" -a \( ${#pkgs[@]} -ne 0 -o ${#archs[@]} -ne 0 \) ]; then
> > +        error "Can not use an autobuild failure at the same time as a package or architcture\n"
> > +    fi
> > +
> > +    # For an autobuild failure, get the failing package and arch,
> > +    # and fake as if they were passed on the command line.
> > +    if [ -n "${autobuild}" ]; then
> > +        # If it starts with http:// then we consider this is an URL to
> > +        # an autobuild result directory, otherwise we consider it is the
> > +        # sha1 of a failed build.
> > +        if [[ "${autobuild}" =~ ^http:// ]]; then
> > +            pkg_url="${autobuild}/build-time.log"
> > +            arch_url="${autobuild}/config"
> > +        else
> > +            short_sha="${autobuild%${autobuild#???}}"
> 
> More readable:
> 
>    short_sha="${autobuild:0:3}"

Yes, but this is a bashism (i.e. not posix). And even though we're
explicitly using bash, I tend to avoid bashisms when it can be done with
POSIX constructs.

But since Thomas alrady raised a concern about that in another patch
(for the autobuilder code itself), I'll switch to using the bashsim.

> But behaves differently from your approach when $autobuild is not valid
> (<= 3 chars long).

I'll put guards in palce to catch this.

[--SNIP--]
> The help message for "-b" is missing.

ACK.

Thanks!

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 1/3] support: add script to find maintainers

[Yann E. MORIN]

Andr?, All,

On 2015-04-25 14:46 +0200, Yann E. MORIN spake thusly:
> On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:
[--SNIP--]
> > Not sure if it makes sense to local-scope getopts vars,
> > there's also OPTIND and maybe OPTERR.
[--SNIP--]
> And OPTERR is not something set by bash, it is a variable that bash
> reads to see how to behave on error.  So, as long as it is not set,
> there's no reason to declare it. But of course, there is, since we
> do not want to inhrit it from the user's environment.

Well, we don;t care the value of OPTERR, since the optstring starts with
a colon, and thus there is explicitly no error reporting (except the one
we manually handle). So there's no reason to have a local OPTERR.

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 1/3] support: add script to find maintainers

[André Erdmann]

2015/4/25 Yann E. Morin <yann.morin.1998@free.fr>:
> Andr?, All,
> 
> On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:
>> 2015/4/24 Yann E. Morin <yann.morin.1998@free.fr>:
>>> With the growing number of packages, as well as the many infrastructures
>>> we now have (package, download...), it is time we introduce the notion
>>> of maintainers.
> [--SNIP--]
>>> diff --git a/support/scripts/check-maintainer b/support/scripts/check-maintainer
>>> new file mode 100755
>>> index 0000000..0ca7136
>>> --- /dev/null
>>> +++ b/support/scripts/check-maintainer
>>> @@ -0,0 +1,157 @@
>>> +#!/usr/bin/env bash
>>> +
>>> +main() {
>>> +    local OPT OPTARGS
>>
>> s/OPTARGS/OPTARG?
> 
> Bummer...
> 
>> Not sure if it makes sense to local-scope getopts vars,
>> there's also OPTIND and maybe OPTERR.
> 
> Well, it does not really matter, even for OPT and OPTARG, since main()
> is the sole function in the script.
> 
> Still, I have for a while tried to declare all local-scope variables
> with 'local', As I find it to be a "good" behaviour.
> 
> But you're right, to be consistent, all should be declared local.
> 
> And OPTERR is not something set by bash, it is a variable that bash
> reads to see how to behave on error.  So, as long as it is not set,
> there's no reason to declare it. But of course, there is, since we
> do not want to inhrit it from the user's environment.
>

bash initializes OPTIND to 1, might be a good idea to do "local -i OPTIND=1" then.

 
> [SNIP]
>>> +    # We can't easily scan the pattern->maintainer relations into an
>>> +    # array, because each pattern may be associated with more than one
>>> +    # maintainer, and entries in bash arrays can't be another array
>>> +    # (i.e. arrays are only one-dimensional).
>>> +    # So, we scan the maintainers file once, and for each pattern we
>>> +    # check if there is a matching file, which in practice is the
>>> +    # optimum solution.
>>> +    # Note: filtering-out comments and empty lines in the bash 'case'
>>> +    # is slightly faster (~10%) than it is to 'sed' them out.
>>> +    while read pattern; do
>>
>> "read -r" unless backslash escapes should be expanded
> 
> There's no need for backslash escapes, so I'll use 'read -r'.
> 
>>> +        case "${pattern}" in
>>> +        "#"*|"")
>>> +            continue
>>> +        ;;
>>> +        *@*)
>>> +            maintainer="${pattern}"
>>> +            continue
>>> +        ;;
>>> +        esac
>>
>> A "have maintainer?" check would slightly help against a malformed
>> maintainers file (file pattern before the first "Person <mail>" line):
>>
>>     [ -z "${maintainer}" ] || \
> 
> Better yet:
>     [ -n "${maintainer}" ] || continue
> 
> No?
>

yes
 
>>> +        for file in "${files[@]}"; do
>>> +            if filematch "${file}" "${pattern}"; then
>>> +                debug "  --> adding '%s'\n" "${maintainer}"
>>> +                rcpt+=( "${maintainer}" )
>>> +            fi
>>> +        done
>>> +    done <MAINTAINERS
>>> +
>>
>> The read-loop basically reads as follows:
>>
>>    foreach maintainer, pattern loop
>>       foreach file loop
>>          if filematch file, pattern then
>>             rank up maintainer
>>          end if
>>       end loop
>>    end loop
>>
>> => Maintainers with more than one pattern matching the same file get ranked up,
>>    try "check-maintainer -p opengl" ;) (after applying patch 3 of this series)
>>
>> Is that on purpose or accepted behavior due to otherwise increased script complexity?
> 
> Yes, that's on purpose, see the comment a few lines later:
>     # Report all interested maintainers, ranking by number of matches
> 
> Is that a problem ranking by number of matches?
> 

Well, you match package/opengl/opengl.mk with two different patterns:

   package/opengl/
   package/opengl*/*

and get a score of 2 - that feels like cheating! ;)

(It gets worse when the number of files and/or the number
of overlapping patterns increase, because the upper score bound
of the current approach is "number of files" * "number of patterns"
and not "number of files".)


>> It'd be rather easy to fix it in bash 4 /w an associative array (*),
>> but I don't know of any sane solution for bash 3, if that's a requirement.
>>
>> (*) as long as all patterns are listed in one block per maintainer
>>     (no "person <mail>" redefinition), but that's guaranteed by the file format
> 
> No, we can not use associative arrays to store { patterns -> maintainer } 
> relations, because a single pattern may have more than one maintainer.
> 
> For example, I'm interested in linux/* and you're interested in linux/*
> too. How can w estore that in an associative array? Remember that arrays
> in bash are one-dimensional, so we can have an associative array like;
>     { pattern -> { maint-1, maint-2 } }
>

You can't track who's interested in which package matched by which pattern unless
you go the eval / dynamic variables / special value encoding route, but you
can track easily if a maintainer has already shown interest in a specific file
(with the "all patterns in one block per maintainer" restriction):

Expanding the pseudo-code snippet from before:

   maintainer := <empty str>
   fmatched   := <don't care>

   foreach nonempty/non-comment input_line loop
      if input_line starts new maintainer block then
         maintainer := input_line
         fmatched   := <empty associative array>

      else
         # is a pattern
         foreach file loop
            if file not in fmatched then
               if filematch file, input_line then
                   fmatched->file := YES
                   rank up maintainer
               end if
            end if
         end loop
      end if
   end loop

 
> That's explained in the comment above the read-loop.
> 
>>> +DESCRIPTION
>>> +    In some projects, some persons are considered responsible for one or
>>> +    more specific parts of the projects; they are called "maintainers",
>>> +    like is done in the Linux kernel.
>>> +
>>> +    In Buildroot, however, we do not have such "maintainers"; rather,
>>> +    some people can declare themselves to be "interested" in specific
>>> +    parts of the project. We still call them "maintainers", though.
>>> +
>>> +    ${my_name} helps in finding such persons. It can be used in two ways.
>>> +
>>> +    In the first synopsis, you pass it a package name or an architecture
>>> +    name, and it will tell you the persons that have declared interests
>>> +    in those.
>>> +
>>> +    In the second synopsis, with no options, it will read a patch from
>>> +    stdin, and it will tell you who has declared interest in the areas
>>> +    touched by the patch.
>>> +
>>> +OPTIONS
>>> +    -h  This help text.
>>> +
>>> +    -p PKG
>>> +        Find the persons interested in package PKG.
>>> +
>>
>> Noteworthy: PKG can also be a glob pattern,
>>             e.g. "usb_modeswitch*" (because of "find -name <PKG>.mk").
> 
> Well, even if that works, do we want to document it? Hmm... yes, it
> might be usefull; consider e.g. 'kodi*' or 'matchbox*', indeed.
> 
> OK, will add to the help.
> 
>> Also, "-p" and "-a" can be given more than once,
> 
> Will make it explicit.
> 
>> whereas "-b" introduced in patch 2 behaves differently.
> 
> Does it make sense to support more than one autobuild failure?
>

Don't know - just looked at the getopts-loop and noticed that it's different.

-- 
Andr?