Re: [xml] Entering freeze for release of libxml2-2.9.9

[Nikolai Weibull <now disu se>]

Hi!

OK, now I understand why it was working in my copy of the 
repository and not yours.  Something went wrong when you applied 
the patch, Daniel, as a line was elided.  Here’s a fix.  We want 
to include XML_RELAXNG_TEXT here as well, otherwise it won’t work. 
The second part of the patch below was just to reorder the types 
to be listed in alphabetical order, so you may certainly skip 
that.

I’m really sorry about the confusion this has caused.  It got a 
bit messy with provisional patches on the mailing list and merge 
requests on GitLab.  I’ll make sure to post proper patches on the 
mailing list in the future.

Thank you, Stefan, for reporting this and being so patient.

 Nikolai

--- ../libxml2/relaxng.c	2018-11-30 11:28:37.000000000 
   +0100
+++ relaxng.c   2018-11-30 11:25:54.000000000 +0100
@@ -4023,8 +4023,8 @@
            ((eora == 2) && ((cur->type == XML_RELAXNG_DATATYPE) 
            ||
                             (cur->type == XML_RELAXNG_ELEMENT) ||
                             (cur->type == XML_RELAXNG_LIST) ||
+                             (cur->type == XML_RELAXNG_TEXT) ||
                             (cur->type == XML_RELAXNG_VALUE)))) {
-
            if (ret == NULL) {
                max = 10;
                ret = (xmlRelaxNGDefinePtr *)
@@ -9285,9 +9285,9 @@
                return (1);
        } else if (((node->type == XML_TEXT_NODE) ||
                    (node->type == XML_CDATA_SECTION_NODE)) &&
-                   ((cur->type == XML_RELAXNG_TEXT) ||
-                   (cur->type == XML_RELAXNG_DATATYPE) ||
+                   ((cur->type == XML_RELAXNG_DATATYPE) ||
                    (cur->type == XML_RELAXNG_LIST) ||
+                    (cur->type == XML_RELAXNG_TEXT) ||
                    (cur->type == XML_RELAXNG_VALUE))) {
            return (1);
        }

Stefan Behnel, 2018-11-30 08:48:

> Daniel Veillard schrieb am 29.11.18 um 21:20:
> > On Mon, Nov 26, 2018 at 11:48:37AM +0100, Nikolai Weibull via 
> > xml wrote:
> > > Stefan Behnel, 2018-11-25 15:37:
> > > > Nikolai Weibull schrieb am 24.11.18 um 00:12:
> > > > > Yes, it seems that my patch for data in interleaves was 
> > > > > added and
> > > > > this may be the cause of these issues. The regression tests 
> > > > > didn’t
> > > > > display them, so this is something different. Could we 
> > > > > perhaps get a
> > > > > minimal test that breaks?
> > >
> > > > Here is what I could come up with so far. Since it's heavily 
> > > > stripped
> > > > down,
> > > > it probably isn't very reasonable anymore. The original 
> > > > schema is here:
>
> https://raw.githubusercontent.com/lxml/lxml/82601a09d015bc3e7a4090223fcbb9a5d5d4590d/src/lxml/isoschematron/resources/rng/iso-schematron.rng
>
> This is the direct file link now. I had attached the shortened 
> test files here:
>
> https://mail.gnome.org/archives/xml/2018-November/msg00023.html
>
>
> > > Thank you!  As far as my tests go, with the patches that I’ve 
> > > provided, this
> > > validates without any issues.  I really hope we can get my 
> > > patches from the
> > > merge request into master so that this issue can be fixed.
> >
> >   TBH it's weird it fails to validate for me with 2.9.8, with 
> >   2.9.9-rc1 and
> > with 2.9.9-rc1 with the data interleave patch reverted ...
>
> I tried both lxml's test suite and my stripped down test files 
> with 2.9.8
> and the two RCs now, and all of them pass with 2.9.8, but fail 
> with both
> 2.9.9-rc1 and 2.9.9-rc2.
>
> I figured out how to build libxml2 from a git checkout now so 
> that I could
> bisect it. The bug was definitely introduced in c8e5f9588, which 
> is
> Nikolai's change from November 22nd.
>
> I used
>
>     git bisect run bash -c "make clean && make &&
>         ./xmllint --relaxng ../iso-schematron.rng 
>         ../fail_schema.sch"
>
> The change looks simple, but also a bit opaque to me. It could 
> be that it's
> related to the interleaving of optional tags/attributes and text 
> somehow.
> At least, that's what this part of the change might suggest:
>
> -        groups[nbgroups]->defs = xmlRelaxNGGetElements(ctxt, 
> cur, 0);
> +        groups[nbgroups]->defs = xmlRelaxNGGetElements(ctxt, 
> cur, 2);
>
> And, in fact, changing that line in the latest master branch 
> back to the
> original "0" argument makes the validation pass for me. It 
> probably also
> reverts most of the intented behaviour that Nikolai wanted to 
> achieve. :(
>
> Stefan