Fixes to make bootstrapping from Linux/musl work #1066

valpackett · 2024-01-17T02:51:34Z

Here's a bunch of fixes that allowed me to build FreeBSD under @chimera-linux which uses musl libc and a port of our own core programs. The more complex changes are definitely subject to improvements, suggestions/bikeshedding welcome!

+also required: openzfs/zfs#15780

bsdimp · 2024-01-17T04:32:14Z

This looks quite interesting... i like parts and have concerns about parts. Ill go through it with a fine toothed comb.

emaste

style(9) disagrees with this change, (<sys/param.h> includes <sys/types.h>; do not include both.) so we'll need to address that somehow -- possibly by amending style to include something like do not include both, unless required for portability.

(gh code review does not make it clear the code review comment is going to be left on the general conversation, not the specific file or commit -- this is with respect to 032c239 sys/capsicum.h)

emaste · 2024-01-24T22:06:43Z

kerberos5/include/config.h

@@ -694,7 +696,9 @@
 #define HAVE_RES_NDESTROY 1

 /* Define to 1 if you have the `res_nsearch' function. */
+#if __RES >= 19991006


cc @cschuber - is there any automation involved in the generation of config.h on krb5 updates?

This likely needs a comment, but this doesn't feel like the right place to solve this...

This file uses __BEGIN_DECLS/__END_DECLS. When bootstrapping from Linux/musl, they were missing. Sponsored by: https://www.patreon.com/valpackett Pull Request: #1066

emaste · 2024-01-24T22:12:32Z

jevents.c should coordinate with upstream

tools/build/cross-build/include/common/langinfo.h

bsdimp · 2024-01-25T00:36:39Z

style(9) disagrees with this change, (<sys/param.h> includes <sys/types.h>; do not include both.) so we'll need to address that somehow -- possibly by amending style to include somethin

We need to document nonstandard usage.

I have a lot of other comments as well. Will do them after bowling.

Specifically this one needs to be more general. Otherwise we'll be breaking this build and not knowing it as things shift around. Better to use the compat bootstrsp stuff to make the environments the same. Otherwise i fear this new bootstrap will be broken too much of the time.

bsdimp

Please don't land anything else from this PR. I have several comments pending i wanted to think through.

bsdimp · 2024-01-25T04:50:31Z

diff --git a/tools/build/cross-build/include/linux/sys/types.h b/tools/build/cross-build/include/linux/sys/types.h
index 770b3057f8a8..e72e1419bccd 100644
--- a/tools/build/cross-build/include/linux/sys/types.h
+++ b/tools/build/cross-build/include/linux/sys/types.h
@@ -35,6 +35,11 @@
  */
 #pragma once
 
+/*
+ * FreeBSD's types.h always includes sys/cdefs.h, but Linux headers often don't.
+ */
+#include <sys/cdefs.h>
+
 #include_next <sys/types.h>
 
 #ifndef __size_t

I think will fix the issue...

bsdimp · 2024-01-25T05:04:56Z

contrib/mandoc/config.h

@@ -13,7 +13,7 @@
 #define HAVE_ENDIAN 0
 #define HAVE_ERR 1
 #define HAVE_FTS 1
-#ifdef __GLIBC__
+#ifdef __linux__


This likely should be upstraemed, but it changes a local change, so it's OK.Both the changes need to go up.... :)

bsdimp

Mostly I'm wondering why the sed/grep changes aren't conditional.

bsdimp · 2024-01-25T05:05:30Z

include/nl_types.h

@@ -34,6 +34,7 @@
 #ifndef _NL_TYPES_H_
 #define _NL_TYPES_H_

+#include <sys/cdefs.h>


I commented on this.

bsdimp · 2024-01-25T05:08:39Z

kerberos5/include/config.h

@@ -694,7 +696,9 @@
 #define HAVE_RES_NDESTROY 1

 /* Define to 1 if you have the `res_nsearch' function. */
