| Summary: | kernel-build.eclass: restores savedconfig after doing /etc/kernel/config.d merging, clobbers snippets (poor interaction with savedconfig) | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Sam James <sam> |
| Component: | Eclasses | Assignee: | Distribution Kernel Project <dist-kernel> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | andrewammerlaan |
| Priority: | Normal | Keywords: | PullRequest |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: | https://github.com/gentoo/gentoo/pull/38238 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
Note that this shows up with making e.g. USE=debug ineffective with savedconfig too, which feels unfortunate. (In reply to Sam James from comment #0) > I'm not sure if my patch is right, or maybe it's almost right but we > shouldn't include the base config, just consider user snippets instead when > merging with savedconfig? After sitting on this a few days and trying it on more machines, I think we should clobber version.config and so on (otherwise you can't override it in savedconfig), and just restore before calling kernel-build_merge_configs? That would make it (untested, will now):
```
--- a/eclass/kernel-build.eclass
+++ b/eclass/kernel-build.eclass
@@ -243,9 +243,6 @@ kernel-build_src_configure() {
MAKEARGS+=( KBZIP2="lbzip2" )
fi
- restore_config .config
- [[ -f .config ]] || die "Ebuild error: please copy default config into .config"
-
if [[ -z "${KV_LOCALVERSION}" ]]; then
KV_LOCALVERSION=$(sed -n -e 's#^CONFIG_LOCALVERSION="\(.*\)"$#\1#p' \
.config)
diff --git a/sys-kernel/gentoo-kernel/gentoo-kernel-6.6.47.ebuild b/sys-kernel/gentoo-kernel/gentoo-kernel-6.6.47.ebuild
index d037ffe5cc80..d18a8094cda9 100644
--- a/sys-kernel/gentoo-kernel/gentoo-kernel-6.6.47.ebuild
+++ b/sys-kernel/gentoo-kernel/gentoo-kernel-6.6.47.ebuild
@@ -149,5 +149,8 @@ src_prepare() {
use secureboot && merge_configs+=( "${dist_conf_path}/secureboot.config" )
+ restore_config .config
+ [[ -f .config ]] || die "Ebuild error: please copy default config into .config"
+
kernel-build_merge_configs "${merge_configs[@]}"
}
```
But we need to then figure out how to handle vanilla-kernel, which was relying on the eclass to handle it.
(In reply to Sam James from comment #3) > That would make it (untested, will now): > [...] This is incorrect. Let me fix it. Fixed variant (it's a bit ugly but it gets the intent across; I don't care what style we use as long as it's functionally the same):
```
diff --git a/eclass/kernel-build.eclass b/eclass/kernel-build.eclass
index c4f3db0028a9..2908a290b3e2 100644
--- a/eclass/kernel-build.eclass
+++ b/eclass/kernel-build.eclass
@@ -243,9 +243,6 @@ kernel-build_src_configure() {
MAKEARGS+=( KBZIP2="lbzip2" )
fi
- restore_config .config
- [[ -f .config ]] || die "Ebuild error: please copy default config into .config"
-
if [[ -z "${KV_LOCALVERSION}" ]]; then
KV_LOCALVERSION=$(sed -n -e 's#^CONFIG_LOCALVERSION="\(.*\)"$#\1#p' \
.config)
diff --git a/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.6.ebuild b/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.6.ebuild
index d037ffe5cc80..2b143e142f46 100644
--- a/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.6.ebuild
+++ b/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.6.ebuild
@@ -125,29 +125,38 @@ src_prepare() {
echo "CONFIG_LOCALVERSION=\"${myversion}\"" > "${T}"/version.config || die
local dist_conf_path="${WORKDIR}/gentoo-kernel-config-${GENTOO_CONFIG_VER}"
- local merge_configs=(
- "${T}"/version.config
- "${dist_conf_path}"/base.config
- )
- use debug || merge_configs+=(
- "${dist_conf_path}"/no-debug.config
- )
- if use hardened; then
- merge_configs+=( "${dist_conf_path}"/hardened-base.config )
+ local merge_configs=()
+
+ if ! use savedconfig ; then
+ merge_configs+=(
+ "${T}"/version.config
+ "${dist_conf_path}"/base.config
+ )
+
+ use debug || merge_configs+=(
+ "${dist_conf_path}"/no-debug.config
+ )
- tc-is-gcc && merge_configs+=( "${dist_conf_path}"/hardened-gcc-plugins.config )
+ if use hardened; then
+ merge_configs+=( "${dist_conf_path}"/hardened-base.config )
- if [[ -f "${dist_conf_path}/hardened-${ARCH}.config" ]]; then
- merge_configs+=( "${dist_conf_path}/hardened-${ARCH}.config" )
+ tc-is-gcc && merge_configs+=( "${dist_conf_path}"/hardened-gcc-plugins.config )
+
+ if [[ -f "${dist_conf_path}/hardened-${ARCH}.config" ]]; then
+ merge_configs+=( "${dist_conf_path}/hardened-${ARCH}.config" )
+ fi
+ fi
+
+ # this covers ppc64 and aarch64_be only for now
+ if [[ ${biendian} == true && $(tc-endian) == big ]]; then
+ merge_configs+=( "${dist_conf_path}/big-endian.config" )
fi
- fi
- # this covers ppc64 and aarch64_be only for now
- if [[ ${biendian} == true && $(tc-endian) == big ]]; then
- merge_configs+=( "${dist_conf_path}/big-endian.config" )
+ use secureboot && merge_configs+=( "${dist_conf_path}/secureboot.config" )
fi
- use secureboot && merge_configs+=( "${dist_conf_path}/secureboot.config" )
+ restore_config .config
+ [[ -f .config ]] || die "Ebuild error: please copy default config into .config"
kernel-build_merge_configs "${merge_configs[@]}"
}
```
But we need to then figure out how to handle vanilla-kernel, which was relying on the eclass to handle it.
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=e290c3c78b7acb59393f46d1d15175d6dbfc77da commit e290c3c78b7acb59393f46d1d15175d6dbfc77da Author: Michał Górny <mgorny@gentoo.org> AuthorDate: 2024-08-21 15:19:07 +0000 Commit: Michał Górny <mgorny@gentoo.org> CommitDate: 2024-08-23 19:25:40 +0000 kernel-build.eclass: Apply savedconfig on top of merged configs Move applying savedconfig from src_configure() into kernel-build_merge_configs(), in order to make it possible to override saved config options. Previously, the saved config would unconditionally overwrite everything, which could be between inconvenient and plain broken (particularly if savedconfig contained signing key paths referring to ${T}). The new logic applies saved config via merge method, on top of the default config along with ebuild and eclass overrides. However, MODULES_SIGN_KEY* and user config snippets do override saved config for convenience. Closes: https://bugs.gentoo.org/938130 Signed-off-by: Michał Górny <mgorny@gentoo.org> eclass/kernel-build.eclass | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) |
I'm not sure if this is a bug, intended savedconfig semantics, or what. It kind of feels like everyone has their own idea of how savedconfig should work. With USE=savedconfig on sys-kernel/gentoo-kernel, any /etc/kernel/config.d snippets seem to take no effect because: 1) gentoo-kernel's src_prepare does the config merging, then 2) kernel-build.eclass impl of src_configure does restore_config, clobbering the earlier changes If I apply the following: ``` --- a/eclass/kernel-build.eclass +++ b/eclass/kernel-build.eclass @@ -243,9 +243,6 @@ kernel-build_src_configure() { MAKEARGS+=( KBZIP2="lbzip2" ) fi - restore_config .config - [[ -f .config ]] || die "Ebuild error: please copy default config into .config" - if [[ -z "${KV_LOCALVERSION}" ]]; then KV_LOCALVERSION=$(sed -n -e 's#^CONFIG_LOCALVERSION="\(.*\)"$#\1#p' \ .config) --- a/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.5.ebuild +++ b/sys-kernel/gentoo-kernel/gentoo-kernel-6.10.5.ebuild @@ -125,6 +125,9 @@ src_prepare() { echo "CONFIG_LOCALVERSION=\"${myversion}\"" > "${T}"/version.config || die local dist_conf_path="${WORKDIR}/gentoo-kernel-config-${GENTOO_CONFIG_VER}" + restore_config .config + [[ -f .config ]] || die "Ebuild error: please copy default config into .config" + local merge_configs=( "${T}"/version.config "${dist_conf_path}"/base.config ``` then things seem to work as expected for /etc/kernel/config.d snippets. I'm not sure if my patch is right, or maybe it's almost right but we shouldn't include the base config, just consider user snippets instead when merging with savedconfig?