Re: [gentoo-dev] [PATCH] flag-o-matic.eclass: bugfix for get-flag()

[rindeal]

> Dnia 15 maja 2016 15:31:29 CEST, Jan Chren <dev.rindeal@×××××.com> napisał(a):
>>- fix case:
>>  - `CFLAGS='-O1 -O2'`
>>  - `get-flag '-O*'`
>>  - before `-O1`
>>  - now `-O2`
>>- fix case:
>>  - `CFLAGS='-W1,-O1'`
>>  - `get-flag '-O*'`
>>  - before `-W1,O1`
>>  - now return 1
>>
>>`get-flag march` == "i686" syntax still works.
>
> Could you add appropriate test cases, in the tests subdirectory?

Done

>
>>---
>> eclass/flag-o-matic.eclass | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>>diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
>>index e0b19e9..f670320 100644
>>--- a/eclass/flag-o-matic.eclass
>>+++ b/eclass/flag-o-matic.eclass
>>@@ -535,7 +535,7 @@ strip-unsupported-flags() {
>> # @DESCRIPTION:
>> # Find and echo the value for a particular flag.  Accepts shell globs.
>> get-flag() {
>>-      local f var findflag="$1"
>>+      local var findflag="${1}"
>>
>>       # this code looks a little flaky but seems to work for
>>       # everything we want ...
>>@@ -543,11 +543,16 @@ get-flag() {
>>       # `get-flag -march` == "-march=i686"
>>       # `get-flag march` == "i686"
>>       for var in $(all-flag-vars) ; do
>>-              for f in ${!var} ; do
>>-                      if [ "${f/${findflag}}" != "${f}" ] ; then
>>-                              printf "%s\n" "${f/-${findflag}=}"
>>+              # reverse loop
>>+              set -- ${!var}
>>+              local i=$#
>
> You are using $ with and without braces inconsistently. Please stick to one form.

Done

>
>>+              while [ $i -gt 0 ] ; do
>
> Please use [[ ]] for conditionals. It has some nice bash magic that makes them whitespace-safe.

Although always a number, but done

>
>>+                      local f="${!i}"
>>+                      if [ "${f#-${findflag#-}}" != "${f}" ] ; then
>
> I know the original code sucked as well but could you replace this with more readable [[ ${f} == -${findflag#-}* ]] or alike (note: not tested).

This is just as buggy as my original implementation, I've reworked it
and thanks to the tests I hope it's now correct.

>
>>+                              printf "%s\n" "${f#-${findflag}=}"
>
> It may be a good idea to add a short explanation why you can't use echo here, as a comment.

I've just copied what was there before, `echo` in bash is notoriously
wild, but with this simple string I guess it's ok, so done.

>
>>                               return 0
>>                       fi
>>+                      ((i--))
>>               done
>>       done
>>       return 1
>
>

apart from the tests, the patch now looks like this:

 eclass/flag-o-matic.eclass | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
index e0b19e9..a8a792e 100644
--- a/eclass/flag-o-matic.eclass
+++ b/eclass/flag-o-matic.eclass
@@ -535,7 +535,7 @@ strip-unsupported-flags() {
 # @DESCRIPTION:
 # Find and echo the value for a particular flag.  Accepts shell globs.
 get-flag() {
-   local f var findflag="$1"
+   local var pattern="${1}"

    # this code looks a little flaky but seems to work for
    # everything we want ...
@@ -543,11 +543,21 @@ get-flag() {
    # `get-flag -march` == "-march=i686"
    # `get-flag march` == "i686"
    for var in $(all-flag-vars) ; do
-       for f in ${!var} ; do
-           if [ "${f/${findflag}}" != "${f}" ] ; then
-               printf "%s\n" "${f/-${findflag}=}"
+       # reverse loop
+       set -- ${!var}
+       local i=${#}
+       while [[ ${i} > 0 ]] ; do
+           local arg="${!i}"
+           local needle="-${pattern#-}" # force dash on
+           local haystack="${arg%%=*}" # we're comparing flags, not values
+           if [[ ${haystack##${needle}} == '' ]] ; then
+               local ret
+               # preserve only value if only flag name was provided
+               ret="${arg#-${pattern}=}"
+               echo "${ret}"
                return 0
            fi
+           ((i--))
        done
    done
    return 1
--
2.7.3