[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Yann E. MORIN]

Currently, the br2-external script uses bash-4's associative arrays.

However, some oldish enterprise-class distros like RHEL5 still use
bash-3.1 which lacks associative arrays.

We restore compatibility with those oldish distros using 'eval' to
emulate associative arrays, as suggested by Arnout.

Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Max Filippov <jcmvbkbc@gmail.com>
---
 support/scripts/br2-external | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 055dc08..84bc334 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -1,10 +1,12 @@
 #!/bin/bash
 set -e
 
-# The names and locations of the br2-external trees, once validated.
+# This script must be able to run with bash-3.1, so it can't use
+# associative arrays. Instead, it emulates them using 'eval'. It
+# can however use indexed arrays, supported since at least bash-3.0.
+
+# The names of the br2-external trees, once validated.
 declare -a BR2_EXT_NAMES
-declare -A BR2_EXT_PATHS
-declare -A BR2_EXT_DESCS
 
 # URL to manual for help in converting old br2-external trees.
 # Escape '#' so that make does not consider it a comment.
@@ -68,7 +70,7 @@ do_validate() {
 
 do_validate_one() {
     local br2_ext="${1}"
-    local br2_name br2_desc n
+    local br2_name br2_desc n d
 
     if [ ! -d "${br2_ext}" ]; then
         error "'%s': no such file or directory\n" "${br2_ext}"
@@ -91,9 +93,10 @@ do_validate_one() {
         error "'%s': name '%s' contains invalid chars: '%s'\n" \
             "${br2_ext}" "${br2_name//\$/\$\$}" "${n//\$/\$\$}"
     fi
-    if [ -n "${BR2_EXT_PATHS["${br2_name}"]}" ]; then
+    eval d="\"\${BR2_EXT_PATHS_${br2_name}}\""
+    if [ -n "${d}" ]; then
         error "'%s': name '%s' is already used in '%s'\n" \
-            "${br2_ext}" "${br2_name}" "${BR2_EXT_PATHS["${br2_name}"]}"
+            "${br2_ext}" "${br2_name}" "${d}"
     fi
     br2_desc="$(sed -r -e '/^desc: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
     if [ ! -f "${br2_ext}/external.mk" ]; then
@@ -105,8 +108,8 @@ do_validate_one() {
 
     # Register this br2-external tree
     BR2_EXT_NAMES+=( "${br2_name}" )
-    BR2_EXT_PATHS["${br2_name}"]="${br2_ext}"
-    BR2_EXT_DESCS["${br2_name}"]="${br2_desc:-${br2_name}}"
+    eval BR2_EXT_PATHS_${br2_name}="\"\${br2_ext}\""
+    eval BR2_EXT_DESCS_${br2_name}="\"\${br2_desc:-\${br2_name}}\""
 }
 
 # Generate the .mk snippet that defines makefile variables
@@ -117,13 +120,10 @@ do_mk() {
     printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
     printf '\n'
 
-    # We can't use ${BR2_EXT_NAMES[@]} directly: it is not guaranteed
-    # to be in the order paths were added (because it is an associative
-    # array). So we need to iterate on BR2_EXT_NAMES, which is sorted
-    # in the order names were added (because it is an indexed array).
     printf 'BR2_EXTERNAL ?='
     for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        printf ' %s' "${BR2_EXT_PATHS["${br2_name}"]}"
+        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
+        printf ' %s' "${br2_ext}"
     done
     printf '\n'
 
@@ -138,8 +138,8 @@ do_mk() {
     fi
 
     for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
-        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
+        eval br2_desc="\"\${BR2_EXT_DESCS_${br2_name}}\""
+        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
         printf '\n'
         printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}"
         printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}"
@@ -165,8 +165,8 @@ do_kconfig() {
     printf '\n'
 
     for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
-        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
+        eval br2_desc="\"\${BR2_EXT_DESCS_${br2_name}}\""
+        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
         if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
             printf 'menu "%s"\n' "${br2_desc}"
         fi
-- 
2.7.4

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Arnout Vandecappelle]

On 19-11-16 11:52, Yann E. MORIN wrote:
> Currently, the br2-external script uses bash-4's associative arrays.
> 
> However, some oldish enterprise-class distros like RHEL5 still use
> bash-3.1 which lacks associative arrays.
> 
> We restore compatibility with those oldish distros using 'eval' to
> emulate associative arrays, as suggested by Arnout.
> 
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Max Filippov <jcmvbkbc@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Below some things I considered while reviewing.

> ---
>  support/scripts/br2-external | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 055dc08..84bc334 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -1,10 +1,12 @@
[snip]
> @@ -91,9 +93,10 @@ do_validate_one() {
>          error "'%s': name '%s' contains invalid chars: '%s'\n" \
>              "${br2_ext}" "${br2_name//\$/\$\$}" "${n//\$/\$\$}"
>      fi
> -    if [ -n "${BR2_EXT_PATHS["${br2_name}"]}" ]; then
> +    eval d="\"\${BR2_EXT_PATHS_${br2_name}}\""

 Just for bikeshedding comparison:

eval d="\"\${BR2_EXT_PATHS_${br2_name}}\""
eval d='"${BR2_EXT_PATHS_'${br2_name}'}"'

 Your version is better :-)


> +    if [ -n "${d}" ]; then
>          error "'%s': name '%s' is already used in '%s'\n" \
> -            "${br2_ext}" "${br2_name}" "${BR2_EXT_PATHS["${br2_name}"]}"
> +            "${br2_ext}" "${br2_name}" "${d}"
>      fi
>      br2_desc="$(sed -r -e '/^desc: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
>      if [ ! -f "${br2_ext}/external.mk" ]; then
> @@ -105,8 +108,8 @@ do_validate_one() {
>  
>      # Register this br2-external tree
>      BR2_EXT_NAMES+=( "${br2_name}" )
> -    BR2_EXT_PATHS["${br2_name}"]="${br2_ext}"
> -    BR2_EXT_DESCS["${br2_name}"]="${br2_desc:-${br2_name}}"
> +    eval BR2_EXT_PATHS_${br2_name}="\"\${br2_ext}\""
> +    eval BR2_EXT_DESCS_${br2_name}="\"\${br2_desc:-\${br2_name}}\""

 And another comparison:
eval BR2_EXT_PATHS_${br2_name}="\"\${br2_ext}\""
eval BR2_EXT_PATHS_${br2_name}='"${br2_ext}"'

 On its own, the version with single quotes is better here, but your version is
more consistent with the other eval lines.


>  }
>  
>  # Generate the .mk snippet that defines makefile variables
> @@ -117,13 +120,10 @@ do_mk() {
>      printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
>      printf '\n'
>  
> -    # We can't use ${BR2_EXT_NAMES[@]} directly: it is not guaranteed

 This comment was already wrong, it should have been BR2_EXT_PATHS. Without the
associative arrays, the option of using ${BR2_EXT_PATHS[@]} is no longer
available, that's why the comment can be removed.

 Perhaps the sentence above should have been part of the commit message :-)

 Regards,
 Arnout

> -    # to be in the order paths were added (because it is an associative
> -    # array). So we need to iterate on BR2_EXT_NAMES, which is sorted
> -    # in the order names were added (because it is an indexed array).
>      printf 'BR2_EXTERNAL ?='
>      for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        printf ' %s' "${BR2_EXT_PATHS["${br2_name}"]}"
> +        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
> +        printf ' %s' "${br2_ext}"
>      done
>      printf '\n'
>  
> @@ -138,8 +138,8 @@ do_mk() {
>      fi
>  
>      for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
> +        eval br2_desc="\"\${BR2_EXT_DESCS_${br2_name}}\""
> +        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
>          printf '\n'
>          printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}"
>          printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}"
> @@ -165,8 +165,8 @@ do_kconfig() {
>      printf '\n'
>  
>      for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
> +        eval br2_desc="\"\${BR2_EXT_DESCS_${br2_name}}\""
> +        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
>          if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
>              printf 'menu "%s"\n' "${br2_desc}"
>          fi
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Cam Hutchison]

Hi Yann,

Since eval is kind of icky, how about something like this patch instead?


Currently, the br2-external script uses bash-4's associative arrays.

However, some oldish enterprise-class distros like RHEL5 still use
bash-3.1 which lacks associative arrays.

Remove the arrays by reading the external.desc files as we generate the
output file.

Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: "Cam Hutchison" <camh@xdna.net>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Cam Hutchison <camh@xdna.net>
---
 support/scripts/br2-external | 94 +++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 41 deletions(-)

diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 055dc08..2afc88e 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -1,11 +1,6 @@
 #!/bin/bash
 set -e
 
-# The names and locations of the br2-external trees, once validated.
-declare -a BR2_EXT_NAMES
-declare -A BR2_EXT_PATHS
-declare -A BR2_EXT_DESCS
-
 # URL to manual for help in converting old br2-external trees.
 # Escape '#' so that make does not consider it a comment.
 MANUAL_URL='https://buildroot.org/manual.html\#br2-external-converting'
@@ -38,14 +33,15 @@ main() {
 
     exec >"${ofile}"
 
-    do_validate ${@//:/ }
+    # Split $PATH style argument into multiple arguments
+    set -- ${@//:/ }
 
-    do_${ofmt}
+    do_validate "${@}"
+
+    do_${ofmt} "${@}"
 }
 
-# Validates the br2-external trees passed as arguments. Makes each of
-# them canonical and store them in the global arrays BR2_EXT_NAMES
-# and BR2_EXT_PATHS.
+# Validates the br2-external trees passed as arguments.
 #
 # Note: since this script is always first called from Makefile context
 # to generate the Makefile fragment before it is called to generate the
@@ -61,14 +57,16 @@ do_validate() {
         return
     fi
 
-    for br2_ext in "${@}"; do
+    for br2_ext; do
         do_validate_one "${br2_ext}"
     done
+
+    validate_unique_names "${@}"
 }
 
 do_validate_one() {
     local br2_ext="${1}"
-    local br2_name br2_desc n
+    local br2_name n
 
     if [ ! -d "${br2_ext}" ]; then
         error "'%s': no such file or directory\n" "${br2_ext}"
@@ -80,7 +78,7 @@ do_validate_one() {
         error "'%s': does not have a name (in 'external.desc'). See %s\n" \
             "${br2_ext}" "${MANUAL_URL}"
     fi
-    br2_name="$(sed -r -e '/^name: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
+    br2_name="$(get_field "${br2_ext}" 'name')"
     if [ -z "${br2_name}" ]; then
         error "'%s/external.desc': does not define the name\n" "${br2_ext}"
     fi
@@ -91,55 +89,69 @@ do_validate_one() {
         error "'%s': name '%s' contains invalid chars: '%s'\n" \
             "${br2_ext}" "${br2_name//\$/\$\$}" "${n//\$/\$\$}"
     fi
-    if [ -n "${BR2_EXT_PATHS["${br2_name}"]}" ]; then
-        error "'%s': name '%s' is already used in '%s'\n" \
-            "${br2_ext}" "${br2_name}" "${BR2_EXT_PATHS["${br2_name}"]}"
-    fi
-    br2_desc="$(sed -r -e '/^desc: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
     if [ ! -f "${br2_ext}/external.mk" ]; then
         error "'%s/external.mk': no such file or directory\n" "${br2_ext}"
     fi
     if [ ! -f "${br2_ext}/Config.in" ]; then
         error "'%s/Config.in': no such file or directory\n" "${br2_ext}"
     fi
+}
 
-    # Register this br2-external tree
-    BR2_EXT_NAMES+=( "${br2_name}" )
-    BR2_EXT_PATHS["${br2_name}"]="${br2_ext}"
-    BR2_EXT_DESCS["${br2_name}"]="${br2_desc:-${br2_name}}"
+validate_unique_names() {
+    # If there are any duplicate names across the external.desc files,
+    # generate_dupes will output at least two lines of the form:
+    # <br2_ext_1> name
+    # <br2_ext_2> name
+    # If there is more that one duplicate, extra lines will be output but
+    # we do not include them in the error message due to formatting limitations
+    # of the error message. Once one duplicate is fixed, subsequent runs will
+    # identify the next duplicate(s).
+    set -- $(generate_dupes "${@}")
+    if [ ${#} -ne 0  ]; then
+        error "'%s': name '%s' is already used in '%s'\n" \
+            "${1}" "${2}" "${3}"
+    fi
+}
+
+generate_dupes() {
+    for br2_ext; do
+        printf '%s %s\n' ${br2_ext} "$(get_field "${br2_ext}" 'name')"
+    done | sort -k2,2 | uniq --skip-fields=1 --all-repeated
+}
+
+# Get a field by name from an external.desc file
+get_field() {
+    local br2_ext="${1}"
+    local field="${2}"
+
+    sed -n -r -e "s/^${field}"': +(.*)$/\1/p' "${br2_ext}/external.desc"
 }
 
 # Generate the .mk snippet that defines makefile variables
 # for the br2-external tree
 do_mk() {
-    local br2_name br2_ext
+    local br2_ext br2_name br2_desc
 
     printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
     printf '\n'
 
-    # We can't use ${BR2_EXT_NAMES[@]} directly: it is not guaranteed
-    # to be in the order paths were added (because it is an associative
-    # array). So we need to iterate on BR2_EXT_NAMES, which is sorted
-    # in the order names were added (because it is an indexed array).
     printf 'BR2_EXTERNAL ?='
-    for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        printf ' %s' "${BR2_EXT_PATHS["${br2_name}"]}"
-    done
+    printf ' %s' "${@}"
     printf '\n'
 
     printf 'BR2_EXTERNAL_NAMES = \n'
     printf 'BR2_EXTERNAL_DIRS = \n'
     printf 'BR2_EXTERNAL_MKS = \n'
 
-    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
+    if [ ${#} -eq 0 ]; then
         printf '\n'
         printf '# No br2-external tree defined.\n'
         return
     fi
 
-    for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
-        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
+    for br2_ext; do
+        br2_name="$(get_field "${br2_ext}" 'name')"
+        br2_desc="$(get_field "${br2_ext}" 'desc')"
         printf '\n'
         printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}"
         printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}"
@@ -151,12 +163,12 @@ do_mk() {
 
 # Generate the kconfig snippet for the br2-external tree.
 do_kconfig() {
-    local br2_name br2_ext
+    local br2_ext br2_name br2_desc
 
     printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
     printf '\n'
 
-    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
+    if [ ${#} -eq 0 ]; then
         printf '# No br2-external tree defined.\n'
         return
     fi
@@ -164,10 +176,10 @@ do_kconfig() {
     printf 'menu "External options"\n'
     printf '\n'
 
-    for br2_name in "${BR2_EXT_NAMES[@]}"; do
-        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
-        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
-        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
+    for br2_ext; do
+        br2_name="$(get_field "${br2_ext}" 'name')"
+        br2_desc="$(get_field "${br2_ext}" 'desc')"
+        if [ ${#} -gt 1 ]; then
             printf 'menu "%s"\n' "${br2_desc}"
         fi
         printf 'comment "%s (in %s)"\n' "${br2_desc}" "${br2_ext}"
@@ -175,7 +187,7 @@ do_kconfig() {
         printf '\tstring\n'
         printf '\tdefault "%s"\n' "${br2_ext}"
         printf 'source "%s/Config.in"\n' "${br2_ext}"
-        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
+        if [ ${#} -gt 1 ]; then
             printf 'endmenu # %s\n' "${br2_name}"
         fi
         printf '\n'
-- 
2.1.4

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Ricardo Martincoski]

Cam,

Those first lines of the commit message should be placed after the line with
'---', because 'git am' would automatically remove them.
There is also a duplicated SoB.
No need to resend because of these, it can be manually fixed when applying.

I didn't review the code, I only tested it.

On Sat, Nov 19, 2016 at 08:53 PM, Cam Hutchison wrote:

> Hi Yann,
> 
> Since eval is kind of icky, how about something like this patch instead?
> 
> 
> Currently, the br2-external script uses bash-4's associative arrays.
> 
> However, some oldish enterprise-class distros like RHEL5 still use
> bash-3.1 which lacks associative arrays.
> 
> Remove the arrays by reading the external.desc files as we generate the
> output file.
> 
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Cam Hutchison" <camh@xdna.net>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Cam Hutchison <camh@xdna.net>
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Simple tests using CentOS 5, bash 3.2.25:
1. make defconfig (in-tree build configuration);
2. created 2 small br2-external with a package and a defconfig each and
   successfully configured out-of-tree build with multiple br2-external
   using both defconfigs and downloaded the sources of both packages.]

Regards,
Ricardo

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Cam Hutchison]

Thanks for the testing, Ricardo.

Noted about the workflow errors. I was just throwing this out for
comment right now. I can spin it properly if needed, but as you say -
also easily adjusted on commit.

One thing I wasn't sure about and forgot to comment on when I posted
was the invocation of uniq(1). I'm using some GNU options and I'm not
sure when they were added to GNU uniq and what version RHEL5 has.

Can you confirm that you can invoke without error:
$ uniq --skip-fields=1 --all-repeated

If not, the generate_dupes() function will need to be fixed.


On 20 November 2016 at 16:18, Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
> Cam,
>
> Those first lines of the commit message should be placed after the line with
> '---', because 'git am' would automatically remove them.
> There is also a duplicated SoB.
> No need to resend because of these, it can be manually fixed when applying.
>
> I didn't review the code, I only tested it.
>
> On Sat, Nov 19, 2016 at 08:53 PM, Cam Hutchison wrote:
>
>> Hi Yann,
>>
>> Since eval is kind of icky, how about something like this patch instead?
>>
>>
>> Currently, the br2-external script uses bash-4's associative arrays.
>>
>> However, some oldish enterprise-class distros like RHEL5 still use
>> bash-3.1 which lacks associative arrays.
>>
>> Remove the arrays by reading the external.desc files as we generate the
>> output file.
>>
>> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> Signed-off-by: "Cam Hutchison" <camh@xdna.net>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>> Signed-off-by: Cam Hutchison <camh@xdna.net>
> Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> [Simple tests using CentOS 5, bash 3.2.25:
> 1. make defconfig (in-tree build configuration);
> 2. created 2 small br2-external with a package and a defconfig each and
>    successfully configured out-of-tree build with multiple br2-external
>    using both defconfigs and downloaded the sources of both packages.]
>
> Regards,
> Ricardo

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Ricardo Martincoski]

Cam,

On Sun, Nov 20, 2016 at 06:40 AM, Cam Hutchison wrote:

> Thanks for the testing, Ricardo.
> 
> Noted about the workflow errors. I was just throwing this out for
> comment right now. I can spin it properly if needed, but as you say -
> also easily adjusted on commit.
> 
> One thing I wasn't sure about and forgot to comment on when I posted
> was the invocation of uniq(1). I'm using some GNU options and I'm not
> sure when they were added to GNU uniq and what version RHEL5 has.
> 
> Can you confirm that you can invoke without error:
> $ uniq --skip-fields=1 --all-repeated

Sure.

[ricardo at centos5 ~]$ grep release /etc/issue
CentOS release 5.11 (Final)
[ricardo at centos5 ~]$ uniq --version | grep uniq
uniq (GNU coreutils) 5.97
[ricardo at centos5 ~]$ uniq --skip-fields=1 --all-repeated

No error message is generated and uniq is waiting for stdin.

[ricardo at centos5 ~]$ uniq --help
Usage: uniq [OPTION]... [INPUT [OUTPUT]]
Discard all but one of successive identical lines from INPUT (or
standard input), writing to OUTPUT (or standard output).

Mandatory arguments to long options are mandatory for short options too.
  -c, --count           prefix lines by the number of occurrences
  -d, --repeated        only print duplicate lines
  -D, --all-repeated[=delimit-method] print all duplicate lines
                        delimit-method={none(default),prepend,separate}
                        Delimiting is done with blank lines.
  -f, --skip-fields=N   avoid comparing the first N fields
  -i, --ignore-case     ignore differences in case when comparing
  -s, --skip-chars=N    avoid comparing the first N characters
  -u, --unique          only print unique lines
  -w, --check-chars=N   compare no more than N characters in lines
      --help     display this help and exit
      --version  output version information and exit

A field is a run of whitespace, then non-whitespace characters.
Fields are skipped before chars.

Report bugs to <bug-coreutils@gnu.org>.
[ricardo at centos5 ~]$

> 
> If not, the generate_dupes() function will need to be fixed.
> 

Regards,
Ricardo

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Yann E. MORIN]

Cam, All,

(Arnout, Thomas, question for you at the end)

On 2016-11-19 22:53 -0000, Cam Hutchison spake thusly:
> Since eval is kind of icky, how about something like this patch instead?

I don't like it much. It is very intrusive, when we are so close to the
release...

You are entirely getting rid of arrays, even the indexed arrays, even
though bash has had support for them for ages now, and even RHEL-4 (4!)
has a bash that supports indexed arrays...

Getting rid of the indxed arrays complexifies the script quite a bit.
Also, using a function to retrieve fields makes the code even yet a
little bit more complex.

Indeed, the eval are icky, but they are pretty much contained.

Plus, see a few comments below...

> Currently, the br2-external script uses bash-4's associative arrays.
> 
> However, some oldish enterprise-class distros like RHEL5 still use
> bash-3.1 which lacks associative arrays.
> 
> Remove the arrays by reading the external.desc files as we generate the
> output file.
> 
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Cam Hutchison" <camh@xdna.net>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Cam Hutchison <camh@xdna.net>
> ---
>  support/scripts/br2-external | 94 +++++++++++++++++++++++++-------------------
>  1 file changed, 53 insertions(+), 41 deletions(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 055dc08..2afc88e 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -61,14 +57,16 @@ do_validate() {
>          return
>      fi
>  
> -    for br2_ext in "${@}"; do
> +    for br2_ext; do

I always find this notation to be puzzling. Yes, I know it uses the
positional arguments, but I don;t like implicit behaviour; it always
leads to maintenance burden.

Just keep the explicit iteration:   for br2_ext in "${@}"; do

[--SNIP--]
> +validate_unique_names() {
> +    # If there are any duplicate names across the external.desc files,
> +    # generate_dupes will output at least two lines of the form:
> +    # <br2_ext_1> name
> +    # <br2_ext_2> name

In fact, the full output would be something like:

    br2-ext-1 name-A
    br2-ext-2 name-A
    br2-ext-3 name-A
    br2-ext-4 name-B
    br2-ext-5 name-B

> +    # If there is more that one duplicate, extra lines will be output but

At first, I was not sure what you meant here. But you meant that, in the
example above, br2-ext-3 will not be reported, and neither would
br2-ext-4 and br2-ext-5.

(That's OK, the current code does not report them either).

> +    # we do not include them in the error message due to formatting limitations
> +    # of the error message. Once one duplicate is fixed, subsequent runs will
> +    # identify the next duplicate(s).
> +    set -- $(generate_dupes "${@}")
> +    if [ ${#} -ne 0  ]; then
> +        error "'%s': name '%s' is already used in '%s'\n" \
> +            "${1}" "${2}" "${3}"
> +    fi
> +}
> +

The documentation of the output of generate_dupes should go there:

   # ...here.
> +generate_dupes() {
> +    for br2_ext; do
> +        printf '%s %s\n' ${br2_ext} "$(get_field "${br2_ext}" 'name')"
> +    done | sort -k2,2 | uniq --skip-fields=1 --all-repeated
> +}
> +
> +# Get a field by name from an external.desc file
> +get_field() {
> +    local br2_ext="${1}"
> +    local field="${2}"
> +
> +    sed -n -r -e "s/^${field}"': +(.*)$/\1/p' "${br2_ext}/external.desc"

Mixing quotes is always tricky... ;-) I try to avoid it when possible:

    "s/^${field}: +(.*)\$/\\1/p"

>  }
>  
>  # Generate the .mk snippet that defines makefile variables
>  # for the br2-external tree
>  do_mk() {
> -    local br2_name br2_ext
> +    local br2_ext br2_name br2_desc
>  
>      printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
>      printf '\n'
>  
> -    # We can't use ${BR2_EXT_NAMES[@]} directly: it is not guaranteed
> -    # to be in the order paths were added (because it is an associative
> -    # array). So we need to iterate on BR2_EXT_NAMES, which is sorted
> -    # in the order names were added (because it is an indexed array).
>      printf 'BR2_EXTERNAL ?='
> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        printf ' %s' "${BR2_EXT_PATHS["${br2_name}"]}"
> -    done
> +    printf ' %s' "${@}"
>      printf '\n'

You can do that in a single command:

    printf 'BR2_EXTERNAL ?= %s\n' "${*}"

>  
>      printf 'BR2_EXTERNAL_NAMES = \n'
>      printf 'BR2_EXTERNAL_DIRS = \n'
>      printf 'BR2_EXTERNAL_MKS = \n'
>  
> -    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
> +    if [ ${#} -eq 0 ]; then
>          printf '\n'
>          printf '# No br2-external tree defined.\n'
>          return
>      fi
>  
> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
> +    for br2_ext; do

Please, keep explicit listing of positional arguments.

> +        br2_name="$(get_field "${br2_ext}" 'name')"
> +        br2_desc="$(get_field "${br2_ext}" 'desc')"
>          printf '\n'
>          printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}"
>          printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}"
> @@ -151,12 +163,12 @@ do_mk() {
>  
>  # Generate the kconfig snippet for the br2-external tree.
>  do_kconfig() {
> -    local br2_name br2_ext
> +    local br2_ext br2_name br2_desc
>  
>      printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
>      printf '\n'
>  
> -    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
> +    if [ ${#} -eq 0 ]; then
>          printf '# No br2-external tree defined.\n'
>          return
>      fi
> @@ -164,10 +176,10 @@ do_kconfig() {
>      printf 'menu "External options"\n'
>      printf '\n'
>  
> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
> -        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
> +    for br2_ext; do

Please, keep explicit listing of positional arguments.

However, I'm not sure you should fix and respin, I think this patch is a
little bit too intrusive and makes the code a little bit more complex
than really needed.

Yet, let's see what others think. Arnout, Thomas?

Regards,
Yann E. MORIN.

> +        br2_name="$(get_field "${br2_ext}" 'name')"
> +        br2_desc="$(get_field "${br2_ext}" 'desc')"
> +        if [ ${#} -gt 1 ]; then
>              printf 'menu "%s"\n' "${br2_desc}"
>          fi
>          printf 'comment "%s (in %s)"\n' "${br2_desc}" "${br2_ext}"
> @@ -175,7 +187,7 @@ do_kconfig() {
>          printf '\tstring\n'
>          printf '\tdefault "%s"\n' "${br2_ext}"
>          printf 'source "%s/Config.in"\n' "${br2_ext}"
> -        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
> +        if [ ${#} -gt 1 ]; then
>              printf 'endmenu # %s\n' "${br2_name}"
>          fi
>          printf '\n'
> -- 
> 2.1.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] core/br2-external: restore compatibility with old distros

[Cam Hutchison]

Yann,

Thanks for the review.

I often said in my last job that if you get five people to review your
shell script, you'll get ten opinions. Something about shell that
gives such a diversity of views.


On 24 November 2016 at 08:57, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> I don't like it much. It is very intrusive, when we are so close to the
> release...

It's a fair point but the script is small enough that it can easily be
tested with a variety of inputs to validate it, which I did. There was
a one character difference between the original script and this one in
one test case but that was not significant.

> You are entirely getting rid of arrays, even the indexed arrays, even
> though bash has had support for them for ages now, and even RHEL-4 (4!)
> has a bash that supports indexed arrays...

Sure. I was presenting a solution that removed the arrays altogether.
The approach I took did not need any arrays, so while the direct
problem was the associative arrays, just getting rid of them and
leaving an indexed array did not make any sense.

> Getting rid of the indxed arrays complexifies the script quite a bit.

Really? To me this is so much simpler. The state has been removed, and
state is always an additional cognitive and testing burden. The script
functions become functional in that their output is a product of their
input. This makes testing simpler and reduces the scope you need to
keep in your head to understand a function.

> Also, using a function to retrieve fields makes the code even yet a
> little bit more complex.

Factoring into smaller functions like this is both a benefit and a
hindrance. It makes the initial reading of the code a bit harder as
you need to understand the abstractions, but once understood it makes
it easier. Since the function is a single line, it is easy to verify
but since it also reduces repetition the complexity is reduced.

>
> Indeed, the eval are icky, but they are pretty much contained.
>
> Plus, see a few comments below...
>
>> Currently, the br2-external script uses bash-4's associative arrays.
>>
>> However, some oldish enterprise-class distros like RHEL5 still use
>> bash-3.1 which lacks associative arrays.
>>
>> Remove the arrays by reading the external.desc files as we generate the
>> output file.
>>
>> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> Signed-off-by: "Cam Hutchison" <camh@xdna.net>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>> Signed-off-by: Cam Hutchison <camh@xdna.net>
>> ---
>>  support/scripts/br2-external | 94 +++++++++++++++++++++++++-------------------
>>  1 file changed, 53 insertions(+), 41 deletions(-)
>>
>> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
>> index 055dc08..2afc88e 100755
>> --- a/support/scripts/br2-external
>> +++ b/support/scripts/br2-external
>> @@ -61,14 +57,16 @@ do_validate() {
>>          return
>>      fi
>>
>> -    for br2_ext in "${@}"; do
>> +    for br2_ext; do
>
> I always find this notation to be puzzling. Yes, I know it uses the
> positional arguments, but I don;t like implicit behaviour; it always
> leads to maintenance burden.
>
> Just keep the explicit iteration:   for br2_ext in "${@}"; do

I can't disagree more with this. In the expanded form, there are four
ways you can iterate arguments of which only one is correct. I have
seen the incorrect ways too often that now when I review code, I tell
people to use the form I use here. The four ways are:

for var in $*
for var in "$*"
for var in $@
for var in "$@"

Not many people understand the different expansions here that I avoid
it altogether. "for var" is valid shell and is *more explicit* in a
way because it means only one thing - iterate over the positional
parameters, correctly maintaining word boundaries. You can't get the
quotes wrong, you can't use the wrong expansion and you can't
accidentally drop in an extra argument from a typo.

>
> [--SNIP--]
>> +validate_unique_names() {
>> +    # If there are any duplicate names across the external.desc files,
>> +    # generate_dupes will output at least two lines of the form:
>> +    # <br2_ext_1> name
>> +    # <br2_ext_2> name
>
> In fact, the full output would be something like:
>
>     br2-ext-1 name-A
>     br2-ext-2 name-A
>     br2-ext-3 name-A
>     br2-ext-4 name-B
>     br2-ext-5 name-B

Yes, and they are both the same *form*. And I did say "at least two
lines". The extra verbiage is not relevant so I omitted it.

>
>> +    # If there is more that one duplicate, extra lines will be output but
>
> At first, I was not sure what you meant here. But you meant that, in the
> example above, br2-ext-3 will not be reported, and neither would
> br2-ext-4 and br2-ext-5.
>
> (That's OK, the current code does not report them either).
>
>> +    # we do not include them in the error message due to formatting limitations
>> +    # of the error message. Once one duplicate is fixed, subsequent runs will
>> +    # identify the next duplicate(s).
>> +    set -- $(generate_dupes "${@}")
>> +    if [ ${#} -ne 0  ]; then
>> +        error "'%s': name '%s' is already used in '%s'\n" \
>> +            "${1}" "${2}" "${3}"
>> +    fi
>> +}
>> +
>
> The documentation of the output of generate_dupes should go there:

I did put it there at first, but what I found in the end was that how
the output was used was more important to understand. I could put a
"head -n 2" in generate_dupes and then move the comment there which
would also make some sense.

>
>    # ...here.
>> +generate_dupes() {
>> +    for br2_ext; do
>> +        printf '%s %s\n' ${br2_ext} "$(get_field "${br2_ext}" 'name')"
>> +    done | sort -k2,2 | uniq --skip-fields=1 --all-repeated
>> +}
>> +
>> +# Get a field by name from an external.desc file
>> +get_field() {
>> +    local br2_ext="${1}"
>> +    local field="${2}"
>> +
>> +    sed -n -r -e "s/^${field}"': +(.*)$/\1/p' "${br2_ext}/external.desc"
>
> Mixing quotes is always tricky... ;-) I try to avoid it when possible:
>
>     "s/^${field}: +(.*)\$/\\1/p"

Yeah. There's no sanity here. I hate backslash-quoting things that if
I can put in the right quotes to avoid it, I will. The mashed-together
double- and single-quote looks a lot more confusing now I see it in my
mail client rather than my editor.

>
>>  }
>>
>>  # Generate the .mk snippet that defines makefile variables
>>  # for the br2-external tree
>>  do_mk() {
>> -    local br2_name br2_ext
>> +    local br2_ext br2_name br2_desc
>>
>>      printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
>>      printf '\n'
>>
>> -    # We can't use ${BR2_EXT_NAMES[@]} directly: it is not guaranteed
>> -    # to be in the order paths were added (because it is an associative
>> -    # array). So we need to iterate on BR2_EXT_NAMES, which is sorted
>> -    # in the order names were added (because it is an indexed array).
>>      printf 'BR2_EXTERNAL ?='
>> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
>> -        printf ' %s' "${BR2_EXT_PATHS["${br2_name}"]}"
>> -    done
>> +    printf ' %s' "${@}"
>>      printf '\n'
>
> You can do that in a single command:
>
>     printf 'BR2_EXTERNAL ?= %s\n' "${*}"

Ack. Good call.

>
>>
>>      printf 'BR2_EXTERNAL_NAMES = \n'
>>      printf 'BR2_EXTERNAL_DIRS = \n'
>>      printf 'BR2_EXTERNAL_MKS = \n'
>>
>> -    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
>> +    if [ ${#} -eq 0 ]; then
>>          printf '\n'
>>          printf '# No br2-external tree defined.\n'
>>          return
>>      fi
>>
>> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
>> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
>> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
>> +    for br2_ext; do
>
> Please, keep explicit listing of positional arguments.

Ack. See rationale above.

>
>> +        br2_name="$(get_field "${br2_ext}" 'name')"
>> +        br2_desc="$(get_field "${br2_ext}" 'desc')"
>>          printf '\n'
>>          printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}"
>>          printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}"
>> @@ -151,12 +163,12 @@ do_mk() {
>>
>>  # Generate the kconfig snippet for the br2-external tree.
>>  do_kconfig() {
>> -    local br2_name br2_ext
>> +    local br2_ext br2_name br2_desc
>>
>>      printf '#\n# Automatically generated file; DO NOT EDIT.\n#\n'
>>      printf '\n'
>>
>> -    if [ ${#BR2_EXT_NAMES[@]} -eq 0 ]; then
>> +    if [ ${#} -eq 0 ]; then
>>          printf '# No br2-external tree defined.\n'
>>          return
>>      fi
>> @@ -164,10 +176,10 @@ do_kconfig() {
>>      printf 'menu "External options"\n'
>>      printf '\n'
>>
>> -    for br2_name in "${BR2_EXT_NAMES[@]}"; do
>> -        br2_desc="${BR2_EXT_DESCS["${br2_name}"]}"
>> -        br2_ext="${BR2_EXT_PATHS["${br2_name}"]}"
>> -        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
>> +    for br2_ext; do
>
> Please, keep explicit listing of positional arguments.

Ack. See rationale above.

>
> However, I'm not sure you should fix and respin, I think this patch is a
> little bit too intrusive and makes the code a little bit more complex
> than really needed.
>
> Yet, let's see what others think. Arnout, Thomas?

I've got no attachment to this - to me, this code was simpler so I
threw it out there (and I hate eval so much - forget trying to quote
sanely with eval). Up to you guys.

>
> Regards,
> Yann E. MORIN.
>
>> +        br2_name="$(get_field "${br2_ext}" 'name')"
>> +        br2_desc="$(get_field "${br2_ext}" 'desc')"
>> +        if [ ${#} -gt 1 ]; then
>>              printf 'menu "%s"\n' "${br2_desc}"
>>          fi
>>          printf 'comment "%s (in %s)"\n' "${br2_desc}" "${br2_ext}"
>> @@ -175,7 +187,7 @@ do_kconfig() {
>>          printf '\tstring\n'
>>          printf '\tdefault "%s"\n' "${br2_ext}"
>>          printf 'source "%s/Config.in"\n' "${br2_ext}"
>> -        if [ ${#BR2_EXT_NAMES[@]} -gt 1 ]; then
>> +        if [ ${#} -gt 1 ]; then
>>              printf 'endmenu # %s\n' "${br2_name}"
>>          fi
>>          printf '\n'
>> --
>> 2.1.4
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  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] core/br2-external: restore compatibility with old distros

[Ricardo Martincoski]

Yann,

On Sat, Nov 19, 2016 at 08:52 AM, Yann E. MORIN wrote:

> Currently, the br2-external script uses bash-4's associative arrays.
> 
> However, some oldish enterprise-class distros like RHEL5 still use
> bash-3.1 which lacks associative arrays.
> 
> We restore compatibility with those oldish distros using 'eval' to
> emulate associative arrays, as suggested by Arnout.
> 
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Simple tests using CentOS 5, bash 3.2.25:
1. make defconfig (in-tree build configuration);
2. created 2 small br2-external with a package and a defconfig each and
   successfully configured out-of-tree build with multiple br2-external
   using both defconfigs and downloaded the sources of both packages.]

Regards,
Ricardo

[Buildroot] [PATCH] core/br2-external: restore compatibility with old distros

[Thomas Petazzoni]

Hello,

On Sat, 19 Nov 2016 11:52:26 +0100, Yann E. MORIN wrote:
> Currently, the br2-external script uses bash-4's associative arrays.
> 
> However, some oldish enterprise-class distros like RHEL5 still use
> bash-3.1 which lacks associative arrays.
> 
> We restore compatibility with those oldish distros using 'eval' to
> emulate associative arrays, as suggested by Arnout.
> 
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  support/scripts/br2-external | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)

Applied to master, thanks.

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