Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 904190

Summary: dev-libs/expat please add Large File Support (LFS)
Product: Gentoo Linux Reporter: Zi Lin <lziest>
Component: Current packagesAssignee: Freedesktop bugs <freedesktop-bugs>
Status: UNCONFIRMED ---    
Severity: enhancement CC: sam, sping, vapier
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on:    
Bug Blocks: 471102    

Description Zi Lin 2023-04-11 18:22:26 UTC
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
Comment 1 Sebastian Pipping gentoo-dev 2023-04-11 20:04:48 UTC
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
Comment 2 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-04-25 02:28:11 UTC
(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.
Comment 3 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-04-25 02:28:32 UTC
(Updated assignee per metadata.xml, but given you're upstream, maybe put yourself first in there? up to you ofc)
Comment 4 SpanKY gentoo-dev 2023-05-04 18:23:41 UTC
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
Comment 5 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-05-05 05:32:07 UTC
(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