[Mesa-dev] [PATCH] bin/get-fixes-pick-list.sh: don't warn if more than one, go over them

[Andres Gomez]

On Mon, 2017-05-08 at 00:48 +0100, Eric Engestrom wrote:
> On Saturday, 2017-05-06 17:09:35 +0300, Andres Gomez wrote:
> > If an identified commit was having more than one fix, we would warn
> > about that and only treat the first.
> > 
> > Now, we don't warn but treat all of them.
> 
> This was on my todo list, thanks for fixing this :)
> 
> > 
> > Signed-off-by: Andres Gomez <agomez at igalia.com>
> > ---
> >  bin/get-fixes-pick-list.sh | 51 +++++++++++++++++++++++-----------------------
> >  1 file changed, 26 insertions(+), 25 deletions(-)
> > 
> > diff --git a/bin/get-fixes-pick-list.sh b/bin/get-fixes-pick-list.sh
> > index f1398f320c..cf95f28377 100755
> > --- a/bin/get-fixes-pick-list.sh
> > +++ b/bin/get-fixes-pick-list.sh
> > @@ -33,37 +33,38 @@ do
> >  
> >  	# For each one try to extract the tag
> >  	fixes_count=`git show $sha | grep -i "fixes:" | wc -l`
> > -	if [ "x$fixes_count" != x1 ] ; then
> > -		printf "WARNING: Commit \"%s\" has more than one Fixes tag\n" \
> > -		       "`git log -n1 --pretty=oneline $sha`"
> > -	fi
> > -	fixes=`git show $sha | grep -i "fixes:" | head -n 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 ' '`
> > +	while [ $fixes_count -gt 0 ] ; do
> > +		fixes=`git show $sha | grep -i "fixes:" | tail -n $fixes_count | head -n 1`
> 
> This will break when there are line breaks within a `Fixes:`. Consider
> something like the following:
> 
> Fixes: ddb99127a6f6c6d0284f3395e0b06aed575f0ecd ("egl/x11: Honor the
>        EGL_PLATFORM_X11_SCREEN_EXT attribute")
> 
> I'm not sure how best to fix this though. Replace newlines that are not
> followed by `Fixes:` with a space, before splitting and parsing each
> line?

Yes, I also realized about that. I don't know how better to solve this
either but I'll keep thinking about it.

> I guess this is better than what we currently have, and it won't fail
> more than it would before, so it's good enough for now;
> Reviewed-by: Eric Engestrom <eric at engestrom.ch>

Thanks for the review!

-- 
Br,

Andres