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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>

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

> ---
> Â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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel