Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 938130 - kernel-build.eclass: restores savedconfig after doing /etc/kernel/config.d merging, clobbers snippets (poor interaction with savedconfig)
Summary: kernel-build.eclass: restores savedconfig after doing /etc/kernel/config.d me...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal
Assignee: Distribution Kernel Project
URL:
Whiteboard:
Keywords: PullRequest
Depends on:
Blocks:
 
Reported: 2024-08-18 02:45 UTC by Sam James
Modified: 2024-08-23 19:25 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-18 02:45:54 UTC
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?
Comment 1 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-18 02:47:03 UTC
Note that this shows up with making e.g. USE=debug ineffective with savedconfig too, which feels unfortunate.
Comment 2 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-20 05:39:33 UTC
(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?
Comment 3 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-21 08:12:38 UTC Comment hidden (obsolete)
Comment 4 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-21 08:21:39 UTC Comment hidden (obsolete)
Comment 5 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-08-21 08:29:26 UTC
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.
Comment 6 Larry the Git Cow gentoo-dev 2024-08-23 19:25:44 UTC
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(-)