[Mesa-dev] [PATCH 2/2] bin/get-fixes-pick-list.sh: better identify multiple "fixes:" tags

[Emil Velikov]

On 13 May 2017 at 01:11, Andres Gomez <agomez at igalia.com> wrote:
> We were not considering as multiple fixes lines with:
> Fixes: $sha_1, Fixes: $sha_2
>
> Now, we split the lines so we will consider them individually, as in:
> Fixes: $sha_1,
> Fixes: $sha_2
>
> Additionally, we try to get the SHA from split lines so:
> Fixes:
> $sha_1
>
> Will be considered as:
> Fixes: $sha_1
>
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
>  bin/get-fixes-pick-list.sh | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/bin/get-fixes-pick-list.sh b/bin/get-fixes-pick-list.sh
> index f9afcc49ce..32d830cda0 100755
> --- a/bin/get-fixes-pick-list.sh
> +++ b/bin/get-fixes-pick-list.sh
> @@ -36,14 +36,18 @@ do
>                 continue
>         fi
>
> +       # Place every "fixes:" tag on its own line and join with the next word
> +       # on its line or a later one.
> +       fixes=`git show -s $sha | tr -d "\n" | sed -e 's/fixes:/\nfixes:/Ig' | grep "fixes:" | sed -e 's/\(fixes:\)[[:space:]]*\([a-zA-Z0-9]*\).*$/\1\2/g'`
Do we need the tr -d?

Nit: Let's also handle any trailing whitespace after the tag.
Something like the following should do it.

's/fixes:[[:space:]]*/\nfixes:/Ig'


> +
>         # For each one try to extract the tag
> -       fixes_count=`git show -s $sha | grep -i "fixes:" | wc -l`
> +       fixes_count=`echo "$fixes" | wc -l`
>         warn=`(test $fixes_count -gt 1 && echo $fixes_count) || echo 0`
>         while [ $fixes_count -gt 0 ] ; do
> -               fixes=`git show -s $sha | grep -i "fixes:" | tail -n $fixes_count`
> +               # Treat only the current line
> +               fix=`echo "$fixes" | tail -n $fixes_count | head -n 1`
>                 fixes_count=$(($fixes_count-1))
> -               # The following sed/cut combination is borrowed from GregKH
> -               id=`echo ${fixes} | sed -e 's/^[ \t]*//' | cut -f 2 -d ':' | sed -e 's/^[ \t]*//' | cut -f 1 -d ' '`
> +               id=`echo "$fix" | cut -d : -f 2`
Nit: fold the fix line in here?

With the nitpicks the series is
Reviewed-by: Emil Velikov <emli.velikov at collabora.com>

-Emil

P.S. An alternative construct that just hit me. It allows us to drop
the "tail | head" dance, drop the count variable and make the whole
thing a bit easier to read.
I'm quite biased of course, but if you agree I can address it as a
follow-up patch.

    # try to fold ill-formed fixes tags
    fixes=`git show -s $sha ...
    count=... // obsolete? see below
    warn=0
    save_IFS...
    IFS=\n
    for i in fixes; do
        $process
        if failed; then
            # warn if we fail to parse any of the tags
            warn=1
            continue
        fi
    done
    restore_IFS

    # only if there's multiple?
    # with the sed pattern fixed, we well drop the multiple 'requirement'
    if warn /* && count -gt 1 */; then
        echo WARNING
    fi