Re: [OSSTEST PATCH] mg-list-all-branches: avoid mistakenly generating `.' in the output

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2016-02-19 at 17:52 +0000, Ian Jackson wrote:
> The regex in mg-list-all-branches assumes that the BRANCHES= will
> either be a singleton entry separated from the following command by a
> hard tab or a single quoted list of space separated entries, however
> the xen-unstable-coverity line is singleton separated from the command
> by a single space.
> 
> We could fix this by using a hard tab, but that ends up aligning
> things in an aesthetically displeasing way, and relying on hard tabs
> is fragile.
> 
> Instead, improve the parsing in mg-list-all-branches: break out a
> couple of semantically (as well as syntactically) common regexp
> elements out into variables, and then provide two regexps: one which
> matches shell "assign default values" substitutions, and the other
> which matches the ordinary shell assignments.
> 
> We use an empty pair of () in the first regexp to make sure that they
> both produce the branch name list in $2.  (It would be possible to use
> named capture groups but I'm not sure whether all our perls are recent
> enough.)
> 
> I have verified that the actual difference in output right now is just
> to remove the erroneous `.' entry.
> 
> Reported-by: Ian Campbell <ian.campbell@citrix.com>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

LGTM, Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  mg-list-all-branches |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/mg-list-all-branches b/mg-list-all-branches
> index 87703c7..62d3ff1 100755
> --- a/mg-list-all-branches
> +++ b/mg-list-all-branches
> @@ -7,11 +7,16 @@ use Sort::Versions;
>  
>  our %branches;
>  
> +my $branchvar_re = '(?:EXTRA_)?BRANCHES';
> +my $branchchr_re = '[-.0-9a-z ]';
> +
>  foreach my $f (qw(cr-for-branches crontab)) {
>      open C, $f or die $!;
>      while (<C>) {
> -        next unless m/(?:EXTRA_)?BRANCHES[:+]?='?([-.0-9a-z ]+)/;
> -        $branches{$_}=1 foreach split /\s+/, $1;
> +        next unless
> +	    m/\$\{$branchvar_re[:+]?=()($branchchr_re+)\b/ ||
> +	    m/$branchvar_re[:+]?=('?)($branchchr_re+?)\1\s/;
> +        $branches{$_}=1 foreach split /\s+/, $2;
>      }
>      close C or die $!;
>  }

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel