introduce maintainers (branch yem/maintainers)

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" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 MAINTAINERS                      |  24 ++++++
 support/scripts/check-maintainer | 157 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+)
 create mode 100644 MAINTAINERS
 create mode 100755 support/scripts/check-maintainer

MAINTAINERS new file +24 -0

new file mode 100644
index 0000000..7ae732c
@@ -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" <[email protected]>
+    MAINTAINERS
+    support/scripts/check-maintainer

support/scripts/check-maintainer new file +0 -0

new file mode 100755
index 0000000..0ca7136

support/scripts/check-maintainer new file +21 -0

+
+    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
+

support/scripts/check-maintainer new file +65 -0

+    # 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.
+

support/scripts/check-maintainer new file +18 -0

+    -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 "${@}"

2015/4/24 Yann E. Morin <[email protected]>:

Quoted 3 lines

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.

Quoted 4 lines

support/scripts/check-maintainer new file +0 -1

new file mode 100755
index 0000000..0ca7136
--- /dev/null

support/scripts/check-maintainer line 4

Quoted 6 lines

+++ b/support/scripts/check-maintainer

support/scripts/check-maintainer new file +4 -0

@@ -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.

support/scripts/check-maintainer line 16

Quoted 12 lines

support/scripts/check-maintainer new file +12 -0

+    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"

support/scripts/check-maintainer line 47

Quoted 10 lines

support/scripts/check-maintainer new file +10 -0

+    # 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

support/scripts/check-maintainer line 56

Quoted 9 lines

support/scripts/check-maintainer new file +9 -0

+        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}" ] || \

support/scripts/check-maintainer line 64

Quoted 8 lines

support/scripts/check-maintainer new file +8 -0

+        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

Quoted 15 lines

support/scripts/check-maintainer new file +15 -0

+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.
+

support/scripts/check-maintainer line 139

Quoted 10 lines

support/scripts/check-maintainer new file +10 -0

+    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.

On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:

Quoted 2 lines

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

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.

Andr?, All,

On 2015-04-25 14:09 +0200, Andr? Erdmann spake thusly:

Quoted 4 lines

2015/4/24 Yann E. Morin <[email protected]>:

Quoted 3 lines

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--]

Quoted 4 lines

Quoted 4 lines

support/scripts/check-maintainer new file +0 -1

new file mode 100755
index 0000000..0ca7136
--- /dev/null

Quoted 8 lines

support/scripts/check-maintainer line 4

Quoted 6 lines

+++ b/support/scripts/check-maintainer

support/scripts/check-maintainer new file +4 -0

@@ -0,0 +1,157 @@
+#!/usr/bin/env bash
+
+main() {
+    local OPT OPTARGS

s/OPTARGS/OPTARG?

Bummer...

Quoted 2 lines

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.

[SNIP]

Quoted 11 lines

support/scripts/check-maintainer line 47

Quoted 10 lines

support/scripts/check-maintainer new file +10 -0

+    # 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

Quoted 1 line

"read -r" unless backslash escapes should be expanded

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

Quoted 13 lines

support/scripts/check-maintainer line 56

Quoted 9 lines

support/scripts/check-maintainer new file +9 -0

+        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):

Quoted 1 line

    [ -z "${maintainer}" ] || \

Better yet:

    [ -n "${maintainer}" ] || continue

No?

Quoted 22 lines

support/scripts/check-maintainer line 64

Quoted 8 lines

support/scripts/check-maintainer new file +8 -0

+        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)

Quoted 1 line

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?

Quoted 5 lines

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.

Quoted 27 lines

Quoted 15 lines

support/scripts/check-maintainer new file +15 -0

+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.
+

support/scripts/check-maintainer line 139

Quoted 10 lines

support/scripts/check-maintainer new file +10 -0

+    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,

Quoted 1 line

            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.

Quoted 1 line

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

Will make it explicit.

Quoted 1 line

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

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

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 support/scripts/check-maintainer | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

support/scripts/check-maintainer +13 -2

index 0ca7136..1dae00b 100755
@@ -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,

support/scripts/check-maintainer +6 -0

+            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); }'
+                 )"

2015/4/24 Yann E. Morin <[email protected]>:

Quoted 40 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 support/scripts/check-maintainer | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

support/scripts/check-maintainer +13 -2

index 0ca7136..1dae00b 100755
@@ -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,

support/scripts/check-maintainer line 38

Quoted 10 lines

support/scripts/check-maintainer +10 -0

+    # 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).

The help message for "-b" is missing.