User Details
- User Since
- Jul 4 2018, 7:23 PM (379 w, 3 d)
Tue, Oct 7
Mon, Oct 6
Sat, Sep 20
Fri, Sep 19
Thu, Sep 18
Tue, Sep 16
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)?
Mon, Sep 15
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.
Sep 10 2025
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.
Sep 9 2025
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.
Sep 7 2025
Sep 6 2025
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.
Sep 4 2025
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).
Sep 3 2025
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.
Sep 2 2025
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?
Aug 29 2025
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.
Aug 28 2025
Aug 27 2025
Aug 23 2025
Aug 22 2025
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.
Aug 19 2025
I need to understand why certctl is needed during the build process instead of later on (as part of the install process).
Aug 18 2025
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
Aug 16 2025
Aug 15 2025
jrtc4@amoracia:~/freebsd-src$ LD_UTRACE=1 ktrace -t u ~/freebsd-obj/home/jrtc4/freebsd-src/amd64.amd64/libexec/rtld-elf/ld-elf.so.1 /usr/bin/true && kdump | grep loaded 80756 ld-elf.so.1 USER RTLD: loaded 0x5a069ee03408 @ 0x3dae988c1000 - 0x3dae988c12e5 ([vdso]) 80756 ld-elf.so.1 USER RTLD: loaded 0x5a069ee03808 @ 0x3dae98deb000 - 0x3dae990edfff (/lib/libc.so.7)
Don't round up
jrtc4@amoracia:~/freebsd-src$ LD_UTRACE=1 ktrace -t u /libexec/ld-elf.so.1 /usr/bin/true && kdump | grep loaded 6613 ld-elf.so.1 USER RTLD: loaded 0x47728f403408 @ 0x1dbb7f8e9000 - 0x1dbb7f8e8fff ([vdso]) 6613 ld-elf.so.1 USER RTLD: loaded 0x47728f403808 @ 0x1dbb7ff71000 - 0x1dbb80273fff (/lib/libc.so.7)
Aug 14 2025
Fix metalog output to be relative to DESTDIR.
Aug 12 2025
Aug 9 2025
I read mmel and jhb as objecting?
Aug 8 2025
That looks a lot better now, thanks.
Correctly-formatted commit message would be:
Aug 7 2025
Jul 30 2025
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}.
Jul 27 2025
Jul 21 2025
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.
Jul 17 2025
Jul 15 2025
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?