Unbreak and clean up hacks to use host packages as hybrid packages #3
Conversation
When we're building natively we want to make sure we don't use the host's ABI, nor do we even want to use its pkg. It's quite possible I've now broken something for the qemu-user case, but that should be fixed following this approach, as the old approach was conceptually wrong at its core. This reinstates behaviour from prior to 5fdad4a ("Choose a pkg repository directory for pkg or pkg64"), e88ac9d ("Bootstrap hybrid ABI pkg when starting a jail") and 3d222b1 ("Install toolchain when starting a jail") where native building worked just fine and didn't pollute the jail with the host's ABI. As part of this, we stop installing llvm-morello into /toolchain and put it in /usr/local64 with all the other hybrid ABI packages (whether host or actual hybrid). This means it lives where everyone expects it to for the normal hybrid case, and the host "hybrid" case mirrors that rather than unnecessarily diverging. Also stop passing in various metadata about the jail and instead just compute it inside the function, since it's always the same at every call site and thus unnecessary boilerplate (and for the real hybrid case some of the arguments are unused so them existing but not doing anything is confusing).
| @@ -69,42 +69,41 @@ hybridset_list() { | |||
| (cd "${MASTER_DATADIR_ABS}" && find "hybridset" -type d -depth 2 | cut -d / -f 3 | sort -u) | |||
| } | |||
|
|
|||
| hybridset_is_really_host() { | |||
| # XXX: Host ABI used as a proxy for whether we're building natively or | |||
There was a problem hiding this comment.
NB: This probably should be looking at QEMU_EMULATING instead
There was a problem hiding this comment.
We should test if the host is FreeBSD instead.
There was a problem hiding this comment.
I thought you were building on CheriBSD/arm64 these days, so that would be insufficient.
There was a problem hiding this comment.
We also build CheriABI packages on FreeBSD/amd64. We could set some flag in make.conf to indicate we're in the Poudriere infrastructure.
| download_toolchain_from_repo | ||
| download_hybridset_pkg_from_repo |
There was a problem hiding this comment.
Is there a reason you did them this way round? When using the jail's pkg this doesn't make sense as an order, and I don't see what the problem is doing them the other way round for the hybrid-means-host case.
There was a problem hiding this comment.
Are you asking why download_hybridset_pkg_from_repo is called after download_toolchain_from_repo? I agree it should be the other way around.
In general, we have to take into account the following cases:
We do use the host's ABI for emulated environments, e.g. FreeBSD:14:amd64, FreeBSD:14:arm64. We can either use the same approach in all cases and always use the host's ABI with host libraries mounted over nullfs within a Poudriere jail or treat the cases making use of FreeBSD as special ones. The former approach is what we've been using so far. I understand you want to make CTSRD-CHERI/poudriere support CheriBSD-hosted environments first, and treat other cases as special with additional hacks. I agree with this approach as we're past the stage we build packages only for end users and we want to support Poudriere on CheriBSD itself. However, to distinguish CheriBSD- and FreeBSD-hosted environments, we cannot look at
We cannot install llvm-morello in /usr/local64 because it would conflict with building the devel/llvm-morello port. I install llvm-morello in /toolchain to build devel/llvm-morello, install devel/llvm-morello in /usr/local64 and allow ports depending on devel/llvm-morello use the variant from /usr/local64. Again, we could install llvm-morello in /usr/local64 when building CheriABI packages but I don't think it's worth it as it would introduce yet another case we have to support and remember about. |
| download_toolchain_from_repo | ||
| download_hybridset_pkg_from_repo |
There was a problem hiding this comment.
Are you asking why download_hybridset_pkg_from_repo is called after download_toolchain_from_repo? I agree it should be the other way around.
| if ! hybridset_is_really_host; then | ||
| return | ||
| fi |
There was a problem hiding this comment.
Why are you returning here? At the beginning of this function, we return in non-CheriABI cases. This check makes the rest of this function ignored as it returns in the CheriABI case.
We need to update /usr/local64/etc/pkg.conf to set ABI to FreeBSD:14:{amd64,arm64} when building on FreeBSD.
There was a problem hiding this comment.
Because this code emphatically must not be run in a native CheriABI jail, the whole point of this PR?
There was a problem hiding this comment.
(hybridset_is_really_host is supposed to abstract away precisely whether you want to mess with pkg.conf to be host packages or not, that's the whole point of it)
There was a problem hiding this comment.
Isn't the rest of this function just dead code?
There was a problem hiding this comment.
Also, hybridset_is_really_host is a really confusing name. Maybe hybridset_is_native would be more informative (with flipped logic).
| @@ -69,42 +69,41 @@ hybridset_list() { | |||
| (cd "${MASTER_DATADIR_ABS}" && find "hybridset" -type d -depth 2 | cut -d / -f 3 | sort -u) | |||
| } | |||
|
|
|||
| hybridset_is_really_host() { | |||
| # XXX: Host ABI used as a proxy for whether we're building natively or | |||
There was a problem hiding this comment.
We should test if the host is FreeBSD instead.
The hybrid ABI package manager should be bootstrapped when a jail is started to provide it to both poudriere bulk and poudriere jail -s operations. Having it available for poudriere jail -s, we can manually build CheriABI packages using hybrid ABI utilities to debug building issues.
Before this change, the toolchain was installed when a jail is created. That approach modifies a root filesystem directory which a user might want to use for other purposes than Poudriere. Also, it doesn't use the latest available toolchain to build packages but a version available when creating the jail. In order to use the latest available toolchain and not disrupt the root filesystem directory, install the toolchain when starting a jail. This change also adds support to install the toolchain to build packages for both Arm Morello and CHERI-RISC-V when using Poudriere on CHERI-extended hardware or with the user mode.
Building the toolchain is inherently a special case that will need special handling, but putting the toolchain for normal package builds anywhere other than /usr/local64 is a bad idea. In particular, /toolchain gets baked into some package builds; you can't pip install things, for example, that want to use a C compiler, because they use that not /toolchain. Package build environments need to look like the normal environment. Plus when you're using a purecap jail you absolutely want to just pkg64 install llvm-morello, no messing with weird databases. The reason we're in this mess with Poudriere not working well natively is because the special case has been treated as the normal case and as a result the normal case doesn't work as it should. This PR is an attempt to flip that around and handle only the special cases weirdly. |
I agree with the point the environment should look like the one a user will use. However, we have to deal with the following issues before we merge this as it would prevent package building in the Poudriere infrastructure:
|
When we're building natively we want to make sure we don't use the host's ABI, nor do we even want to use its pkg. It's quite possible I've now broken something for the qemu-user case, but that should be fixed following this approach, as the old approach was conceptually wrong at its core.
This reinstates behaviour from prior to 5fdad4a ("Choose a pkg repository directory for pkg or pkg64"), e88ac9d ("Bootstrap hybrid ABI pkg when starting a jail") and 3d222b1 ("Install toolchain when starting a jail") where native building worked just fine and didn't pollute the jail with the host's ABI.
As part of this, we stop installing llvm-morello into /toolchain and put it in /usr/local64 with all the other hybrid ABI packages (whether host or actual hybrid). This means it lives where everyone expects it to for the normal hybrid case, and the host "hybrid" case mirrors that rather than unnecessarily diverging.
Also stop passing in various metadata about the jail and instead just compute it inside the function, since it's always the same at every call site and thus unnecessary boilerplate (and for the real hybrid case some of the arguments are unused so them existing but not doing anything is confusing).