New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build failure on musl-1.2.5 due to removed basename prototype from string.h #778
Comments
What is the suggested fix? |
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, I'm assuming the fix is to add libgen.h, right? |
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 kmod-project/kmod#32 in several projects, and that means - as documented in that PR - the solution is to never include libgen.h. :) |
That's helpful, thanks. I'll add a helper working as the GNU basename and drop libgen.h. |
What basename(3) does with the argument depends on _GNU_SOURCE and inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported. We want the GNU semantics that does not modify the argument. Common way to make it portable is to add own helper. This is now implemented in path_basename() that does not use the libc provided basename but preserves the semantics. The path_dirname() is just for parity, otherwise same as dirname(). Sources: - https://bugs.gentoo.org/926288 - https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7 Issue: #778 Signed-off-by: David Sterba <dsterba@suse.com>
Fixed in devel. The manual page says that path |
What basename(3) does with the argument depends on _GNU_SOURCE and inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported. We want the GNU semantics that does not modify the argument. Common way to make it portable is to add own helper. This is now implemented in path_basename() that does not use the libc provided basename but preserves the semantics. The path_dirname() is just for parity, otherwise same as dirname(). Sources: - https://bugs.gentoo.org/926288 - https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7 Issue: #778 Signed-off-by: David Sterba <dsterba@suse.com>
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! 👍 |
What basename(3) does with the argument depends on _GNU_SOURCE and inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported. We want the GNU semantics that does not modify the argument. Common way to make it portable is to add own helper. This is now implemented in path_basename() that does not use the libc provided basename but preserves the semantics. The path_dirname() is just for parity, otherwise same as dirname(). Sources: - https://bugs.gentoo.org/926288 - https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7 Issue: #778 Signed-off-by: David Sterba <dsterba@suse.com>
The simple implementation using strrchr() does not work in cases like |
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. |
What basename(3) does with the argument depends on _GNU_SOURCE and inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported. We want the GNU semantics that does not modify the argument. Common way to make it portable is to add own helper. This is now implemented in path_basename() that does not use the libc provided basename but preserves the semantics. The path_dirname() is just for parity, otherwise same as dirname(). Sources: - https://bugs.gentoo.org/926288 - https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7 Issue: #778 Signed-off-by: David Sterba <dsterba@suse.com>
This is a followup to 884a609 ("btrfs-progs: add basename wrappers for unified semantics"). Test cli/019-subvolume-create-parents fails as there are paths with trailing slashes. The GNU semantics does not change the argument of basename(3) but this is problematic with trailing slashes. This is not uncommon and could potentially break things. To minimize impact of the basename behaviour depending on the include of libgen.h use the single wrapper in path utils that has to include libgen anyway for dirname. Our code passes writable buffers to basename. Issue: #778 Signed-off-by: David Sterba <dsterba@suse.com>
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
. 😞Current calls to basename are in:
btrfs-progs/common/device-utils.c
Line 353 in 3793e98
btrfs-progs/cmds/subvolume.c
Line 173 in 3793e98
btrfs-progs/libbtrfsutil/subvolume.c
Line 673 in 3793e98
The text was updated successfully, but these errors were encountered: