| Message ID | 1455904336-31539-1-git-send-email-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 $!; > }
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 $!; }