[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fedora-devel-list
Subject:    Re: libxml2 2.12.0 (and 2.12.1) in rawhide, with some API breaks
From:       Dodji Seketeli <dodji () seketeli ! org>
Date:       2023-11-28 17:26:18
Message-ID: 87r0k9rdqd.fsf () seketeli ! org
[Download RAW message or body]

Hello,

Petr Pisar <ppisar@redhat.com> a écrit:

> V Tue, Nov 28, 2023 at 12:24:45PM +0100, Dodji Seketeli napsal(a):

[...]

> > For what it's worth, the ABI compatibility verifier caught this change
> > between libxml2.so.2.11.5 and libxml2.so.2.12.0 and categorized it as
> > being "ABI compatible" (i.e, not incompatible/breaking) at:
> > https://artifacts.dev.testing-farm.io/b7d369b0-f288-4564-a331-48492c20bf8c/:
> > 
> > [C] 'function int xmlCopyError(xmlErrorPtr, xmlErrorPtr)' at error.c:985:1 has \
> > some indirect sub-type changes: parameter 1 of type 'typedef xmlErrorPtr' \
> > changed: entity changed from 'typedef xmlErrorPtr' to compatible type 'const \
> > xmlError*' in pointed to type 'struct _xmlError':
> > entity changed from 'struct _xmlError' to 'const _xmlError'
> > type size hasn't changed
> > 
> > I guess the important bit here is "entity changed from 'struct _xmlError' to \
> > 'const _xmlError'". 
> > So, yes, this is not an ABI incompatible change.
> > 
> Well abidiff for createrepo_c
> <https://artifacts.dev.testing-farm.io/3a97fed7-84b8-4220-9a7b-9f988dd8cd90/work-rpm \
> inspectud7bmqxi/rpminspect/execute/data/guest/default-0/rpminspect-1/data/viewer.html>
>  complains:
> 
> underlying type 'xmlParserCtxt*' changed:
> in pointed to type 'typedef xmlParserCtxt' at tree.h:40:1:
> underlying type 'struct _xmlParserCtxt' at parser.h:181:1 changed:
> type size changed from 3840 to 3968 (in bits)
> 4 data member insertions:
> 'unsigned int maxAmpl', at offset 3840 (in bits) at parser.h:323:1
> 'xmlParserNsData* nsdb', at offset 3872 (in bits) at parser.h:325:1
> 'unsigned int attrHashMax', at offset 3904 (in bits) at parser.h:326:1
> 'xmlAttrHashBucket* attrHash', at offset 3936 (in bits) at parser.h:327:1
> 1 data member changes (5 filtered):
> type of 'int* attallocs' changed:
> in pointed to type 'int':
> type name changed from 'int' to 'unsigned int'
> type size hasn't changed

That is correct.  That change to the layout of struct _xmlParserCtxt is
an ABI change.

However that change is only "reachable" (from an exported libxml2
function) through a pointer, abidiff categorized the change as "needing
review".  Hence the "VERIFY" categorization of the rpminspect.

In other words determining if this is an /incompatible/ ABI change
(a.k.a ABI break) or not ought to be discussed by human beings.

> While most (all?) libxml2 functions pass a pointer to xmlParserCtxt, a
> definition of the xmlParserCtxt structure is open to anybody in
> <libxml/parser.h>.

Correct, again.

However, ABI matters are often a about conventions that are specific to
each project.

In the case of libxml2, there is a (probably unwritten) convention which
says that data structures that are essentially internal to the library
have to be let alone.  Client code should not access those and should
only pass them to libxml2 functions through pointers, even though these
data structures are defined in /usr/include/libxml2/libxml/*.h.

> That means an application built against old libxml2 can construct a
> different structure than libxml2 expects.

If the application does that, then it violates the ABI convention
(softly) set by libxml2.

Maybe we should send patches to the libxml2 hackers to put that
convention into writing and help better set expectations in the future.

We can also teach abidiff about these essentially private data
structures of libxml2 so that it doesn't complain whenever they are
modified in the feature.  This would be done by providing a
libxml2-specific ABI change suppression specification[1].

> That is called an ABI breakage.

I guess I would argue (as I started above) that these things are not
that simple, unfortunately.

[...]

[1]: https://sourceware.org/libabigail/manual/libabigail-concepts.html#suppression-specifications


Cheers,

-- 
		Dodji
--
_______________________________________________
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-leave@lists.fedoraproject.org
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
 Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue


[prev in list] [next in list] [prev in thread] [next in thread]