https://github.com/libexpat/libexpat/issues/707 We would like to add lfs support flags in the ebuild as a patch to the above issue. Without it, ChromiumOS build will constantly fail on 32-bit boards. Reproducible: Always Actual Results: 15:54:37.405 * QA Notice: The following files were not built with LFS support: 15:54:37.416 * Please see https://issuetracker.google.com/201531268 for details. 15:54:37.427 * fopen,__open_2,fstat,mmap /usr/bin/xmlwf 15:54:37.429 * __open_2 /usr/lib/libexpat.so.1.8.10 15:54:37.440 * Full build files: fopen,__open_2,fstat,mmap /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/xmlwf/.libs/xmlwf fopen /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/xmlwf/xmlwf-xmlwf.o __open_2,fstat,mmap /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/xmlwf/xmlwf-unixfilemap.o __open_2 /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/xmlwf/xmlwf-xmlfile.o stat,fopen /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/tests/benchmark/.libs/benchmark stat,fopen /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/tests/benchmark/benchmark.o __open_2 /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/lib/.libs/xmlparse.o __open_2 /build/arm-generic/tmp/portage/dev-libs/expat-2.5.0/work/expat-2.5.0-.arm/lib/.libs/libexpat.so.1.8.10 Expected Results: build succeeds. Sample patch. diff --git a/dev-libs/expat/expat-2.5.0.ebuild b/dev-libs/expat/expat-2.5.0.ebuild index d918b182f..3702e8ea9 100644 --- a/dev-libs/expat/expat-2.5.0.ebuild +++ b/dev-libs/expat/expat-2.5.0.ebuild @@ -4,7 +4,7 @@ EAPI=7 AUTOTOOLS_AUTO_DEPEND=no AT_NOEAUTOHEADER=yes # because expat_config.h.in would need post-processing -inherit autotools multilib-minimal +inherit autotools multilib-minimal flag-o-matic DESCRIPTION="Stream-oriented XML parser library" HOMEPAGE="https://libexpat.github.io/" @@ -38,6 +38,7 @@ src_prepare() { multilib_src_configure() { local myconf="$(use_enable static-libs static) --without-docbook" + append-lfs-flags mkdir -p "${BUILD_DIR}"w || die
Hi Zi, I will do a first braindump here with all the pieces I see going into this topic and any related decisions: - Expat has a build switch "XML_LARGE_SIZE" that controls the types used behind the typedefs for XML_Index and XML_Size and hence breaks ABI to anyone not compiling with XML_LARGE_SIZE also: #ifdef XML_LARGE_SIZE typedef long long XML_Index; typedef unsigned long long XML_Size; #else typedef long XML_Index; typedef unsigned long XML_Size; #endif This flag would be needed e.g. for xmlwf error reporting regarding the precise location of the error in the file. Please elaborate whether you would you need this flag to be on or off for ChromiumOS and why so. - dev-libs/expat in Gentoo currently installs 1-2 copies of the library (expat and expatw) and 1 copy of xmlwf. If(!) we want to enable XML_LARGE_SIZE, we will need a third (or third and fourth) copy and make xmlwf use that copy rather than the regular expat for full effect. And that third library would need a name — expat + expatw + expat??? — that plays well with any other Linux distro offering a large-size copy. PS: Debian seems to have had lib64expat1 in the past that they compiled with -m64 but not XML_LARGE_SIZE and it no longer exists. - I am wondering if you have any real users for this use case to match the cost or if it's mostly about silencing a QA tool. Do you have actual users processing >2G XML files on 32bit hardware in 2023? - I am wondering if adding append-lfs-flags by default breaks anything. I don't expect any breakage from applying it by default (and see it applied in a few other ebuilds) but we better sleep over that change and give it a few days for others to have the chance to jump in with yet-unknown concerns. - (There seem to be both flags "lfs" and "largefile" around in Gentoo, today). What do you think? Best, Sebastian
(In reply to Sebastian Pipping from comment #1) > - I am wondering if adding append-lfs-flags by default breaks anything. > I don't expect any breakage from applying it by default > (and see it applied in a few other ebuilds) but we better sleep > over that change and give it a few days for others to have the > You mean in general, to everything? It'll break ABI whenever an off_t or friends are used in interfaces. See https://wiki.gentoo.org/wiki/Project:Toolchain/time64_migration#LFS and the rest of that page.
(Updated assignee per metadata.xml, but given you're upstream, maybe put yourself first in there? up to you ofc)
there's more to LFS than just reading a file. 64bit inodes break the ability to stat a file regardless of its size, and the code is def doing that now. and we'll see the same thing with 64-bit time with 2038. i think we're conflating things here a bit. nothing is stopping expat from using LFS interfaces when it access the filesystem. it does not need to change its exported ABI as a result. it does have to take a bit of care when returning offsets that might overflow (if sizeof(off_t) > sizeof(XML_Index)). i don't know how many APIs utilize those types to say how difficult it is to add. i'll note that on many systems (i.e. 64-bit) where sizeof(long long) == sizeof(long), there is no ABI breakage by enabling XML_LARGE_SIZE. we're talking about systems (i.e. legacy 32-bit) where sizeof(long) != sizeof(long long). if we look at the history of libexpat, it has broken ABI before. there used to be libexpat.so.0 and we have libexpat.so.1 now. maybe it's time to bump to libexpat.so.2 and basically always enable XML_LARGE_SIZE. the comment "not all compilers support long long" is horribly antiquated nowadays as that basically means "the compiler doesn't support uint64_t" which (1) is required by C/POSIX standards and (2) expat is already using at least in "siphash.h". as noted, this doesn't affect systems that already do LFS by default (i.e. 64-bit), so breaking ABI SONAME there would suck. it would also suck to add logic to try and determine "is this a 32-bit breakage and thus use libexpat.so.2, or is everything 64-bit where sizeof(long) == sizeof(long long) and thus use libexpat.so.1". maybe use this opportunity to make a lot of future-proof changes: * enable XML_UNICODE so we don't have libexpat.so & libexpatw.so -- libexpat.so.2 only * enable XML_LARGE_SIZE & LFS -- assert that sizeof(off_t) == 64-bit, and use int64_t instead of long long in public interfaces
(In reply to SpanKY from comment #4) > if we look at the history of libexpat, it has broken ABI before. there used > to be libexpat.so.0 and we have libexpat.so.1 now. maybe it's time to bump > to libexpat.so.2 and basically always enable XML_LARGE_SIZE. > [...] I agree this is really the best thing to do if one can stomach it. We've had a bunch of libraries with legacy hacks like this (like XML_LARGE_SIZE, including xmlsec & iirc libxml2 had something similar too) and it's best to rip the bandaid off. > [...] > * enable XML_UNICODE so we don't have libexpat.so & libexpatw.so -- > libexpat.so.2 only > * enable XML_LARGE_SIZE & LFS -- assert that sizeof(off_t) == 64-bit, and > use int64_t instead of long long in public interfaces
