Simplify building host tools during DIRDEPS_BUILD

[sjg]

Summary

The whole point of the DIRDEPS_BUILD is to avoid tree walks
and basically build everything in a single pass.
We use the pseudo MACHINE "host" to represent the build host.

When the build host is not FreeBSD or is an older version of FreeBSD
it may need some help to build host-tools.

The directory tools/build does this - building libegacy.

local.dirdeps.mk: create a pseudo option MK_host_egacy to indicate
if tools/build needs to be built for "host".
If so MK_host_egacy.host= yes which will only apply when DEP_MACHINE is
"host" and MK_host_egacy= no covers everyone else.

This allows a Makefile.depend.options in makefs to cause tools/build
to be built for host but only if necessary.

local.init.mk: use ISYSTEM as arg to -isystem so that it can be overridden.
The default remains ${STAGE_INCLUDEDIR}

Also if HAVE_FLAGS is not empty add a bunch of -DHAVE_* flags to CFLAGS
with value of ${HAVE_*} defaulting to 1. This makes it easy for
a linux host to override HAVE_* flags with 0 where needed.

src.init.mk: if MACHINE is host, include src.init.${.MAKE.OS:tl}.mk
if it exists.

For older versions of FreeBSD add libegacy when building PROGs for "host"

Also instead of -isystem${STAGE_INCLUDEDIR} we want
-I${STAGE_INCLUDEDIR} and -isystem/usr/include so we override ISYSTEM.
This means any headers we stage for "host" will take precedence over
system headers but #include_next will DTRT.

src.init.linux.mk: add
-I${SRCTOP}/tools/build/cross-build/include/linux
and generally deal with building host tools on Linux.
Eg. static linking does not work so set NO_SHARED= no
Override some HAVE_ flags.

src.sys.env.mk: on linux awk throws an warning about # in newvers.sh
just send stderr to /dev/null

Sponsored by: Juniper Networks, Inc.

[sjg]

FWIW None of these changes affect targets like buildworld,

MACHINE=host make host-tools-jobs "just works" on Ubuntu 20 and FreeBSD 13.1

of course I actually use mk-host host-tools-jobs

[arichardson]

share/mk/src.init.linux.mk

28

This workaround should already be in the include/Linux headers

[arichardson]

share/mk/src.init.linux.mk

6

Pretty sure you also want the common/ headers here

[imp]

share/mk/src.sys.env.mk

27 ↗

