On Fri, May 23, 2025 at 06:40:38PM +0100, James Le Cuirot wrote:
> The kernel we're building against may have had its tools (e.g. modpost)
> built for the target arch or even some other arch than the we're
> building with now. To work around this, rebuild those tools with make
> modules_prepare when necessary.
>
> Doing this in pkg_setup is not ideal, but some ebuilds have their own
> src_configure and src_compile functions. It will not trigger in the vast
> majority of cases, and it is not done when installing binary packages.
>
> Signed-off-by: James Le Cuirot <ch...@gentoo.org>
> ---
> eclass/linux-mod-r1.eclass | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/eclass/linux-mod-r1.eclass b/eclass/linux-mod-r1.eclass
> index 16c280219ef89..a62f1b96bb5e9 100644
> --- a/eclass/linux-mod-r1.eclass
> +++ b/eclass/linux-mod-r1.eclass
> @@ -352,6 +352,31 @@ linux-mod-r1_pkg_setup() {
> _modules_set_makeargs
>
> _modules_sanity_gccplugins
> +
> + # Check whether modpost runs locally as it might have been built for a
> + # different arch. It should do nothing successfully when called without
> + # args. If it doesn't run, build it with make modules_prepare in a new
> + # environment using just the necessary files, and repoint KV_OUT_DIR
> there.
I'd kind of prefer if this was split like the rest into e.g. an
@INTERNAL _modules_prepare_cross or similar to match the rest of the
eclass and call it here after _set_makeargs and before gccplugins
because the latter builds using KV_OUT_DIR (not that it uses modpost,
and also isn't a big deal if that sanity check fails for cross, no
need to test it) and the former uses MAKEARGS.
The above text or similar could be used for the function description
(just split it at 72 chars, keep double spaces after periods, and
put the function before _modules_prepare_kernel for alphabetical).
And maybe start the function with:
[[ ${CBUILD:-${CHOST}} == "${KERNEL_CHOST}" ]] && return
It could avoid trying to set this up if modpost failed for another
reason (sometime users have kind of broken builds), not that it'd
be a big deal but may make errors more confusing.
> + if ! "${KV_OUT_DIR}"/scripts/mod/modpost &>/dev/null; then
> + # Try to run make modules_prepare in a new separate output
> directory.
> + # This cannot be done if there is not already a separate output
> + # directory because the build system will complain that the
> source
> + # directory is not clean. In that case, copy the source
> directory.
> + if [[ ${KV_OUT_DIR} -ef ${KV_DIR} ]]; then
> + cp -rLT --reflink=auto "${KV_OUT_DIR}"
> "${WORKDIR}"/extmod-build || die
fwiw the rest of the eclass uses --, be nice to have just to be
consistent (aka ... --reflink=auto -- "${KV_OUT_DIR}" ...). Not that
everything wouldn't explode either way if directories started with -
(I use it only on principle).
> + emake -C "${WORKDIR}"/extmod-build
> "${MODULES_MAKEARGS[@]}" modules_prepare
> + else
> + mkdir "${WORKDIR}"/extmod-build || die
mkdir -- too.
> + # Don't use /proc/config.gz as it probably won't match
> this kernel.
Can drop this comment, feels superfluous. Using build .config is kind
of obvious for modules, and /proc/config.gz may also not even exist
if support is disabled.
> + cp "${KV_OUT_DIR}"/{.config,Module.symvers}
> "${WORKDIR}"/extmod-build || die
cp --
> + # KV_OUT_DIR may have been prepared with
> install-extmod-build, which
> + # doesn't include all the files needed to call make
> modules_prepare,
> + # so use the Makefile from the full kernel sources.
> + emake -C "${WORKDIR}"/extmod-build
> "${MODULES_MAKEARGS[@]}" modules_prepare \
> + -f "${KERNEL_MAKEFILE}"
> KBUILD_OUTPUT="${WORKDIR}"/extmod-build
> + fi
> + KV_OUT_DIR="${WORKDIR}"/extmod-build
Can drop quotes here.
That aside, while it is not shell readonly, linux-info calls that
variable "A read-only variable" in its description, not a big deal
but does make overriding it feel like an abuse of internal details.
> + fi
> }
>
> # @FUNCTION: linux-mod-r1_src_compile
> --
> 2.49.0
>
(haven't really looked at linux-info changes nor tested cross, but
I trust that you know what you are doing here -- also linux-info is
an inconsistent mess so no need to be picky there)
--
ionen
signature.asc
Description: PGP signature
