Re: [Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
On Tue, 2018-12-18 at 10:47 +, Emil Velikov wrote:
> On Mon, 17 Dec 2018 at 22:35, Andres Gomez wrote:
> >
> > On Mon, 2018-12-17 at 18:36 +, Emil Velikov wrote:
> > > On Mon, 17 Dec 2018 at 18:14, Andres Gomez wrote:
> > > >
> > > > On Mon, 2018-12-17 at 16:43 +, Emil Velikov wrote:
> > > > > Currently our is_sha_nomination does:
> > > > > - folds any whitespace, attempting to extract sha-like information
> > > > > - checks that at least one of the shas has landed
> > > > >
> > > > > Split it in two and do sha-like validation first.
> > > > >
> > > > > This way, commits with mesa-stable and sha nominations will feature
> > > > > the
> > > > > fixes/revert/etc instead of stable (a) or will be omitted if not
> > > > > applicable for the respective branch (b).
> > > > >
> > > > > Misc examples from 18.3
> > > > >
> > > > > (a)
> > > > > -[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > > > rendering
> > > > > +[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > > > rendering
> > > > >
> > > > > (b)
> > > > > -[ stable ] 9a7b3199037 anv/query: flush render target before
> > > > > copying results
> > > > >
> > > > > CC: Andres Gomez
> > > > > CC: Juan A. Suarez
> > > > > CC: Dylan Baker
> > > > > CC: mesa-sta...@lists.freedesktop.org
> > > > > Signed-off-by: Emil Velikov
> > > > > ---
> > > > > Juan I've noticed that you've been experiencing the above annoyance
> > > > > for
> > > > > a while. Having less false-positives should ease things up a bit :-)
> > > > > ---
> > > > > bin/get-pick-list.sh | 46
> > > > > +---
> > > > > 1 file changed, 30 insertions(+), 16 deletions(-)
> > > > >
> > > > > diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
> > > > > index 9f9cbc44026..08a783f35a8 100755
> > > > > --- a/bin/get-pick-list.sh
> > > > > +++ b/bin/get-pick-list.sh
> > > > > @@ -21,32 +21,36 @@ is_typod_nomination()
> > > > > git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
> > > > > }
> > > > >
> > > > > +fixes=
> > > > > +
> > > >
> > > > Splitting in 2 functions for which we need now a global variable is not
> > > > very nice. Anyway, let's not make this more complicated than needed.
> > > >
> > >
> > > Wasn't too happy about it either. As you said - I've decided to go
> > > with the simpler solution.
> > >
> > > > > # Helper to handle various mistypos of the fixes tag.
> > > > > # The tag string itself is passed as argument and normalised within.
> > > > > +#
> > > > > +# Resulting string in the global variable "fixes" and contains
> > > > > entries
> > > > > +# in the form "fixes:$sha"
> > > > > is_sha_nomination()
> > > > > {
> > > > > fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
> > > > > sed -e 's/'"$2"'/\nfixes:/Ig' | \
> > > > > grep -Eo 'fixes:[a-f0-9]{8,40}'`
> > > > >
> > > > > - fixes_count=`echo "$fixes" | wc -l`
> > > > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> > > >
> > > > Why is the extra "grep" needed here?
> > > >
> > >
> > > Commits that include "fixes $some_test" will result in fixes="".
> > > Thus wc -l will return 1
> >
> > Argh!
> >
> > Well, this is more a consequence of wc counting newline characters and
> > echo adding a new one at the end, even when not printing anything.
> > Let's grep again, then ☹
> >
> > > > > if test $fixes_count -eq 0; then
> > > > > - return 0
> > > > > + return 1
> > > >
> > > > Ugh! Right.
> > > >
> > > > > fi
> > > > > + return 0
> > > > > +}
> > > > > +
> > > > > +# Checks if at least one of offending commits, listed in the global
> > > > > +# "fixes", is in branch.
> > > > > +sha_in_range()
> > > > > +{
> > > > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> > > >
> > > > Ditto.
> > > >
> > > > > while test $fixes_count -gt 0; do
> > > > > # Treat only the current line
> > > > > id=`echo "$fixes" | tail -n $fixes_count | head -n 1 |
> > > > > cut -d : -f 2`
> > > > > fixes_count=$(($fixes_count-1))
> > > > >
> > > > > - # Bail out if we cannot find suitable id.
> > > > > - # Any specific validation the $id is valid and not some
> > > > > junk, is
> > > > > - # implied with the follow up code
> > > > > - if test "x$id" = x; then
> > > > > - continue
> > > > > - fi
> > > > > -
> > > > > - #Check if the offending commit is in branch.
> > > > > -
> > > >
> > > > Was this never needed in the first place? Feels right to remove it
> > > > since $fixes should have some content by now, but I wonder how this got
> > > > here in the first place.
> > > >
> > >
> > > Left-over from the old standalone script. Copied a bit too much :-(
> > >
> > > > > # Be that cherry-picked ...
> > > > > # ... or landed before
Re: [Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
On Mon, 17 Dec 2018 at 22:35, Andres Gomez wrote:
>
> On Mon, 2018-12-17 at 18:36 +, Emil Velikov wrote:
> > On Mon, 17 Dec 2018 at 18:14, Andres Gomez wrote:
> > >
> > > On Mon, 2018-12-17 at 16:43 +, Emil Velikov wrote:
> > > > Currently our is_sha_nomination does:
> > > > - folds any whitespace, attempting to extract sha-like information
> > > > - checks that at least one of the shas has landed
> > > >
> > > > Split it in two and do sha-like validation first.
> > > >
> > > > This way, commits with mesa-stable and sha nominations will feature the
> > > > fixes/revert/etc instead of stable (a) or will be omitted if not
> > > > applicable for the respective branch (b).
> > > >
> > > > Misc examples from 18.3
> > > >
> > > > (a)
> > > > -[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > > rendering
> > > > +[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > > rendering
> > > >
> > > > (b)
> > > > -[ stable ] 9a7b3199037 anv/query: flush render target before copying
> > > > results
> > > >
> > > > CC: Andres Gomez
> > > > CC: Juan A. Suarez
> > > > CC: Dylan Baker
> > > > CC: mesa-sta...@lists.freedesktop.org
> > > > Signed-off-by: Emil Velikov
> > > > ---
> > > > Juan I've noticed that you've been experiencing the above annoyance for
> > > > a while. Having less false-positives should ease things up a bit :-)
> > > > ---
> > > > bin/get-pick-list.sh | 46 +---
> > > > 1 file changed, 30 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
> > > > index 9f9cbc44026..08a783f35a8 100755
> > > > --- a/bin/get-pick-list.sh
> > > > +++ b/bin/get-pick-list.sh
> > > > @@ -21,32 +21,36 @@ is_typod_nomination()
> > > > git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
> > > > }
> > > >
> > > > +fixes=
> > > > +
> > >
> > > Splitting in 2 functions for which we need now a global variable is not
> > > very nice. Anyway, let's not make this more complicated than needed.
> > >
> >
> > Wasn't too happy about it either. As you said - I've decided to go
> > with the simpler solution.
> >
> > > > # Helper to handle various mistypos of the fixes tag.
> > > > # The tag string itself is passed as argument and normalised within.
> > > > +#
> > > > +# Resulting string in the global variable "fixes" and contains entries
> > > > +# in the form "fixes:$sha"
> > > > is_sha_nomination()
> > > > {
> > > > fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
> > > > sed -e 's/'"$2"'/\nfixes:/Ig' | \
> > > > grep -Eo 'fixes:[a-f0-9]{8,40}'`
> > > >
> > > > - fixes_count=`echo "$fixes" | wc -l`
> > > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> > >
> > > Why is the extra "grep" needed here?
> > >
> >
> > Commits that include "fixes $some_test" will result in fixes="".
> > Thus wc -l will return 1
>
> Argh!
>
> Well, this is more a consequence of wc counting newline characters and
> echo adding a new one at the end, even when not printing anything.
> Let's grep again, then ☹
>
> > > > if test $fixes_count -eq 0; then
> > > > - return 0
> > > > + return 1
> > >
> > > Ugh! Right.
> > >
> > > > fi
> > > > + return 0
> > > > +}
> > > > +
> > > > +# Checks if at least one of offending commits, listed in the global
> > > > +# "fixes", is in branch.
> > > > +sha_in_range()
> > > > +{
> > > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> > >
> > > Ditto.
> > >
> > > > while test $fixes_count -gt 0; do
> > > > # Treat only the current line
> > > > id=`echo "$fixes" | tail -n $fixes_count | head -n 1 |
> > > > cut -d : -f 2`
> > > > fixes_count=$(($fixes_count-1))
> > > >
> > > > - # Bail out if we cannot find suitable id.
> > > > - # Any specific validation the $id is valid and not some
> > > > junk, is
> > > > - # implied with the follow up code
> > > > - if test "x$id" = x; then
> > > > - continue
> > > > - fi
> > > > -
> > > > - #Check if the offending commit is in branch.
> > > > -
> > >
> > > Was this never needed in the first place? Feels right to remove it
> > > since $fixes should have some content by now, but I wonder how this got
> > > here in the first place.
> > >
> >
> > Left-over from the old standalone script. Copied a bit too much :-(
> >
> > > > # Be that cherry-picked ...
> > > > # ... or landed before the branchpoint.
> > > > if grep -q ^$id already_picked ||
> > > > @@ -103,20 +107,30 @@ do
> > > > continue
> > > > fi
> > > >
> > > > - if is_stable_nomination "$sha"; then
> > > > - tag=stable
> > > > - elif is_typod_nomination "$sha"; then
> > > > - tag=typod
> > > > - elif is_fixes_nomination "$sha"; then
> >
Re: [Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
On Mon, 2018-12-17 at 18:36 +, Emil Velikov wrote:
> On Mon, 17 Dec 2018 at 18:14, Andres Gomez wrote:
> >
> > On Mon, 2018-12-17 at 16:43 +, Emil Velikov wrote:
> > > Currently our is_sha_nomination does:
> > > - folds any whitespace, attempting to extract sha-like information
> > > - checks that at least one of the shas has landed
> > >
> > > Split it in two and do sha-like validation first.
> > >
> > > This way, commits with mesa-stable and sha nominations will feature the
> > > fixes/revert/etc instead of stable (a) or will be omitted if not
> > > applicable for the respective branch (b).
> > >
> > > Misc examples from 18.3
> > >
> > > (a)
> > > -[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > rendering
> > > +[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > > rendering
> > >
> > > (b)
> > > -[ stable ] 9a7b3199037 anv/query: flush render target before copying
> > > results
> > >
> > > CC: Andres Gomez
> > > CC: Juan A. Suarez
> > > CC: Dylan Baker
> > > CC: mesa-sta...@lists.freedesktop.org
> > > Signed-off-by: Emil Velikov
> > > ---
> > > Juan I've noticed that you've been experiencing the above annoyance for
> > > a while. Having less false-positives should ease things up a bit :-)
> > > ---
> > > bin/get-pick-list.sh | 46 +---
> > > 1 file changed, 30 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
> > > index 9f9cbc44026..08a783f35a8 100755
> > > --- a/bin/get-pick-list.sh
> > > +++ b/bin/get-pick-list.sh
> > > @@ -21,32 +21,36 @@ is_typod_nomination()
> > > git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
> > > }
> > >
> > > +fixes=
> > > +
> >
> > Splitting in 2 functions for which we need now a global variable is not
> > very nice. Anyway, let's not make this more complicated than needed.
> >
>
> Wasn't too happy about it either. As you said - I've decided to go
> with the simpler solution.
>
> > > # Helper to handle various mistypos of the fixes tag.
> > > # The tag string itself is passed as argument and normalised within.
> > > +#
> > > +# Resulting string in the global variable "fixes" and contains entries
> > > +# in the form "fixes:$sha"
> > > is_sha_nomination()
> > > {
> > > fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
> > > sed -e 's/'"$2"'/\nfixes:/Ig' | \
> > > grep -Eo 'fixes:[a-f0-9]{8,40}'`
> > >
> > > - fixes_count=`echo "$fixes" | wc -l`
> > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> >
> > Why is the extra "grep" needed here?
> >
>
> Commits that include "fixes $some_test" will result in fixes="".
> Thus wc -l will return 1
Argh!
Well, this is more a consequence of wc counting newline characters and
echo adding a new one at the end, even when not printing anything.
Let's grep again, then ☹
> > > if test $fixes_count -eq 0; then
> > > - return 0
> > > + return 1
> >
> > Ugh! Right.
> >
> > > fi
> > > + return 0
> > > +}
> > > +
> > > +# Checks if at least one of offending commits, listed in the global
> > > +# "fixes", is in branch.
> > > +sha_in_range()
> > > +{
> > > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
> >
> > Ditto.
> >
> > > while test $fixes_count -gt 0; do
> > > # Treat only the current line
> > > id=`echo "$fixes" | tail -n $fixes_count | head -n 1 | cut
> > > -d : -f 2`
> > > fixes_count=$(($fixes_count-1))
> > >
> > > - # Bail out if we cannot find suitable id.
> > > - # Any specific validation the $id is valid and not some
> > > junk, is
> > > - # implied with the follow up code
> > > - if test "x$id" = x; then
> > > - continue
> > > - fi
> > > -
> > > - #Check if the offending commit is in branch.
> > > -
> >
> > Was this never needed in the first place? Feels right to remove it
> > since $fixes should have some content by now, but I wonder how this got
> > here in the first place.
> >
>
> Left-over from the old standalone script. Copied a bit too much :-(
>
> > > # Be that cherry-picked ...
> > > # ... or landed before the branchpoint.
> > > if grep -q ^$id already_picked ||
> > > @@ -103,20 +107,30 @@ do
> > > continue
> > > fi
> > >
> > > - if is_stable_nomination "$sha"; then
> > > - tag=stable
> > > - elif is_typod_nomination "$sha"; then
> > > - tag=typod
> > > - elif is_fixes_nomination "$sha"; then
> > > + if is_fixes_nomination "$sha"; then
> > > tag=fixes
> > > elif is_brokenby_nomination "$sha"; then
> > > tag=brokenby
> > > elif is_revert_nomination "$sha"; then
> > > tag=revert
> > > + elif is_stable_nomination "$sha"; then
>
Re: [Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
On Mon, 17 Dec 2018 at 18:14, Andres Gomez wrote:
>
> On Mon, 2018-12-17 at 16:43 +, Emil Velikov wrote:
> > Currently our is_sha_nomination does:
> > - folds any whitespace, attempting to extract sha-like information
> > - checks that at least one of the shas has landed
> >
> > Split it in two and do sha-like validation first.
> >
> > This way, commits with mesa-stable and sha nominations will feature the
> > fixes/revert/etc instead of stable (a) or will be omitted if not
> > applicable for the respective branch (b).
> >
> > Misc examples from 18.3
> >
> > (a)
> > -[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > rendering
> > +[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct
> > rendering
> >
> > (b)
> > -[ stable ] 9a7b3199037 anv/query: flush render target before copying
> > results
> >
> > CC: Andres Gomez
> > CC: Juan A. Suarez
> > CC: Dylan Baker
> > CC: mesa-sta...@lists.freedesktop.org
> > Signed-off-by: Emil Velikov
> > ---
> > Juan I've noticed that you've been experiencing the above annoyance for
> > a while. Having less false-positives should ease things up a bit :-)
> > ---
> > bin/get-pick-list.sh | 46 +---
> > 1 file changed, 30 insertions(+), 16 deletions(-)
> >
> > diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
> > index 9f9cbc44026..08a783f35a8 100755
> > --- a/bin/get-pick-list.sh
> > +++ b/bin/get-pick-list.sh
> > @@ -21,32 +21,36 @@ is_typod_nomination()
> > git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
> > }
> >
> > +fixes=
> > +
>
> Splitting in 2 functions for which we need now a global variable is not
> very nice. Anyway, let's not make this more complicated than needed.
>
Wasn't too happy about it either. As you said - I've decided to go
with the simpler solution.
> > # Helper to handle various mistypos of the fixes tag.
> > # The tag string itself is passed as argument and normalised within.
> > +#
> > +# Resulting string in the global variable "fixes" and contains entries
> > +# in the form "fixes:$sha"
> > is_sha_nomination()
> > {
> > fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
> > sed -e 's/'"$2"'/\nfixes:/Ig' | \
> > grep -Eo 'fixes:[a-f0-9]{8,40}'`
> >
> > - fixes_count=`echo "$fixes" | wc -l`
> > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
>
> Why is the extra "grep" needed here?
>
Commits that include "fixes $some_test" will result in fixes="".
Thus wc -l will return 1
> > if test $fixes_count -eq 0; then
> > - return 0
> > + return 1
>
> Ugh! Right.
>
> > fi
> > + return 0
> > +}
> > +
> > +# Checks if at least one of offending commits, listed in the global
> > +# "fixes", is in branch.
> > +sha_in_range()
> > +{
> > + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
>
> Ditto.
>
> > while test $fixes_count -gt 0; do
> > # Treat only the current line
> > id=`echo "$fixes" | tail -n $fixes_count | head -n 1 | cut -d
> > : -f 2`
> > fixes_count=$(($fixes_count-1))
> >
> > - # Bail out if we cannot find suitable id.
> > - # Any specific validation the $id is valid and not some junk,
> > is
> > - # implied with the follow up code
> > - if test "x$id" = x; then
> > - continue
> > - fi
> > -
> > - #Check if the offending commit is in branch.
> > -
>
> Was this never needed in the first place? Feels right to remove it
> since $fixes should have some content by now, but I wonder how this got
> here in the first place.
>
Left-over from the old standalone script. Copied a bit too much :-(
> > # Be that cherry-picked ...
> > # ... or landed before the branchpoint.
> > if grep -q ^$id already_picked ||
> > @@ -103,20 +107,30 @@ do
> > continue
> > fi
> >
> > - if is_stable_nomination "$sha"; then
> > - tag=stable
> > - elif is_typod_nomination "$sha"; then
> > - tag=typod
> > - elif is_fixes_nomination "$sha"; then
> > + if is_fixes_nomination "$sha"; then
> > tag=fixes
> > elif is_brokenby_nomination "$sha"; then
> > tag=brokenby
> > elif is_revert_nomination "$sha"; then
> > tag=revert
> > + elif is_stable_nomination "$sha"; then
> > + tag=stable
> > + elif is_typod_nomination "$sha"; then
> > + tag=typod
> > else
> > continue
> > fi
> >
> > + case "$tag" in
> > + fixes | brokenby | revert )
> > + if ! sha_in_range; then
> > + continue
> > + fi
> > + ;;
> > + * )
> > + ;;
> > + esac
> > +
> > printf "[ %8s ] " "$tag"
> > git --no-pager show --summary --oneline $sha
> > done
>
> I'm not sure we
Re: [Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
On Mon, 2018-12-17 at 16:43 +, Emil Velikov wrote:
> Currently our is_sha_nomination does:
> - folds any whitespace, attempting to extract sha-like information
> - checks that at least one of the shas has landed
>
> Split it in two and do sha-like validation first.
>
> This way, commits with mesa-stable and sha nominations will feature the
> fixes/revert/etc instead of stable (a) or will be omitted if not
> applicable for the respective branch (b).
>
> Misc examples from 18.3
>
> (a)
> -[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct rendering
> +[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct rendering
>
> (b)
> -[ stable ] 9a7b3199037 anv/query: flush render target before copying
> results
>
> CC: Andres Gomez
> CC: Juan A. Suarez
> CC: Dylan Baker
> CC: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Emil Velikov
> ---
> Juan I've noticed that you've been experiencing the above annoyance for
> a while. Having less false-positives should ease things up a bit :-)
> ---
> bin/get-pick-list.sh | 46 +---
> 1 file changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
> index 9f9cbc44026..08a783f35a8 100755
> --- a/bin/get-pick-list.sh
> +++ b/bin/get-pick-list.sh
> @@ -21,32 +21,36 @@ is_typod_nomination()
> git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
> }
>
> +fixes=
> +
Splitting in 2 functions for which we need now a global variable is not
very nice. Anyway, let's not make this more complicated than needed.
> # Helper to handle various mistypos of the fixes tag.
> # The tag string itself is passed as argument and normalised within.
> +#
> +# Resulting string in the global variable "fixes" and contains entries
> +# in the form "fixes:$sha"
> is_sha_nomination()
> {
> fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
> sed -e 's/'"$2"'/\nfixes:/Ig' | \
> grep -Eo 'fixes:[a-f0-9]{8,40}'`
>
> - fixes_count=`echo "$fixes" | wc -l`
> + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
Why is the extra "grep" needed here?
> if test $fixes_count -eq 0; then
> - return 0
> + return 1
Ugh! Right.
> fi
> + return 0
> +}
> +
> +# Checks if at least one of offending commits, listed in the global
> +# "fixes", is in branch.
> +sha_in_range()
> +{
> + fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
Ditto.
> while test $fixes_count -gt 0; do
> # Treat only the current line
> id=`echo "$fixes" | tail -n $fixes_count | head -n 1 | cut -d :
> -f 2`
> fixes_count=$(($fixes_count-1))
>
> - # Bail out if we cannot find suitable id.
> - # Any specific validation the $id is valid and not some junk, is
> - # implied with the follow up code
> - if test "x$id" = x; then
> - continue
> - fi
> -
> - #Check if the offending commit is in branch.
> -
Was this never needed in the first place? Feels right to remove it
since $fixes should have some content by now, but I wonder how this got
here in the first place.
> # Be that cherry-picked ...
> # ... or landed before the branchpoint.
> if grep -q ^$id already_picked ||
> @@ -103,20 +107,30 @@ do
> continue
> fi
>
> - if is_stable_nomination "$sha"; then
> - tag=stable
> - elif is_typod_nomination "$sha"; then
> - tag=typod
> - elif is_fixes_nomination "$sha"; then
> + if is_fixes_nomination "$sha"; then
> tag=fixes
> elif is_brokenby_nomination "$sha"; then
> tag=brokenby
> elif is_revert_nomination "$sha"; then
> tag=revert
> + elif is_stable_nomination "$sha"; then
> + tag=stable
> + elif is_typod_nomination "$sha"; then
> + tag=typod
> else
> continue
> fi
>
> + case "$tag" in
> + fixes | brokenby | revert )
> + if ! sha_in_range; then
> + continue
> + fi
> + ;;
> + * )
> + ;;
> + esac
> +
> printf "[ %8s ] " "$tag"
> git --no-pager show --summary --oneline $sha
> done
I'm not sure we are gaining something with the splitting.
Maybe I'm not understanding correctly the patch but it seems to me that
every successful "is_sha_nomination" is followed by a "sha_in_range"
call. Hence, we are only trading splitting into 2 functions by having a
global variable (!)
Additionally, in the second patch of the series we are adding a warning
that, in case of having a single function, could be placed in the same
while loop, instead of having now 2 loops (?)
--
Br,
Andres
___
mesa-dev mailing list
[Mesa-dev] [PATCH 1/2] bin/get-pick-list.sh: rework handing of sha nominations
Currently our is_sha_nomination does:
- folds any whitespace, attempting to extract sha-like information
- checks that at least one of the shas has landed
Split it in two and do sha-like validation first.
This way, commits with mesa-stable and sha nominations will feature the
fixes/revert/etc instead of stable (a) or will be omitted if not
applicable for the respective branch (b).
Misc examples from 18.3
(a)
-[ stable ] 5bc509363b6 glx: make xf86vidmode mandatory for direct rendering
+[fixes ] 5bc509363b6 glx: make xf86vidmode mandatory for direct rendering
(b)
-[ stable ] 9a7b3199037 anv/query: flush render target before copying results
CC: Andres Gomez
CC: Juan A. Suarez
CC: Dylan Baker
CC: mesa-sta...@lists.freedesktop.org
Signed-off-by: Emil Velikov
---
Juan I've noticed that you've been experiencing the above annoyance for
a while. Having less false-positives should ease things up a bit :-)
---
bin/get-pick-list.sh | 46 +---
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/bin/get-pick-list.sh b/bin/get-pick-list.sh
index 9f9cbc44026..08a783f35a8 100755
--- a/bin/get-pick-list.sh
+++ b/bin/get-pick-list.sh
@@ -21,32 +21,36 @@ is_typod_nomination()
git show --summary "$1" | grep -q -i -o "CC:.*mesa-dev"
}
+fixes=
+
# Helper to handle various mistypos of the fixes tag.
# The tag string itself is passed as argument and normalised within.
+#
+# Resulting string in the global variable "fixes" and contains entries
+# in the form "fixes:$sha"
is_sha_nomination()
{
fixes=`git show --pretty=medium -s $1 | tr -d "\n" | \
sed -e 's/'"$2"'/\nfixes:/Ig' | \
grep -Eo 'fixes:[a-f0-9]{8,40}'`
- fixes_count=`echo "$fixes" | wc -l`
+ fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
if test $fixes_count -eq 0; then
- return 0
+ return 1
fi
+ return 0
+}
+
+# Checks if at least one of offending commits, listed in the global
+# "fixes", is in branch.
+sha_in_range()
+{
+ fixes_count=`echo "$fixes" | grep "fixes:" | wc -l`
while test $fixes_count -gt 0; do
# Treat only the current line
id=`echo "$fixes" | tail -n $fixes_count | head -n 1 | cut -d :
-f 2`
fixes_count=$(($fixes_count-1))
- # Bail out if we cannot find suitable id.
- # Any specific validation the $id is valid and not some junk, is
- # implied with the follow up code
- if test "x$id" = x; then
- continue
- fi
-
- #Check if the offending commit is in branch.
-
# Be that cherry-picked ...
# ... or landed before the branchpoint.
if grep -q ^$id already_picked ||
@@ -103,20 +107,30 @@ do
continue
fi
- if is_stable_nomination "$sha"; then
- tag=stable
- elif is_typod_nomination "$sha"; then
- tag=typod
- elif is_fixes_nomination "$sha"; then
+ if is_fixes_nomination "$sha"; then
tag=fixes
elif is_brokenby_nomination "$sha"; then
tag=brokenby
elif is_revert_nomination "$sha"; then
tag=revert
+ elif is_stable_nomination "$sha"; then
+ tag=stable
+ elif is_typod_nomination "$sha"; then
+ tag=typod
else
continue
fi
+ case "$tag" in
+ fixes | brokenby | revert )
+ if ! sha_in_range; then
+ continue
+ fi
+ ;;
+ * )
+ ;;
+ esac
+
printf "[ %8s ] " "$tag"
git --no-pager show --summary --oneline $sha
done
--
2.19.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