(On Diff #120790)

why 2>/dev/null?
Shouldn't the warning from gawk be fixed instead? I think it's a legit complaint and the proper fix is there:

-RELDATE=$(awk '/^\#define[[:space:]]*__FreeBSD_version/ {print $3}' ${PARAMFILE:-${SYSDIR}/sys/param.h})
+RELDATE=$(awk '/^#define[[:space:]]*__FreeBSD_version/ {print $3}' ${PARAMFILE:-${SYSDIR}/sys/param.h})

Also, if you're looking for 'chunking' advice: had this been the right fix, this one file would have been best to be a single commit rather than lumping it in with the rest. In the past, we've recommended big dumps for things from vendors, but with git, it's a lot easier to break them down than with svn where it was nearly impossible.

[sjg]

We no longer need HAVE_STRUCT_DINODE_DI_SHORTLINK

[imp]

share/mk/local.dirdeps.mk

67

Is the verison check here too strict? I forget if this is a major number check or what? Maybe fix the comment s/old/different major version/? And what about building in the future? Eg, the host is newer than what we're building (eg 13-stable on 14-current)

83

This aside likely should be its own commit. Also, I'm not sure what it's trying to say here...

share/mk/local.init.mk

23

There's no space here, is that intentional?

This chunk could also be it's own commit: It's an independent building block of the larger goal.

45

So what are HAVE_FLAGS? Why do we add them? The substitution is puzzling enough to warrant at least an aside anyway.

This also seems to be unconnected to anything else that's going in, so seems to be another building block that could be done separately.

share/mk/src.init.linux.mk

22

How does HAVE_foo -> HAVE_FLAGS?

33

Maybe add

# Bring in the full GNU namespace

or some such. These flags are alien to most FreeBSD readers and look at first blush like they might be an ugly hack, when in reality it's the standard way to request a richer environment.

share/mk/src.init.mk

14

The version check doesn't match above... why? And why not use the MK.xxx defined above with the same check?

[sjg]

share/mk/src.init.linux.mk

6

I tried that at one point - it did not work as well.
common includes headers that we apparently don't want, which is why the selected ones get staged, also this arrangement allows use of #include_next to work well

28

Without this I get failure in tools/build/setmode.o
`In file included from /usr/include/sys/sysctl.h:45,

from /b/sjg/work/FreeBSD/sb/src/lib/libc//gen/setmode.c:42:

/usr/include/linux/sysctl.h:42:24: error: expected identifier or '(' before '[' token

42 |  unsigned long __unused[4];

`

share/mk/src.sys.env.mk

27 ↗

(On Diff #120790)

Thanks I can make that fix (if you haven't already) I *assumed* the \\ was there
for a reason ;-)

And agree about committing that independently.

[sjg]

share/mk/local.dirdeps.mk

67

This is a major number check, I needed this to be able to build on 13.1

83

The method being used here to deal with optional dependencies is out-of-date.
Makefile.depend.options was designed later to deal with the issue much better.
We will be slowly adding those to the tree as needed.
I can just drop this comment - this section will get likely get removed in future.

share/mk/local.init.mk

23

Yes, the lack of space is intentional.
I have tools for analyzing meta files, and it is handy to break the command into one word per line - so you can examine/compare -Is or -Ds etc
Avoding space after -system allows those tools to show what its value was - eg. include -isystem in the -I output.

45

The use would be more apparent in the other review which jrtc27 objected to as being "too much". Eg. in makefs there is a -DHAVE_STRUCT_STAT_ST_FLAGS=1 but that is incorrect for Linux at least.
Rather than litter all the tree with checks for Linux, this indrection allows src.init.linux.mk to set HAVE_STRUCT_STAT_ST_FLAGS=0 and everything "just work"

I can add a comment.

share/mk/src.init.linux.mk

22

In makefs/Makefile for example rather than:
CFLAGS+=-DHAVE_STRUCT_STAT_ST_FLAGS=1
we have
HAVE_FLAGS+= HAVE_STRUCT_STAT_ST_FLAGS

33

Sure

[sjg]

share/mk/src.init.mk

14

In this case we are only dealing with old FreeBSD which will need -legacy
Non-FreeBSD hosts got dealt with via src.init.${.MAKE.OS:tl}.mk

Are you referring to MK_host_egacy above? I thought of that, probably a good idea - better name suggestions welcome of course ;-)

[jrtc27]

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

[imp]

share/mk/src.sys.env.mk

27 ↗

(On Diff #120790)

I think it was a mistake that happened to work.
I've not done the deep dive as to why it was there, when it appeared, etc.

[imp]

share/mk/src.sys.env.mk

27 ↗

(On Diff #120790)

also, I just went ahead and pushed this in.

[sjg]

Address some feedback

[sjg]

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

share/mk/local.dirdeps.mk

67

As for building older code base on newer host, we generally have not found that to be an issue (10 on 11, 11 on 12, 10,11,12 on 13 etc)

[jrtc27]

In D39744#904672, @sjg wrote:

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

Ok, but I would much prefer the two builds use the same approach. Having divergence in the compatibility headers / source files / etc will lead to madness.

[sjg]

In D39744#904675, @jrtc27 wrote:

In D39744#904672, @sjg wrote:

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

Ok, but I would much prefer the two builds use the same approach. Having divergence in the compatibility headers / source files / etc will lead to madness.

I don't disagree, but I don't see that being possible. None of these changes are harmful to the non-dirdeps build.

[jrtc27]

In D39744#904676, @sjg wrote:

In D39744#904675, @jrtc27 wrote:

In D39744#904672, @sjg wrote:

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

Ok, but I would much prefer the two builds use the same approach. Having divergence in the compatibility headers / source files / etc will lead to madness.

I don't disagree, but I don't see that being possible. None of these changes are harmful to the non-dirdeps build.

These ones don't touch it, but you had various diffs to the C sources for base programs that are harmful to anything but the dirdeps-on-non-freebsd build; we've tried hard to limit the diffs to base sources and contain it in build system changes and tools/build/cross-build compatibility bits.

[sjg]

In D39744#904679, @jrtc27 wrote:

In D39744#904676, @sjg wrote:

In D39744#904675, @jrtc27 wrote:

In D39744#904672, @sjg wrote:

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

Ok, but I would much prefer the two builds use the same approach. Having divergence in the compatibility headers / source files / etc will lead to madness.

I don't disagree, but I don't see that being possible. None of these changes are harmful to the non-dirdeps build.

These ones don't touch it, but you had various diffs to the C sources for base programs that are harmful to anything but the dirdeps-on-non-freebsd build; we've tried hard to limit the diffs to base sources and contain it in build system changes and tools/build/cross-build compatibility bits.

The changes in makefs have been discarded, there are only 2 changes outside of tools/build/ contrib/mtree uses HAVE_STRUCT_STAT_ST_FLAGS in a manner inconsistent with elsewhere (where the value matters) the changes are harmless

[jrtc27]

In D39744#904697, @sjg wrote:

In D39744#904679, @jrtc27 wrote:

In D39744#904676, @sjg wrote:

In D39744#904675, @jrtc27 wrote:

In D39744#904672, @sjg wrote:

In D39744#904652, @jrtc27 wrote:

I don’t understand why you need these hacks for makefs and mkimg when they already build just fine in the non-dirdeps case.

I don't either, but the non-dirdeps build is impossible to decompose in any sensible manner, it depends on side effects of tree walks etc.

Ok, but I would much prefer the two builds use the same approach. Having divergence in the compatibility headers / source files / etc will lead to madness.

I don't disagree, but I don't see that being possible. None of these changes are harmful to the non-dirdeps build.

These ones don't touch it, but you had various diffs to the C sources for base programs that are harmful to anything but the dirdeps-on-non-freebsd build; we've tried hard to limit the diffs to base sources and contain it in build system changes and tools/build/cross-build compatibility bits.

The changes in makefs have been discarded, there are only 2 changes outside of tools/build/ contrib/mtree uses HAVE_STRUCT_STAT_ST_FLAGS in a manner inconsistent with elsewhere (where the value matters) the changes are harmless

Ok, but you really should be getting things like __va_list from tools/build/cross-build/include/common/sys/_types.h. We don't want two ways of doing the same thing in-tree (and it's in your own best interest, too, longer term, since it means whenever someone works on the non-dirdeps build to add or fix something you'll also get the fix, rather than having to rediscover it yourself).

[sjg]

The changes in makefs have been discarded, there are only 2 changes outside of tools/build/ contrib/mtree uses HAVE_STRUCT_STAT_ST_FLAGS in a manner inconsistent with elsewhere (where the value matters) the changes are harmless

Ok, but you really should be getting things like __va_list from tools/build/cross-build/include/common/sys/_types.h. We don't want two ways of doing the same thing in-tree (and it's in your own best interest, too, longer term, since it means whenever someone works on the non-dirdeps build to add or fix something you'll also get the fix, rather than having to rediscover it yourself).

lib/libc/stdlib/getopt_long.c
usr.bin/mkimg/image.c

can be fixed by just adding #include <sys/types.h> before including err.h
but libnetbsd will need more work

[jrtc27]

In D39744#904712, @sjg wrote:

The changes in makefs have been discarded, there are only 2 changes outside of tools/build/ contrib/mtree uses HAVE_STRUCT_STAT_ST_FLAGS in a manner inconsistent with elsewhere (where the value matters) the changes are harmless

Ok, but you really should be getting things like __va_list from tools/build/cross-build/include/common/sys/_types.h. We don't want two ways of doing the same thing in-tree (and it's in your own best interest, too, longer term, since it means whenever someone works on the non-dirdeps build to add or fix something you'll also get the fix, rather than having to rediscover it yourself).

lib/libc/stdlib/getopt_long.c
usr.bin/mkimg/image.c

can be fixed by just adding #include <sys/types.h> before including err.h
but libnetbsd will need more work

Both build today for non-dirdeps, as does libnetbsd, so you really should not need to patch them.

[sjg]

Ensure sys/types.h included before err.h and we don't have issue with va_list

[arichardson]

share/mk/src.init.linux.mk

6

https://github.com/freebsd/freebsd-src/pull/725 shows that including both works just fine:

CFLAGS+= -I${SRCTOP}/tools/build/cross-build/include/linux
CFLAGS+= -I${SRCTOP}/tools/build/cross-build/include/common

23

These are set in tools/build/cross-build/include/linux/nbtool_config.h, which is why Makefile.boot passes -DHAVE_NBTOOL_CONFIG_H=1

30

I am pretty sure you only want one of those flags - -D_GNU_SOURCE will enable all the extensions. I also think -D_XOPEN_SOURCE without a number argument won't do anything.

[sjg]

Make src.init.linux.mk mimic what buildworld does wrt -Is

[sjg]

We only need -D_GNU_SOURCE

[arichardson]

Thanks for the update, looks like it's much closer to the build world flags now. Would be good to know if the __unused bits can be removed but otherwise looks good to me (well the parts that I understand).

share/mk/src.init.linux.mk

22

This __unused workaround is not needed for the normal build world. Does it work now that you also have the common includes?

[sjg]

Remove -D__unused no longer needed.

[sjg]

share/mk/src.init.linux.mk

22

A test I did this afternoon failed without this, but seems we don't need it now.
Will remove

[sjg]

Move some common bits of non-FreeBSD handling out of src.init.linux.mk

[arichardson]

Looks good to me now but please wait for @jrtc27 and/or @imp before committing.

share/mk/src.init.mk

16

It would be nice if we could share these settings between the different build configurations but that cleanup can be done later.

[jrtc27]

Can't comment on whether it's exactly right but nothing here that looks surprising any more and like it deviates obviously from normal buildworld.