dcantrell / bsdutils

Alternative to GNU coreutils using software from FreeBSD
Other
169 stars 9 forks source link

add musl support #3

Closed q66 closed 3 years ago

q66 commented 3 years ago

i know this cannot be merged as is, since it doesn't integrate into the patch workflow, i'm mostly putting this here for feedback for now, so that i know what kind of direction you plan to take

anyway, i intend to use this as the coreutils for an experimental distribution i'm currently working on - but in order to do that, i need musl support (and to finish the ports of the remaining utilities, but that is for later)

there are a few obstacles to that:

1) the widespread usage of the __unused macro is a problem, since musl uses it as a field name in some structures in its public API, that results in build failures (the C spec reserves that namespace for itself, so it's allowed to) 2) the tinfo bits are a separate thing, but might as well for correctness 3) sys/cdefs.h is a non-standard header; glibc technically has it, it's different from the freebsd one (it doesn't contain some of the definitions freebsd needs), musl does not have one at all; my proposed solution here is to provide our own compat version, which also reduces the need to actually define a lot of the macros within the build system 4) fts and rpmatch is provided as separate third party libraries for musl, so add bits to link those 5) there are missing includes that break musl builds around the place... so i fixed those 6) there's a bunch of BSD macros that glibc ships and musl does not, define those as necessary

what needs to be done: integrate the changes into the patches

if you have any better ideas on what to do with __unused (and possibly with cdefs.h) i'll be happy to hear them, nothing else comes to mind...

dcantrell commented 3 years ago

First, thank you for this PR. Responses below and I look forward to seeing more contributions from you.

i know this cannot be merged as is, since it doesn't integrate into the patch workflow, i'm mostly putting this here for feedback for now, so that i know what kind of direction you plan to take

anyway, i intend to use this as the coreutils for an experimental distribution i'm currently working on - but in order to do that, i need musl support (and to finish the ports of the remaining utilities, but that is for later)

Understood. The remaining utilities were the ones that require additional thought to Linuxify them whereas the completed ones needed zero to simple patching to get going. I only recently switched this project over to FreeBSD as the upstream source, having previously used OpenBSD. So most of these have OpenBSD ports in the git history, I just haven't dug through that to see what I did. Leave df for me, I already did that via OpenBSD and I'm fairly certain my work from that will apply here. :)

there are a few obstacles to that:

1. the widespread usage of the `__unused` macro is a problem, since musl uses it as a field name in some structures in its public API, that results in build failures (the C spec reserves that namespace for itself, so it's allowed to)

In this case, I would prefer to drop my redefine of unused to nothing since musl is using it. I would prefer explicit patching all uses of unused to attribute((unused)).

2. the tinfo bits are a separate thing, but might as well for correctness

Yep, that's fine.

3. `sys/cdefs.h` is a non-standard header; glibc technically has it, it's different from the freebsd one (it doesn't contain some of the definitions freebsd needs), musl does not have one at all; my proposed solution here is to provide our own compat version, which also reduces the need to actually define a lot of the macros within the build system

My own take on this is I don't like the FreeBSD one. I don't like these macros. My approach with the FreeBSD base of code was different than what I did with OpenBSD and I thought most of this stuff could be dealt with using macros. Since that only works on a glibc-based system and not musl, I would prefer we blank out the macros via either CFLAGS -D options or just add them to compat.h. I'm mostly indifferent here, but I think I would prefer the latter since that can carry a comment in a more obvious place about why we are doing this. That then requires removing those include lines from the source files, but I would prefer that to adding in a sys/cdefs.h file in a compat capacity.

4. fts and rpmatch is provided as separate third party libraries for musl, so add bits to link those

That's fine.

5. there are missing includes that break musl builds around the place... so i fixed those

Also fine.

6. there's a bunch of BSD macros that glibc ships and musl does not, define those as necessary

My preference here is to avoid defining the BSD one if we can get the same thing by changing it to a macro or expression built from macros we already had.

what needs to be done: integrate the changes into the patches

My utils/genpatch.sh script is what I was using to generate patches of src/ stuff, but otherwise I was doing things manually. Any improvement here is welcome.

if you have any better ideas on what to do with __unused (and possibly with cdefs.h) i'll be happy to hear them, nothing else comes to mind...

Thanks.

q66 commented 3 years ago

Understood. The remaining utilities were the ones that require additional thought to Linuxify them whereas the completed ones needed zero to simple patching to get going. I only recently switched this project over to FreeBSD as the upstream source, having previously used OpenBSD. So most of these have OpenBSD ports in the git history, I just haven't dug through that to see what I did. Leave df for me, I already did that via OpenBSD and I'm fairly certain my work from that will apply here. :)

