This change is needed to make cross-build/include/linux/fcntl.h compatible with LLVM since that uses ::open(...) which doesn't work with the statement macro.
Can also split this into a separate commit?

ncurses has a build-tools target so can't be built early enough to provide a bootstrap ncurses for the bootstrap-tools tblgen

Yes it would be, but then we get silent success without a lock if one of the bootstrap tools uses these flags. I think not defining it in that case is safer.

In general we want to make sure that bootstrap tools don't use sysctl/sysctlbyname() since they generally expect FreeBSD sysctl and then might not work as expected. However, in the case of LLVM we do need the sysctls() on macOS so we need some way of opting into using the real sysctl definitions.

It might be possible to build the bootstrap tools without linking ncurses. Will look into that.

That's what my patch did, worked just fine on macOS. On Linux I made it use -ltinfo since that's what LLVM's CMakeLists.txt auto-detects, though you might be able to get away without that too, I didn't try.

This is why ncurses is not needed for macos but is on FreeBSD/Linux. Will upload a new patch that avoids terminfo for bootstrap tools.

I think we should still enable it on FreeBSD, it works fine there already so this could potentially regress colours in certain weird cases. Simplifies the Makefiles too, you can use the same condition for execinfo as for ncursesw.

Shouldn't this already be included thanks to src.opts.mk -> bsd.compiler.mk -> bsd.linker.mk? I didn't need it in my patch.

That's fine too. Since ncurses is assumed to be available unconditionally on the host that seems fine.

The only problem I can think of is that it might cause issues for shared builds after upgrading ncurses soversion. Is that something we should worry about?

I seem to recall that I got build errors without it, will see if I can drop it.

Well that already needs to be handled carefully today, not a new issue. Plus bumping the ncurses soname is a rare event...

Given this is the vendored source, why not just #if 1 this? The condition should always be true in every situation we build this. Then (combined with my sysctl.h suggestion) you don't need LLVM_BOOTSTRAPPING.

This diff can be much more minimal if you just do #ifndef/#define 0 like for O_CLOEXEC above (i.e., that's all you need to do, then the existing code here just works). Not in the header, just this file, so it's still an error to try to use one of the missing flags. And yes, this should probably be a separate commit.

Hmm. You could make this a BOOTSTRAPPING_WANT_NATIVE_SYSCTL define? That way it's both clearer and cleaner IMO.

Yeah that's what I had initially (with a different spelling), will update to use yours.

Why's this commented-out? It should build fine during buildworld?

This no longer will be passed when building the cross-toolchain on FreeBSD, which I would guess risks breaking some old system GCCs from when libstdc++ was used. I suggest adding the old .if ${.MAKE.OS} == "FreeBSD" || !defined(BOOTSTRAPPING) to this rather than putting it in the .else.

It is fine during buildworld (but not during bootstrap). However, we don't build clangd/clang-query so it just adds to the build time unnecessarily.

Sounds good will fix.

Will push a pep8 fix commit for that script shortly.

A dumb question: when building bootstrap-tools during buildworld, llvm-tblgen is built and libllvmminimal.a is linked in. The latter contains references to backtrace() since config.h defines HAVE_BACKTRACE unconditionally and llvm::sys::PrintStackTrace() uses it, so the build fails since libexecinfo isn't linked. BOOTSTRAPPING is defined to 0 by the top-level makefile, so how is this supposed to work?

glibc defines it, or at least it does in the 2.27 version present in Ubuntu 18.04, and macOS defines it in libsystem_c.dylib

I see, thanks.