+#if __RES >= 19991006


This likely needs a comment, but this doesn't feel like the right place to solve this...

lib/libpmc/pmu-events/jevents.c

bsdimp · 2024-01-25T05:10:18Z

sys/sys/capsicum.h

@@ -40,6 +40,7 @@
 #define	_SYS_CAPSICUM_H_

 #include <sys/param.h>
+#include <sys/types.h>


This should likely just be added to ./tools/build/cross-build/include/linux/sys/param.h

bsdimp · 2024-01-25T05:14:24Z

usr.bin/grep/grep.c

 int		 cflags = REG_NOSUB | REG_NEWLINE;
-int		 eflags = REG_STARTEND;


This is a very odd change.... It should be #ifdef linux or something, no?

There's #ifdef REG_STARTEND but inside of util.c. There it's choosing between using the flag if it exists, and a fallback manual implementation in the #else. With that, the global default for the flags doesn't quite make sense.

bsdimp · 2024-01-25T05:26:30Z

Also, if the changes to use cross-tools includes work... then maybe we can eliminate some of the added ctypes.h includes.

tools/build/cross-build/include/common/pwd.h

bsdimp · 2024-04-21T04:53:32Z

Ping? What's up with this? Sounded like you were preparing an updated, but it doesn't look like one's been pushed.
I don't have this environment, so I can't easily mine this for changes and commit piecemeal.
Can you rebase and update the PR please? Despite all the fiddly comments, I think this would be good to have.
If the upstream bits are getting in the way, maybe just resubmit those w/o them, we'll get that in and then see what
to do about upstream?
Thanks!

Required for building under Linux/musl. Sponsored by: https://www.patreon.com/valpackett

On musl, libresolv is just a dummy, these don't exist. Let's just build the ones from our own libc instead. Sponsored by: https://www.patreon.com/valpackett

musl needs it too. The file is only build on Linux anyway, so no ifdef is required at all. Sponsored by: https://www.patreon.com/valpackett

musl libc does not include fts, but musl distros typically include an external implementation by the Void Linux team. Sponsored by: https://www.patreon.com/valpackett

grep and sed are part of bootstrap-tools, so they must build on Linux. musl libc does not have REG_STARTEND, so we must emulate it manually. Coincidentally, Chimera Linux uses musl and our grep/sed so we can adapt the patch from chimerautils. Obtained from: Chimera Linux, modified Sponsored by: https://www.patreon.com/valpackett

They are completely absent there. Sponsored by: https://www.patreon.com/valpackett

res_nsearch is not present there; use the __RES API level check to determine whether it is. Sponsored by: https://www.patreon.com/valpackett

Specifically isxdigit is absent in the musl header. Let's check the defines one by one to be safe. Sponsored by: https://www.patreon.com/valpackett

valpackett · 2024-04-22T09:10:14Z

Oop… yeah, sorry, was so busy with all the everything again. Thanks for the ping. Rebased, removed langinfo, did the cdefs-via-types thing instead of touching capsicum.h.

BTW, feel free to do more cherry-picking-and-pushing of commits you'd consider uncontroversial, the PR getting smaller always makes it better to work with (:

bsdimp · 2024-04-23T18:07:54Z

tools/build/cross-build/include/common/sys/_types.h

@@ -45,3 +45,6 @@
 * Neither GLibc nor macOS define __va_list but many FreeBSD headers require it.
 */
 typedef __builtin_va_list __va_list;
+
+/* Needed for opensolaris compat. */
+typedef	__int64_t	off64_t;


this is fine now, but may cause some grief in the future when I land our better compat with open solaris off64_t stuff

bsdimp

just a few notes... I'll see what I can land from this and we'll iterate.

bsdimp · 2024-04-23T18:09:03Z

tools/build/cross-build/include/mac/sys/_types.h

@@ -41,5 +41,3 @@
 * __darwin_ct_rune_t exists.
 */
 typedef __darwin_ct_rune_t __ct_rune_t;
