Build failure on musl-1.2.5 due to removed basename prototype from string.h

[hhoffstaette]

Originally found in Gentoo: https://bugs.gentoo.org/926288

musl 1.2.5 removed basename from string.h.

This is messy because of the different semantics between the POSIX and GNU implementations and might require not just a conditional import of libgen.h. :disappointed:

Current calls to basename are in: https://github.com/kdave/btrfs-progs/blob/3793e987d2b4e878410da16f33d963043d137d48/common/device-utils.c#L353 https://github.com/kdave/btrfs-progs/blob/3793e987d2b4e878410da16f33d963043d137d48/cmds/subvolume.c#L173 https://github.com/kdave/btrfs-progs/blob/3793e987d2b4e878410da16f33d963043d137d48/libbtrfsutil/subvolume.c#L673 (multiple)

[kdave]

What is the suggested fix?

[kdave]

The CI images are using alpine:edge that has musl version 1.2.5r0, so this should fail the build but it does not. We're using the GNU basename, _GNU_SOURCE is defined for the entire build.

From the example only one seems to miss the libgen.h include, common/device-utils.c, for subvolume.c it's already there and in libbtrfsutil it's only a local variable.

I'm assuming the fix is to add libgen.h, right?

[hhoffstaette]

Note that I'm just the messenger, I don't use musl myself.

How to fix this really depends on the code of every individual project. Search all of github for "musl basename" and you will find countless slightly different approaches.

If you want the GNU semantics where the input argument is never modified and therefore does not crash when it's a static constant string (IMHO the sanest real-world approach, despite the fact that this is not POSIX compliant) then maybe just steal a gnu_basename replacement function from some other project and only compile it as basename() when it is undefined.

In Gentoo we have added variations of https://github.com/kmod-project/kmod/pull/32 in several projects, and that means - as documented in that PR - the solution is to never include libgen.h. :)

[kdave]

That's helpful, thanks. I'll add a helper working as the GNU basename and drop libgen.h.

[kdave]

Fixed in devel. The manual page says that path / should be returned as / from basename, this should be added to the kmod project wrapper I think.

[hhoffstaette]

I have a musl chroot but it's still on 1.2.4; will update and give this a try tomorrow.

[hhoffstaette]

I have a musl chroot but it's still on 1.2.4; will update and give this a try tomorrow.

Seems to compile - thanks! :+1:

[kdave]

The simple implementation using strrchr() does not work in cases like /path/dir///// as it returns empty path due to number of trailing slashes. This also breaks one test that uses such paths. I think this can happen by accident or even one trailing slash will break it. This is where the argument modifying basename works and I'll probably change the internal implementation to behave like that. There should be no change regarding Musl support.

[kdave]

I'll keep current version for 6.8.1 as it works in most cases and fixes the musl support but this looks like reimplementation of posix basename behaviour. It is also possible that path handling can be simplified in some cases, changing just before a release won't be good.