yeah, i figured - i assume at least some of the leftover ones should also be relatievly easy, but e.g. i don't see uname getting ported from the current source (since it relies on freebsd specifics heavily, and we're better off just reimplementing it)

In this case, I would prefer to drop my redefine of unused to nothing since musl is using it. I would prefer explicit patching all uses of unused to attribute((unused)).

that's alright, i defined it to a macro since otherwise pasting it around is quite verbose and makes for really long lines, which makes for uglier patches...

My own take on this is I don't like the FreeBSD one. I don't like these macros. My approach with the FreeBSD base of code was different than what I did with OpenBSD and I thought most of this stuff could be dealt with using macros. Since that only works on a glibc-based system and not musl, I would prefer we blank out the macros via either CFLAGS -D options or just add them to compat.h. I'm mostly indifferent here, but I think I would prefer the latter since that can carry a comment in a more obvious place about why we are doing this. That then requires removing those include lines from the source files, but I would prefer that to adding in a sys/cdefs.h file in a compat capacity.

the thing with that is, i don't like it either, but since cdefs.h does not exist on musl, doing it via buildsystem would require patching out all these includes; the reason i did things this way is in order to keep the modifications of freebsd codebase to a minimum (since i believe it'd be best to keep the patches as small as possible, in order to ease the rebasing burden)

My preference here is to avoid defining the BSD one if we can get the same thing by changing it to a macro or expression built from macros we already had.

we can definitely do that, but the same thing applies as above (keeping patching to a minimum)

dcantrell commented 3 years ago

Understood. The remaining utilities were the ones that require additional thought to Linuxify them whereas the completed ones needed zero to simple patching to get going. I only recently switched this project over to FreeBSD as the upstream source, having previously used OpenBSD. So most of these have OpenBSD ports in the git history, I just haven't dug through that to see what I did. Leave df for me, I already did that via OpenBSD and I'm fairly certain my work from that will apply here. :)

yeah, i figured - i assume at least some of the leftover ones should also be relatievly easy, but e.g. i don't see uname getting ported from the current source (since it relies on freebsd specifics heavily, and we're better off just reimplementing it)

uname will be almost a complete rewrite. An interesting thing to compare is the uname implementation in OpenBSD vs. FreeBSD. FreeBSD took some significant turns with that command.

In this case, I would prefer to drop my redefine of unused to nothing since musl is using it. I would prefer explicit patching all uses of unused to attribute((unused)).

that's alright, i defined it to a macro since otherwise pasting it around is quite verbose and makes for really long lines, which makes for uglier patches...

Eh, I'm ok with the long lines.

My own take on this is I don't like the FreeBSD one. I don't like these macros. My approach with the FreeBSD base of code was different than what I did with OpenBSD and I thought most of this stuff could be dealt with using macros. Since that only works on a glibc-based system and not musl, I would prefer we blank out the macros via either CFLAGS -D options or just add them to compat.h. I'm mostly indifferent here, but I think I would prefer the latter since that can carry a comment in a more obvious place about why we are doing this. That then requires removing those include lines from the source files, but I would prefer that to adding in a sys/cdefs.h file in a compat capacity.

the thing with that is, i don't like it either, but since cdefs.h does not exist on musl, doing it via buildsystem would require patching out all these includes; the reason i did things this way is in order to keep the modifications of freebsd codebase to a minimum (since i believe it'd be best to keep the patches as small as possible, in order to ease the rebasing burden)

That's a fair point and actually one of the reasons I was initially adding as many -D options to CFLAGS as possible to avoid verbose patching like I did for the OpenBSD ports.

Given that, I'm ok incorporating a compat cdefs.h header so we can keep patches shorter.

My preference here is to avoid defining the BSD one if we can get the same thing by changing it to a macro or expression built from macros we already had.

we can definitely do that, but the same thing applies as above (keeping patching to a minimum)

That's fine. I would like to revisit previous patches to make things more consistent.

q66 commented 3 years ago

sounds good :) i will update the PR later, now that things are cleared up, and start doing other changes afterwards. I already have some ideas lined up.

q66 commented 3 years ago

alright...

sorry it took me long; i had other work to do regarding my system's infrastructure and i couldn't get to it until now - here's a new version of the PR with review changes incorporated

additionally I also fixed the import-src.sh and refreshed all the patches (now when you import the source and have it apply the patches, it comes out exactly the same)

as far as I can tell it should be ready, but feel free to bring it up if there's anything

dcantrell commented 3 years ago

This looks good to me. Thanks for fixing things up and taking time to get it working with musl. I'm continuing working on the ports of remaining programs in the collection. Feel free to send more PRs.