xfsdump musl patch questions

xfsdump musl patch questions

[Adam Thiede]

Hello - I'm interested in packaging xfsdump for alpine linux. However, 
alpine uses musl libc and I had to change a lot of things to get xfsdump 
to build. Mostly it was changing types that are specific to glibc (i.e. 
stat64 -> stat). I'm not much of a c programmer myself so I am likely 
misunderstanding some things, but changing these types allows xfsdump to 
compile and function on musl libc. xfsdump still compiles on Debian with 
this patch too.

Would the maintainers of xfsdump be interested in this patch? It's >4000 
lines so I'm not sure of the right way to send it. It's available in the 
following merge request, and linked directly.

https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/88452
https://gitlab.alpinelinux.org/alpine/aports/-/raw/f042233eff197591777663751848ff504210002e/testing/xfsdump/musl.patch

Thanks for your time,

- Adam Thiede

Re: xfsdump musl patch questions

[Holger Hoffstätte]

On 2025-08-30 13:23, Adam Thiede wrote:
> Hello - I'm interested in packaging xfsdump for alpine linux.
> However, alpine uses musl libc and I had to change a lot of things to
> get xfsdump to build. Mostly it was changing types that are specific
> to glibc (i.e. stat64 -> stat). I'm not much of a c programmer myself
> so I am likely misunderstanding some things, but changing these types
> allows xfsdump to compile and function on musl libc. xfsdump still
> compiles on Debian with this patch too.

You might want to double-check with Gentoo's Musl porting notes:
https://wiki.gentoo.org/wiki/Musl_porting_notes
esp. 2.6: "error: LFS64 interfaces".

We currently still take the "workaround" route:
https://gitweb.gentoo.org/repo/gentoo.git/commit/sys-fs/xfsdump/xfsdump-3.1.12.ebuild?id=33791d44f8bbe7a8d1566a218a76050d9f51c33d

..but fixing this for real is certainly a good idea!

cheers
Holger

Re: xfsdump musl patch questions

[Adam Thiede]

On 8/30/25 12:46, Holger Hoffstätte wrote:
> On 2025-08-30 13:23, Adam Thiede wrote:
>> Hello - I'm interested in packaging xfsdump for alpine linux.
>> However, alpine uses musl libc and I had to change a lot of things to
>> get xfsdump to build. Mostly it was changing types that are specific
>> to glibc (i.e. stat64 -> stat). I'm not much of a c programmer myself
>> so I am likely misunderstanding some things, but changing these types
>> allows xfsdump to compile and function on musl libc. xfsdump still
>> compiles on Debian with this patch too.
> 
> You might want to double-check with Gentoo's Musl porting notes:
> https://wiki.gentoo.org/wiki/Musl_porting_notes
> esp. 2.6: "error: LFS64 interfaces".
> 
> We currently still take the "workaround" route:
> https://gitweb.gentoo.org/repo/gentoo.git/commit/sys-fs/xfsdump/ 
> xfsdump-3.1.12.ebuild?id=33791d44f8bbe7a8d1566a218a76050d9f51c33d
> 
> ..but fixing this for real is certainly a good idea!
> 
> cheers
> Holger

Thanks - using -D_LARGEFILE64_SOURCE also fixes the issue without the 
enormous patch. However the following small patch is necessary since 
alpine builds with -Wimplicit-function-declaration

diff --git a/invutil/invidx.c b/invutil/invidx.c
index 5874e8d..9506172 100644
--- a/invutil/invidx.c
+++ b/invutil/invidx.c
@@ -28,6 +28,7 @@
  #include <sys/stat.h>
  #include <string.h>
  #include <uuid/uuid.h>
+#include <libgen.h>

  #include "types.h"
  #include "mlog.h"
diff --git a/common/main.c b/common/main.c
index 6141ffb..f5e959f 100644
--- a/common/main.c
+++ b/common/main.c
@@ -38,6 +38,7 @@
  #include <string.h>
  #include <uuid/uuid.h>
  #include <locale.h>
+#include <libgen.h>

  #include "config.h"


I think this one would be good to include at the very least.

Re: xfsdump musl patch questions

[Holger Hoffstätte]

On 2025-08-30 20:10, Adam Thiede wrote:

> Thanks - using -D_LARGEFILE64_SOURCE also fixes the issue without the enormous patch. However the following small patch is necessary since alpine builds with -Wimplicit-function-declaration
> 
> diff --git a/invutil/invidx.c b/invutil/invidx.c
> index 5874e8d..9506172 100644
> --- a/invutil/invidx.c
> +++ b/invutil/invidx.c
> @@ -28,6 +28,7 @@
>   #include <sys/stat.h>
>   #include <string.h>
>   #include <uuid/uuid.h>
> +#include <libgen.h>
> 
>   #include "types.h"
>   #include "mlog.h"
> diff --git a/common/main.c b/common/main.c
> index 6141ffb..f5e959f 100644
> --- a/common/main.c
> +++ b/common/main.c
> @@ -38,6 +38,7 @@
>   #include <string.h>
>   #include <uuid/uuid.h>
>   #include <locale.h>
> +#include <libgen.h>
> 
>   #include "config.h"

aargh..basename again!

We fix it like this:
https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-fs/xfsdump/files/xfsdump-3.1.12-mimic-basename-for-nonglibc.patch

It's trickier than it looks since you need to actually audit the
call sites, else you might get crashes at runtime.