-/* Needed for opensolaris compat. */
-typedef	__int64_t	off64_t;


I don't understand this one...

Moved to common given it’s needed for musl, it seems

bsdimp · 2024-04-23T18:09:18Z

tools/build/cross-build/progname.c

@@ -36,7 +36,6 @@
 #include <sys/cdefs.h>
 #include <stdlib.h>

-#ifdef __GLIBC__


I guess because tools/build/Makefile only builds it on Linux, so it was previously a stupid ifdef, and now is unhelpful for musl.

I feel like this file might have been compiled for macos in the past so the ifdef was there to only use it for Linux. Dropping it looks good.

bsdimp · 2024-04-23T18:12:29Z

usr.bin/localedef/bootstrap/ctype.h

 __BEGIN_DECLS
+#ifndef isalpha
 int	isalpha(int);


quick question here: there's two ways to fix this: one is how you've done (and is likely fine for bootstrap), the other is to make these int (isalpha)(int); to prevent the macro expansion. Is there a reason to prefer one over the other?

I guess you shouldn't have a prototype for something that's implemented as a macro, since it doesn't really exist

arichardson · 2024-04-23T18:31:08Z

usr.bin/sed/process.c

@@ -672,11 +671,23 @@ regexec_e(regex_t *preg, const char *string, int eflags, int nomatch,
 		defpreg = preg;

 	/* Set anchors */
+#ifndef REG_STARTEND /* in cross-build tools */


This looks different from the wrapper in grep.c (the former does not have a loop). Would it make sense to pull this out into a shared helper?

jrtc27 · 2024-04-23T22:25:04Z

@bsdimp Please liaise with me before landing any of this, I would like to ensure I review it first

jrtc27

My general comment is that there's very little by way of explanation and justification, with a lot of unrelated changes being lumped into a single commit that has no body in the message. Each change should document why it's needed, and be limited to one thing. As it stands for a lot of these I look at the diff and can only really speculate on why they're needed.

jrtc27 · 2024-04-23T23:53:21Z

tools/build/cross-build/include/common/pwd.h

 {
+	(void)stayopen;


The correct fix is to make sure __unused is defined

jrtc27 · 2024-04-23T23:54:31Z

tools/build/cross-build/include/common/resolv.h

@@ -0,0 +1,25 @@
+/*-
+ * SPDX-License-Identifier: ISC


We should be using fragments from our resolv.h, not openbsd-compat's

jrtc27 · 2024-04-23T23:55:13Z

tools/build/cross-build/include/linux/pthread.h

+#include_next <pthread.h>
+
+#define PTHREAD_DETACHED PTHREAD_CREATE_DETACHED
+#define pthread_yield sched_yield


Are these ok to do for glibc?..

jrtc27 · 2024-04-23T23:57:27Z

tools/build/cross-build/include/linux/pthread.h

+#define pthread_yield sched_yield
+
+#ifndef PTHREAD_MUTEX_ADAPTIVE_NP
+#define PTHREAD_MUTEX_ADAPTIVE_NP PTHREAD_MUTEX_DEFAULT


I'm assuming this is all for usr.bin/sort/radixsort.c? I'd be tempted to just whack MK_SORT_THREADS off when bootstrapping...

jrtc27 · 2024-04-23T23:59:29Z

tools/build/cross-build/include/linux/sys/types.h

@@ -35,6 +35,23 @@
 */
 #pragma once

+/*
+ * FreeBSD's types.h always includes sys/cdefs.h, but Linux headers often don't.


sys/types.h

jrtc27 · 2024-04-23T23:59:57Z

tools/build/cross-build/include/linux/sys/types.h

+typedef	uint8_t	__uint8_t;
+typedef	uint16_t	__uint16_t;
+typedef	uint32_t	__uint32_t;
+typedef	uint64_t	__uint64_t;


This makes things a bit circular and confusing... can we not follow _size_t instead?

jrtc27 · 2024-04-24T00:01:09Z

tools/build/cross-build/progname.c

@@ -36,7 +36,6 @@
 #include <sys/cdefs.h>
 #include <stdlib.h>

-#ifdef __GLIBC__


I guess because tools/build/Makefile only builds it on Linux, so it was previously a stupid ifdef, and now is unhelpful for musl.

jrtc27 · 2024-04-24T00:02:17Z

tools/build/mk/Makefile.boot

 .if ${.MAKE.OS} == "Linux"
 CFLAGS+=	-I${SRCTOP}/tools/build/cross-build/include/linux
 CFLAGS+=	-D_GNU_SOURCE=1
 # Needed for sem_init, etc. on Linux (used by usr.bin/sort)
 LDADD+=	-pthread
+.if exists(/usr/lib/libfts.so) || exists(/usr/lib/libfts.a) || exists(/lib/libfts.so) || exists(/lib/libfts.a)


This feels dodgy

jrtc27 · 2024-04-24T00:03:10Z

usr.bin/localedef/bootstrap/ctype.h

 __BEGIN_DECLS
+#ifndef isalpha
 int	isalpha(int);


I guess you shouldn't have a prototype for something that's implemented as a macro, since it doesn't really exist

bsdimp · 2024-04-24T03:39:18Z

@bsdimp Please liaise with me before landing any of this, I would like to ensure I review it first

Will do. The changes themselves likely are OK, but I share your concerns about justification and regressions, though the cross build github jobs work. The changes also aren't chunked up quite right, and need to be regroups a little. I'll post phab reviews and CC you. I need to find a musl build env...

bsdimp · 2024-05-23T21:43:30Z

contrib/mandoc/config.h

@@ -13,7 +13,7 @@
 #define HAVE_ENDIAN 0
 #define HAVE_ERR 1
 #define HAVE_FTS 1
-#if defined(__GLIBC__) || defined(__APPLE__)
+#if defined(__linux__) || defined(__APPLE__)


Is this really #ifndef FreeBSD? Or does NetBSD/OpenBSD also have this new const interface?

off64_t is needed for both Linux (musl) and MacOS, so move them to the common area. Somehow glibc provides the definition, but defining it doesn't hurt and hels in the musl case. Reviewed by: allanjude, jrtc27 Pull Request: #1066

fts has different types for its compare rotuine. Other systems, not 4.4BSD based, have a non-const version. Before we tested against __GLIBC__, but now we test against __linux__ because that's Linux's API and musl doesn't define __GLIBC__. In addition, link against libftl on this platform since musl doesn't include ftl routines in libc, but rather in libftl. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

On Linux with musl, sys/cdefs.h isn't included with fcntl.h, so when we use __BEGIN_DECL and __END_DECL in this file, it fails. There's no harm in unconditionally including sys/cdefs.h here, so do that to avoid encoding exactly where it is or isn't needed so we don't have to know too much about the internal state of other libc implementations. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

FreeBSD has a style(9) enforced assumption that sys/param.h includes sys/types.h. However, Linux under musl doesn't do this so go ahead and explicitly include it here. It won't hurt on the glibc systems, and helps musl. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

string.h defines strmode with a mode_t argument. POSIX states that one must include sys/types.h to get mode_t, so do that here. This makes musl happier. We know that sys/types.h will include sys/cdefs.h, so just replace the latter with the former. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

FreeBSD assumes that sys/types.h includes sys/cdefs.h, so add it here. FreeBSD also needs __*int*_t defined for software we bootstrap (a lot of it). GLIBC defines these, but musl does not, so we have to define them here, even though it looks backwards. There's no good #define to key off of, so use !defined GLIBC since on Linux defacto there's only two libc implementations. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

bsdimp · 2024-05-24T21:16:05Z

OK. To make progress on this, I've setup a Alpine Linux VM. I've started mining this PR for bits of code that can be committed. I've also added a few comments for some weird things that I've noticed. I'm testing this now. https://reviews.freebsd.org/D45349 is the first of 7 reviews that is the result. These are all uninteresting bits. I added Val as a co-author on almost all of them. I could swap author and co-author on them though. I wrote either extra comments, or a commit message for these, so I think it's only fair to share the credit and I'm agnostic on how. The co-authored-by stuff was the path of least resistance for me, but let's chat if that bothers anybody. It also slices up what's here a bit differently, but not that differently. I hope Co-authored-by: is the right git trailer to use for github to give co-credit. If not, I'll change them to whatever does.

I added @jrtc27 to the reviews as well. My hope is that we can commit those reviews, close this review out and then do the rest of this pull request with new, smaller pull requests. The size of this request made it linger far too long and I'd like to try to get things in faster, which means I think that future pull requests should be smaller and more easy to manage. There's just too many moving parts otherwise.

bsdimp · 2024-05-24T21:33:25Z

OK. All three builds succeeded in the musl branch I pushed. Though on alpine Linux 'success' was defined to be 'made it as far as the hack and slash version I did to validate what I cleaned up and put into the musl branch'. The stat stuff looks ready too, I overlooked it earlier and I'm out of time today. The base64 stuff, the resolver stuff, the pthreads stuff and the regexp stuff I don't pretend to grok all remain, each of which may need some tweaking and I'd suggest should be a separate future pull request in the future to keep things from getting bogged down.

bsdimp · 2024-05-26T17:41:38Z

so, it's time to close this one. I've mined all the best bits out. Maybe there's more to get (like the stat.h patch), but I'm going to close this and we'll start on the other stuff from that base.

fts has different types for its compare rotuine. Other systems, not 4.4BSD based, have a non-const version. Before we tested against __GLIBC__, but now we test against __linux__ because that's Linux's API and musl doesn't define __GLIBC__. In addition, link against libftl on this platform since musl doesn't include ftl routines in libc, but rather in libftl. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: #1066 Reviewed by: val_packett.cool Differential Revision: https://reviews.freebsd.org/D45349

On Linux with musl, sys/cdefs.h isn't included with fcntl.h, so when we use __BEGIN_DECL and __END_DECL in this file, it fails. There's no harm in unconditionally including sys/cdefs.h here, so do that to avoid encoding exactly where it is or isn't needed so we don't have to know too much about the internal state of other libc implementations. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: #1066 Reviewed by: val_packett.cool Differential Revision: https://reviews.freebsd.org/D45351

FreeBSD has a style(9) enforced assumption that sys/param.h includes sys/types.h. However, Linux under musl doesn't do this so go ahead and explicitly include it here. It won't hurt on the glibc systems, and helps musl. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: #1066 Reviewed by: val_packett.cool Differential Revision: https://reviews.freebsd.org/D45352

string.h defines strmode with a mode_t argument. POSIX states that one must include sys/types.h to get mode_t, so do that here. This makes musl happier. We know that sys/types.h will include sys/cdefs.h, so just replace the latter with the former. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: #1066 Reviewed by: val_packett.cool Differential Revision: https://reviews.freebsd.org/D45353

FreeBSD assumes that sys/types.h includes sys/cdefs.h, so add it here. FreeBSD also needs __*int*_t defined for software we bootstrap (a lot of it). GLIBC defines these, but musl does not, so we have to define them here, even though it looks backwards. There's no good #define to key off of, so use !defined GLIBC since on Linux defacto there's only two libc implementations. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: #1066 Reviewed by: val_packett.cool Differential Revision: https://reviews.freebsd.org/D45354

GLIBC defines these, but MUSL does not. FreeBSD's bootstrap code uses these defines, so define them if they aren't yet defined. Co-authored-by: Val Packett <val@packett.cool> Sponsored by: Netflix Pull Request: freebsd#1066

valpackett requested a review from bsdimp as a code owner January 17, 2024 02:51

emaste reviewed Jan 24, 2024

View reviewed changes

freebsd-git pushed a commit that referenced this pull request Jan 24, 2024

nl_types.h: explicitly include cdefs.h

820401b

This file uses __BEGIN_DECLS/__END_DECLS. When bootstrapping from Linux/musl, they were missing. Sponsored by: https://www.patreon.com/valpackett Pull Request: #1066

jrtc27 reviewed Jan 24, 2024

View reviewed changes

tools/build/cross-build/include/common/langinfo.h Outdated Show resolved Hide resolved

bsdimp requested changes Jan 25, 2024

View reviewed changes

bsdimp reviewed Jan 25, 2024

View reviewed changes

arichardson reviewed Jan 25, 2024

View reviewed changes

tools/build/cross-build/include/common/pwd.h Outdated Show resolved Hide resolved

bsdimp added the changes-required label Apr 19, 2024

valpackett force-pushed the muslstrap branch from 5e12418 to 5421a63 Compare April 22, 2024 06:59

valpackett added 8 commits April 22, 2024 05:41

cross-build: add various missing defines and includes

81863e4

Required for building under Linux/musl. Sponsored by: https://www.patreon.com/valpackett

cross-build: Use b64_pton/b64_ntop from our libc

206a85a

On musl, libresolv is just a dummy, these don't exist. Let's just build the ones from our own libc instead. Sponsored by: https://www.patreon.com/valpackett

cross-build: Don't restrict getprogname to glibc

3d20f28

musl needs it too. The file is only build on Linux anyway, so no ifdef is required at all. Sponsored by: https://www.patreon.com/valpackett

cross-build: Support musl-fts

05718e1

musl libc does not include fts, but musl distros typically include an external implementation by the Void Linux team. Sponsored by: https://www.patreon.com/valpackett

cross-build: define __intX_t aliases on Linux/musl

7474b39

They are completely absent there. Sponsored by: https://www.patreon.com/valpackett

kerberos5: fix bootstrap under Linux/musl

c81c820

res_nsearch is not present there; use the __RES API level check to determine whether it is. Sponsored by: https://www.patreon.com/valpackett

localedef: fix bootstrap on Linux/musl

321e701

Specifically isxdigit is absent in the musl header. Let's check the defines one by one to be safe. Sponsored by: https://www.patreon.com/valpackett

valpackett force-pushed the muslstrap branch from 5421a63 to 321e701 Compare April 22, 2024 09:01

bsdimp reviewed Apr 23, 2024

View reviewed changes

arichardson reviewed Apr 23, 2024

View reviewed changes

jrtc27 reviewed Apr 24, 2024

View reviewed changes

bsdimp added the special label May 10, 2024

bsdimp reviewed May 23, 2024

View reviewed changes

bsdimp closed this May 26, 2024

bsdimp added merged and removed changes-required labels May 26, 2024

		int cflags = REG_NOSUB \| REG_NEWLINE;
		int eflags = REG_STARTEND;

Fixes to make bootstrapping from Linux/musl work #1066

Fixes to make bootstrapping from Linux/musl work #1066

Conversation

valpackett commented Jan 17, 2024

bsdimp commented Jan 17, 2024

emaste left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emaste commented Jan 24, 2024

bsdimp commented Jan 25, 2024 • edited

bsdimp left a comment

Choose a reason for hiding this comment

bsdimp commented Jan 25, 2024

Choose a reason for hiding this comment

bsdimp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsdimp commented Jan 25, 2024

bsdimp commented Apr 21, 2024

valpackett commented Apr 22, 2024

Choose a reason for hiding this comment

bsdimp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Apr 23, 2024

jrtc27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsdimp commented Apr 24, 2024

Choose a reason for hiding this comment

bsdimp commented May 24, 2024

bsdimp commented May 24, 2024

bsdimp commented May 26, 2024

emaste left a comment •

edited

bsdimp commented Jan 25, 2024 •

edited