Re: [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry

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

On Wed, 2014-02-05 at 11:49 +0000, George Dunlap wrote:
> On 02/05/2014 09:22 AM, Ian Campbell wrote:
> > On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
> >> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> >> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> >> boot after the installation.
> >>
> >> In addition to this, RHEL 7 menu entries have two different single-quote
> >> delimited strings on the same line, and the greedy grouping for menuentry
> >> parsing gets both strings, and the options inbetween.
> 
> So you're saying that adding the '?' just happens to change the match 
> because of a quirk in the algorithms in the python library? That seems 
> more like a hack than a proper fix; there may be other versions of 
> python (future versions, for instance) where the new regexp will have 
> the same effect as the old one, and we'll have another regression.
> 
> Even if the behavior described is part of the defined interface,

I believe it is. Joby posted a link earlier. It also seems to be part of
the Perl re syntax -- and lots of things use Perl's regex syntax so I
think it is pretty "standard" (although I was not previously aware of it
either). Wikipedia's regex page talks about it too.

>  I'd be 
> wary of using this because future developers may not realize what it's 
> for, or how to modify it properly to retain the properties it has now.

Hypothetical developer ignorance might call for a comment, but I think
avoiding language features which provide the semantics we need just
because they are a bit obscure would be a mistake.

> >> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Cc: george.dunlap@citrix.com
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > IMHO this can go into 4.4, unless George objects today I shall commit.
> 
> I'm a bit on the fence about this one.  If this had been sent a month 
> ago, it would be a no-brainer.  It certainly looks like it should work 
> just fine.  On the other hand, pygrub is an important bit of 
> functionality, and I'm not sure how much testing it gets. But of course 
> the XenServer XenRT tests probably exercise it fairly well (or else they 
> wouldn't be submitting this patch).

FWIW I intended to run it over the (admittedly small) set of test cases
in the tree as part of the commit process. I believe Joby has already
done so anyway.

> The Register seems to think that RHEL will be released "in the first 
> half of 2014", which would certainly be before 4.5.  But we should have 
> another point release before then, with enough time to do better testing 
> and (possibly) come up with a better solution to the regexp problem 
> above (assuming my interpretation is correct).
> 
> I'm wondering though whether it would make more sense to save this for 
> 4.4.1.