Re: [lmi] [PATCH] Use submodules for, and newer versions of, libxml2 and libxslt

[Vadim Zeitlin]

On Fri, 2 Oct 2020 20:19:47 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> On 2020-10-02 16:37, Vadim Zeitlin wrote:
GC> > On Fri, 2 Oct 2020 16:22:11 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:
GC> > 
GC> > GC> That's okay--it's exactly what I asked for, because I've been
GC> > GC> running these tests so often lately that it's easy to select
GC> > GC> the right commands from shell history. And I have encountered
GC> > GC> an error:
GC> > GC> 
GC> > GC> In file included from /opt/lmi/src/lmi/xml_xslt_wrapp.cpp:39:
GC> > GC> /opt/lmi/third_party/src/libxml/event_parser.cxx:61:38: error: 
conflicting declaration of C functio
GC> > GC> n ‘void xmlSAX2InitDefaultSAXHandler(xmlSAXHandlerV1*, int)’
GC> > GC>    61 |     #define initxmlDefaultSAXHandler 
xmlSAX2InitDefaultSAXHandler
GC> > GC>       |                                      
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
GC> [...]
GC> > GC> /opt/lmi/local/include/libxml2/libxml/SAX2.h:159:3: note: previous 
declaration ‘void xmlSAX2InitDef
GC> > GC> aultSAXHandler(xmlSAXHandler*, int)’
GC> [...]
GC> > so I'm already looking into it. I don't understand what is the problem 
just
GC> > yet, but I'll find and fix it a.s.a.p.
GC> 
GC> No rush. My guess is that this calls for an xmlwrapp upgrade,

 Yes, indeed, I've finally understood what was going on here and I'd like
to explain what it was because it's a rather curious case. But please feel
free to skip over all this, just please don't skip the second part of this
message as there is a question later that I'd like to you to answer.

<explanation for the curious>

 So what happens here is that we #define initxmlDefaultSAXHandler as
something else, in order to allow to compile the code which calls this
function using both with the ancient (pre-2.6.0, which was released 17
years ago) versions of libxml2, which defined this function in SAX.h, and
newer ones, which define xmlSAX2InitDefaultSAXHandler() in SAX2.h (while
still providing the old function in SAX.h for compatibility, of course).
This is ugly, but doesn't create any real problems as long as each file is
compiled separately.

 When they're all compiled as a single translation unit in lmi, however, we
have a problem: event_parser.cxx defines initxmlDefaultSAXHandler(), but
then tree_parser.cxx, whose contents is compiled later, includes SAX.h
which defines the real function with this name. But because this function
was redefined as some other function, with a different declaration, in
SAX2.h which had been already included from event_parser.cxx before, this
results in the error.

 This much became clear almost immediately, but I couldn't understand why
has this changed when we upgraded libxml2 because git-diff showed no
changes neither in SAX.h nor SAX2.h between the old and the new versions.
And the answer turned out to be that in the old version SAX.h was already
included long before event_parser.cxx had the opportunity to do its
mischief because it was implicitly included from the other libxml2 headers,
via libxml2/globals.h. A couple of years ago (in libxml2 2.9.9), they have
finally removed this obsolete header from globals.h, which means that now
we actually include it from tree_parser.cxx for the first time, while
before we just hit the header guard and didn't really parse the contents of
this header when compiling it.

 And this is how I learnt that you can break compilation of downstream code
by removing some header inclusion even if that code does correctly include
the header, as it's supposed to do. Of course, this is not libxml2 fault,
but that of xmlwrapp which should have never used this ugly hack in the
first place, but I just find the whole sad story so exquisitely baroque
that I wanted to share it.

</end explanation

 Anyhow, I've fixed this in xmlwrapp master now (and will probably make
another release of it soon), so we will be able to re-upgrade libxml2 again
as soon as we integrate the latest xmlwrapp into lmi as a submodule.


GC> and instead of using some lmi makefile for that, I'd rather
GC> just wait for the xmlwrapp submodule.

 This is actually an important point: I was going to preserve the current
"unity" build of xmlwrapp which is used right now, i.e. continue using
xml_xslt_wrapp.cpp file, but just set up the include paths so that it finds
the files it includes in third_party/xmlwrapp submodule rather than
somewhere else on the system. But does the above mean that you'd like to
change this and use xmlwrapp normal build?

 I'm probably still going to do it in 2 steps, i.e. add xmlwrapp submodule
first and change the build system later, if wanted, so this is not very
urgent, but please let me know if you think the second step is worth doing
at all. FWIW personally I wouldn't have added xml_xslt_wrapp.cpp from the
beginning, but now that it's there, I don't see any real problem that
having it presents. Maybe just the fact that it results in some weird
errors from time to time, which don't appear in the normal xmlwrapp build,
such as this one (and I believe it's not the first time that this
happened).

 Thanks in advance,
VZ

pgpt0PNmtZL1k.pgp
Description: PGP signature