In D53119#1213395, @mhorne wrote:

In D53119#1213368, @jrtc27 wrote:

The logic for removing this in the FDT is that, on Linux, the clock framework comes up early enough (or at least some early part of it) in order to be able to query the UART clock's frequency from the clock driver, whereas we don't do anything like that with clocks prior to newbus passes, so way too late for cninit.

Yes. I can note this in the commit message.

What do you think of the patch?

The logic for removing this in the FDT is that, on Linux, the clock framework comes up early enough (or at least some early part of it) in order to be able to query the UART clock's frequency from the clock driver, whereas we don't do anything like that with clocks prior to newbus passes, so way too late for cninit.

Is the expectation that we ensure all release media have all supported package sets present in an offline package repo? Because disc1/memstick won't have the real src package, for example, and bootonly/mini-memstick won't have any real packages at all. So I think for this to work even bootonly/mini-memstick needs to have a package repo of the package sets (as a pkgbase analogue to how it still has the MANIFEST file that lists all distribution sets, just the sets themselves aren't present)?

I think they should all be vital, in the sense that if you've installed that metapackage then you don't want your system to ever remove it without very explicit confirmation, no?

Please make base the default. bsdinstall defaults should be such that you can mash enter without thinking and get a sensible system. I suspect minimal is not what most want.

Is there a reason to limit this at all? If we’re restricting the set of kernels built already then should we not just pick up any and all kernel packages that release/Makefile got Makefile.inc1 to package?

I'm a bit uneasy about (a) removing the (Experimental) from pkgbase (b) adding the (Legacy) to Dist Sets when pkgbase is currently not fit for release whilst distribution sets are tried and tested. So this feels more aspirational than truthful, but I also understand the objective of the release is to do pkgbase... I just think we need to be very careful not to push pkgbase on our users too hard until we are truly willing to support it in the form they will be installing. I think doing so before we have metapackages integrated into bsdinstall, in particular, is premature.

I guess my view is that OLD_LIBS is really there so that soname bumps don't break existing binaries; you need the new library to be able to build the new binary, but don't want a window between installing the new library and being able to build the new binary where it doesn't work. That isn't really important for this case, because you can just not set WITHOUT_LIBFOO until you've updated all your binaries to not use that ABI. But at the same time, if we're using OLD_LIBS for all the libraries here, then yes, that's pointless if you're using OLD_FILES for rtld itself.

Does delete-old-libs even delete all of a given libcompat if you set WITHOUT_LIBFOO? Oh that's what this code is trying to do.

Just because sys/sysctl.h is self-contained doesn't mean certctl.c shouldn't also be including sys/types.h. Presumably it itself is using types from that header and so should explicitly include it, regardless of whether sys/sysctl.h is including it for its own needs (which is an implementation detail; what means it uses to give things it defines the right types is up to it). And given style(9) header ordering rules that would mean including sys/types.h first, which would render this obsolete.

It does appear both macOS and GNU date support -Iseconds. However, my concern with this change is that such timestamps are rather uglier and less human-readable, which is less of a problem for build system messages but more so for uname. I note that Linux uses LC_ALL=C date output for its uname, running uname -a on an Ubuntu system.

In D52387#1196252, @markj wrote:

In D52387#1196215, @jrtc27 wrote:

Are we sure all these scripts aren't set -u? Even if that's true, I don't love relying on that.

Yes, they are definitely not set -u-clean. I don't quite follow though: how does this patch rely on that? : ${FOO:=bar} is ok even with set -u configured.

Are we sure all these scripts aren't set -u? Even if that's true, I don't love relying on that.

Should this not be tied to SOURCE_DATE_EPOCH rather than some arbitrary and misleading timestamp (1st Jan 2024, which predates any release that will include this code)?

And to be clear, as far as I can tell, absolutely nothing has changed in Clang to make Clang 20 any less happy with this configuration than Clang 12; that is, I believe using Clang 12 with that configuration when this was committed would have been just as broken, and so adding any new version-specific behaviour is inappropriate (and so the subject and body of this review are misguided).

Ok so yeah there is the assumption that you're not doing that. Probably the correct fix is to drop the :S/^ld.//1W. But the other point to make is that your script is unnecessary, just pass --cross-toolchain=llvm@20 and it'll set all the X* variables for you to values that are known to work (because that's what CI is using), and you don't need to mess with PATH to make that work. You also do not need to set CC explicitly unless you really want to override the defaults, which I doubt.

In D52330#1195266, @wosch wrote:

In D52330#1195007, @jrtc27 wrote:

And what happens if you ls /opt/homebrew/opt/lld@20/bin?

$ ls -l /opt/homebrew/opt/lld@20/bin
total 128
lrwxrwxr-x 1 wosch admin 3 Jul 9 01:06 ld.lld -> lld
lrwxrwxr-x 1 wosch admin 3 Jul 9 01:06 ld64.lld -> lld
-r-xr-xr-x 1 wosch admin 62160 Sep 1 20:33 lld
lrwxrwxr-x 1 wosch admin 3 Jul 9 01:06 lld-link -> lld
lrwxrwxr-x 1 wosch admin 3 Jul 9 01:06 wasm-ld -> lld

$ /opt/homebrew/opt/lld@20/bin/ld.lld --version
Homebrew LLD 20.1.8 (compatible with GNU linkers)

calling “ldd” directly gives an error

$ /opt/homebrew/opt/lld@20/bin/lld --version
lld is a generic driver.
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead
echo $?
1

And what happens if you ls /opt/homebrew/opt/lld@20/bin?

Please commit this to main, it’s not appropriate to leave that broken

(By "this is odd" I mean "this change is definitely not right and will break other things even if it happens to end up producing a working build for you", so it's important to understand what exactly the problem you're seeing is rather than just deleting bits of code until your specific system is happy)

This looks odd. How were you trying to build? Have you seen https://github.com/freebsd/freebsd-src/commit/81fc74fd26da2f75c3010595db7a706c3e28d95b and its parent?

This does too many things in one change. Inferring whether it's pre- or post-install via looking at BSDINSTALL_CHROOT adds unnecessary complexity when the caller of the script already knows if it's pre-install or post-install. D52196 maintains that intentionality, and also better handles the time zone used during the install with a working /etc/localtime rather than needing to mess about with TZ. There may be ways you think the implementation there could be better, but I believe that the general design of it is better.

Note that I don't think the idea of setting the time early is inherently bad; in fact it makes sense to have the correct time for as much of the install as possible. But there's more work needed to be able to actually do it properly; for example, you likely need to have some kind of /etc/localtime -> /tmp/bsdinstall_etc/localtime or whatever like we do for resolv.conf, and then use *that* to populate /etc/localtime for the installed system. If that can be made to work, then I think it should be done unconditionally, otherwise it shouldn't be done at all.

In D51935#1188136, @ngie wrote:

In D51935#1188118, @kevans wrote:

Indeed, that run has also failed because we don't have ssl headers available on the host

Yes, I see the pickle you're dealing with now.

I'll get out of the way. This is not super ideal, but no less ideal than the current implementation I suppose.

In D51935#1188113, @ngie wrote:

In D51935#1188110, @ngie wrote:

I wouldn't say building libcrypto is small necessarily... I need to understand why certctl is needed during the build process instead of later on (as part of the install process).

certctl is used as part of distributeworld/installworld. Seems like a prime candidate to build as part of build-tools or cross-tools instead of prebuilding another copy/set of libraries.

I need to understand why certctl is needed during the build process instead of later on (as part of the install process).

What about release/scripts/pkgbase-stage.lua, which has a bunch of duplicated logic to decide which kernels to put on install media?

Fixes: 81d8827ad875 ("certctl: Reimplement in C")

Please land this with this one bug fixed (you can add my Reviewed by: for that modified version of this patch), assuming you've verified it does fix the build still

In D51924#1186691, @kib wrote:

In the description: s/eto/to/

Don't round up

In D51882#1185733, @lwhsu wrote:

Maybe make it optional and can be set in the config so we have "develop" and "release" modes?

Fix metalog output to be relative to DESTDIR.

I read mmel and jhb as objecting?

That looks a lot better now, thanks.

In D51823#1184158, @des wrote:

In D51823#1184154, @obiwac wrote:

I'm guessing I can shorten the hash and add the commit title in "Fixes:"?

I usually just use the first twelve characters of the hash and no commit title. If you include the title, it has to be ("in parentheses and double quotes").

Correctly-formatted commit message would be:

In D51516#1179413, @cperciva wrote:

For 15.0, pkgbase is going to be mandatory. So this can go in /etc/pkg in HEAD. For 14.x pkgbase is optional so it belongs in /usr/local/etc/pkg there.

To make this work in cheribuild I passed INSTALL="sh /path/to/src/tools/install.sh". Doing something automatically here that works would be good, though I'd want confirmation from someone like @sjg that ${.OBJDIR}/.. is definitely going to always be world's ${OBJTOP}.

In D50388#1178050, @markmi_dsl-only.net wrote:

I'd argue a little harder for MALLOC_PRODUCTION staying as-is since that's more broadly applicable. Though, we have src.conf knobs for both of these exactly so that folks can just flip them easily for their own use-cases without having to constantly argue defaults, and the current defaults reflect generally what we do with debug features in the kernel (as you noted).

I will note that as of pkg 2.1+ being involved, including 2.2.1, main-*, such as main-amd64 on beefy18 and main-arm64 on ampere2, take significantly longer than the others to finish bulk -Ca like builds on those machines. (This is in comparison to pkg 2.0.6 .) Think Pkgs_built_so_far/Elapsed_time_so_far decreasing rather systematically. The main-arm64 and main-armv7 cycle on ampere2 now takes about a month, so each is being updated only about 12 times a year for the rate, as stands. Not exactly good for keeping the security status up to date.

Also, it is worse than that long term: the more builds that have completed already, the longer the total for the stages build-depends, lib-depends, and run-depends when a later package has dependencies. With 10s of thousands of smaller-to-build packages, that trend is not viable long term. It is not just or primarily the big-build jobs that are taking more overall time: it is the increasing total across the vast number of smaller-to-build packages. (bapt has an idea for eventually using a different technique.)

In my view, having distinct builders what is to be distributed vs. doing builds for testing would help, such as using beefy17 as a second main-amd64 once it stops handling main-i386 might be a useful example. (ampere2 is not so easy to suggest an example for without getting more aarch64 systems.)

Anyway, I think there are definite tradeoffs with using WITHOUT_MALLOC_PRODUCTION for production on the scale of building the likes of 36,000 packages for distribution.

Please note in the commit message that this is specific to direct exec; I was initially confused how this was broken for so long without noticing as PDEs with TLS aren't that rare, especially prior to WITH_PIE being the default.

In D42095#1173152, @dfr wrote:

I think this is wrong. This makes it impossible to install caroot without pulling in all of FreeBSD-runtime. My though process is 'can I use caroot without certctl' and the answer is a qualified yes - it can be done by running certctl with DESTDIR set. Conversely, 'can I use certctl without caroot' - clearly not since certctl is useless without certs. Therefor (in my mind), certctl should depend on caroot, not the other way around.

This may be a stupid question as I've not reminded myself of the details of INTRng and IPIs, but why can't the PIC expose a single isrc that internally happens to send multiple interrupts as far as the hardware is concerned?