> I think this one would be good to include at the very least.

The thing is that your longer patch is conceptually the better way
forward, just a bit incomplete as it is. We just discussed this on
IRC and I can quote what our resident toolchain guru Sam had to say,
which provides some background as well:

[19:52:50] <sam_> the patch is wrong as it is, at least without some rationale & checking that AC_SYS_LARGEFILE is used (to guarantee off_t is 64-bit on 32-bit glibc systems)
[19:53:05] <sam_> in general, the stat64, off64_t, etc types should go away (they're transitional)
[19:53:12] <sam_> the issue is when getting rid of them, people often get it wrong
[19:53:25] <sam_> the types were added into glibc to allow people to have dual ABIs in their headers
[19:53:26] <sam_> it never took off
[19:53:45] <sam_> you can look at the thread where I ported xfsprogs itself

The thread he's referring to starts here:
https://lore.kernel.org/linux-xfs/20240205232343.2162947-1-sam@gentoo.org/

Of course the shorter workaround route should be good as first fix too.

I guess it depends on what the xfsdump maintainer prefers.

cheers
Holger

Re: xfsdump musl patch questions

[Donald Douwsma]

On 31/8/25 04:39, Holger Hoffstätte wrote:
> On 2025-08-30 20:10, Adam Thiede wrote:
> 
>> Thanks - using -D_LARGEFILE64_SOURCE also fixes the issue without the 
>> enormous patch. However the following small patch is necessary since 
>> alpine builds with -Wimplicit-function-declaration
>>
>> diff --git a/invutil/invidx.c b/invutil/invidx.c
>> index 5874e8d..9506172 100644
>> --- a/invutil/invidx.c
>> +++ b/invutil/invidx.c
>> @@ -28,6 +28,7 @@
>>   #include <sys/stat.h>
>>   #include <string.h>
>>   #include <uuid/uuid.h>
>> +#include <libgen.h>
>>
>>   #include "types.h"
>>   #include "mlog.h"
>> diff --git a/common/main.c b/common/main.c
>> index 6141ffb..f5e959f 100644
>> --- a/common/main.c
>> +++ b/common/main.c
>> @@ -38,6 +38,7 @@
>>   #include <string.h>
>>   #include <uuid/uuid.h>
>>   #include <locale.h>
>> +#include <libgen.h>
>>
>>   #include "config.h"
> 
> aargh..basename again!
> 
> We fix it like this:
> https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-fs/xfsdump/files/xfsdump-3.1.12-mimic-basename-for-nonglibc.patch
> 
> It's trickier than it looks since you need to actually audit the
> call sites, else you might get crashes at runtime.

My initial feeling is to just include libgen.h in include/config.h.in as

#if !defined(__GLIBC__)
#include <libgen.h>
#endif

Its less targeted than your patch which only touches files using
basename, but xfsprogs resolved this by including libgen.h in
platform_defs.in unconditionally somewhere after string.h

>> I think this one would be good to include at the very least.
> 
> The thing is that your longer patch is conceptually the better way
> forward, just a bit incomplete as it is. We just discussed this on
> IRC and I can quote what our resident toolchain guru Sam had to say,
> which provides some background as well:
> 
> [19:52:50] <sam_> the patch is wrong as it is, at least without some 
> rationale & checking that AC_SYS_LARGEFILE is used (to guarantee off_t 
> is 64-bit on 32-bit glibc systems)
> [19:53:05] <sam_> in general, the stat64, off64_t, etc types should go 
> away (they're transitional)
> [19:53:12] <sam_> the issue is when getting rid of them, people often 
> get it wrong
> [19:53:25] <sam_> the types were added into glibc to allow people to 
> have dual ABIs in their headers
> [19:53:26] <sam_> it never took off
> [19:53:45] <sam_> you can look at the thread where I ported xfsprogs itself
> 
> The thread he's referring to starts here:
> https://lore.kernel.org/linux-xfs/20240205232343.2162947-1-sam@gentoo.org/

Thanks for the context, I noticed the comment about _LARGEFILE64_SOURCE
in the release notes you mentioned earlier
https://musl.libc.org/releases.html

In general xfsdump should follow how xfsprogs builds.

Ill also look over the other changes made to the xfsprogs build and
see if there's anything else that's been missed.

Don

Re: xfsdump musl patch questions

[Christoph Hellwig]

On Sat, Aug 30, 2025 at 06:23:36AM -0500, Adam Thiede wrote:
> Hello - I'm interested in packaging xfsdump for alpine linux. However,
> alpine uses musl libc and I had to change a lot of things to get xfsdump to
> build. Mostly it was changing types that are specific to glibc (i.e. stat64
> -> stat). I'm not much of a c programmer myself so I am likely
> misunderstanding some things, but changing these types allows xfsdump to
> compile and function on musl libc. xfsdump still compiles on Debian with
> this patch too.
> 
> Would the maintainers of xfsdump be interested in this patch? It's >4000
> lines so I'm not sure of the right way to send it. It's available in the
> following merge request, and linked directly.
> 
> https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/88452
> https://gitlab.alpinelinux.org/alpine/aports/-/raw/f042233eff197591777663751848ff504210002e/testing/xfsdump/musl.patch
> 

Using the proper LFS APIs instead of the *64 is a good thing, and
xfsprogs has done this long ago.  So in general patches are welcome,
but please split them into one issue per patch, maybe starting with
removing the *64 APIs.