[gentoo-dev] [PATCH 0/8] linux-info.eclass: cleanup & better non-Linux support

[gentoo-dev] [PATCH 0/8] linux-info.eclass: cleanup & better non-Linux support

[Michał Górny]

Hi, everyone.

Here's a patch set for linux-info.eclass. Besides some minor cleanup,
it improves error handling and adds explicit handling for non-Linux
systems.

Currently, the eclass pretty much wrongly assumes that every system
is Linux. This causes e.g. a lot of spurious warnings or completely
random behavior on FreeBSD. While we could technically solve it
by adding appropriate conditionals to ebuilds, it seems pointless
to have to add it everywhere if the eclass can never be useful
for non-Linux targets.

For the purpose of these patches, I've split the public-ish API
of the eclass into three groups:

a. check_extra_config & pkg_setup are commonly used in 'assert'-style
   with non-fatal results. Those functions are made no-ops on non-Linux
   systems.

b. Some functions are fatal assert-style, i.e. die if kernel doesn't
   support X. Those now die explicitly on non-Linux systems (as they
   would probably anyway).

c. Functions that provide true/false results and get version can't work
   correctly on FreeBSD, and the failure can't be cleanly expressed
   in true/false (think of kernel_is). Those functions now also die
   on non-Linux systems and needs to be guarded in ebuilds.

Please review.

--
Best regards,
Michał Górny

Michał Górny (8):
  linux-info.eclass: get_localversion, do not call 'ls'
  linux-info.eclass: Replace unnecessary $? checks
  linux-info.eclass: linux-info_get_any_version, die on failure
  linux-info.eclass: Move get_version to require_configured_kernel
  linux-info.eclass: require_configured_kernel, improve error handling
  linux-info.eclass: Ignore check_extra_config on non-Linux
  linux-info.eclass: Die in most of public-ish APIs on non-Linux
  linux-info.eclass: Skip linux_config_*_exists on non-Linux

 eclass/linux-info.eclass | 72 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 14 deletions(-)

-- 
2.16.2

[gentoo-dev] [PATCH 1/8] linux-info.eclass: get_localversion, do not call 'ls'

[Michał Górny]

Use bash array to perform a safe filename glob instead of calling 'ls'.
Also, use nullglob to cleanly handle no matches instead of silencing
errors.
---
 eclass/linux-info.eclass | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 035b722e2d6d..b8c1a524bae8 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -390,8 +390,13 @@ kernel_is() {
 get_localversion() {
 	local lv_list i x
 
+	local shopt_save=$(shopt -p nullglob)
+	shopt -s nullglob
+	local files=( ${1}/localversion* )
+	${shopt_save}
+
 	# ignore files with ~ in it.
-	for i in $(ls ${1}/localversion* 2>/dev/null); do
+	for i in "${files[@]}"; do
 		[[ -n ${i//*~*} ]] && lv_list="${lv_list} ${i}"
 	done
 
-- 
2.16.2

[gentoo-dev] [PATCH 2/8] linux-info.eclass: Replace unnecessary $? checks

[Michał Górny]

---
 eclass/linux-info.eclass | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index b8c1a524bae8..df6227220b79 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -628,8 +628,7 @@ get_running_version() {
 # This attempts to find the version of the sources, and otherwise falls back to
 # the version of the running kernel.
 linux-info_get_any_version() {
-	get_version
-	if [[ $? -ne 0 ]]; then
+	if ! get_version; then
 		ewarn "Unable to calculate Linux Kernel version for build, attempting to use running version"
 		get_running_version
 	fi
@@ -848,13 +847,11 @@ check_zlibinflate() {
 
 	ebegin "checking ZLIB_INFLATE"
 	linux_chkconfig_builtin CONFIG_ZLIB_INFLATE
-	eend $?
-	[ "$?" != 0 ] && die
+	eend $? || die
 
 	ebegin "checking ZLIB_DEFLATE"
 	linux_chkconfig_builtin CONFIG_ZLIB_DEFLATE
-	eend $?
-	[ "$?" != 0 ] && die
+	eend $? || die
 
 	local LINENO_START
 	local LINENO_END
-- 
2.16.2

[gentoo-dev] [PATCH 3/8] linux-info.eclass: linux-info_get_any_version, die on failure

[Michał Górny]

Make linux-info_get_any_version die if it can't determine any version
of the Linux kernel. This indicates a problem with the eclass code
(as it should not happen on Linux) and the missing KV_* variables
are going to cause random misbehavior and failures.
---
 eclass/linux-info.eclass | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index df6227220b79..37a60b430646 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -630,7 +630,9 @@ get_running_version() {
 linux-info_get_any_version() {
 	if ! get_version; then
 		ewarn "Unable to calculate Linux Kernel version for build, attempting to use running version"
-		get_running_version
+		if ! get_running_version; then
+			die "Unable to determine any Linux Kernel version, please report a bug"
+		fi
 	fi
 }
 
-- 
2.16.2

[gentoo-dev] [PATCH 4/8] linux-info.eclass: Move get_version to require_configured_kernel

[Michał Górny]

All require_configured_kernel calls in this eclass are followed
by a get_version call. Since even calling it proactively wouldn't hurt,
move it to require_configured_kernel. This saves us from having
to manually implement error handling for it everywhere.
---
 eclass/linux-info.eclass | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 37a60b430646..177cb7fafcde 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -295,6 +295,7 @@ require_configured_kernel() {
 		qeerror "it points to the necessary object directory so that it might find .config."
 		die "Kernel not configured; no .config found in ${KV_OUT_DIR}"
 	fi
+	get_version
 }
 
 # @FUNCTION: linux_chkconfig_present
@@ -646,7 +647,6 @@ linux-info_get_any_version() {
 check_kernel_built() {
 	# if we haven't determined the version yet, we need to
 	require_configured_kernel
-	get_version
 
 	local versionh_path
 	if kernel_is -ge 3 7; then
@@ -676,7 +676,6 @@ check_kernel_built() {
 check_modules_supported() {
 	# if we haven't determined the version yet, we need too.
 	require_configured_kernel
-	get_version
 
 	if ! linux_chkconfig_builtin "MODULES"; then
 		eerror "These sources do not support loading external modules."
@@ -831,7 +830,6 @@ check_extra_config() {
 check_zlibinflate() {
 	# if we haven't determined the version yet, we need to
 	require_configured_kernel
-	get_version
 
 	# although I restructured this code - I really really really dont support it!
 
-- 
2.16.2

[gentoo-dev] [PATCH 5/8] linux-info.eclass: require_configured_kernel, improve error handling

[Michał Górny]

Add error handling for failing get_version call
in require_configured_kernel. Give just a simple 'die' message since
the get_version function should verbosely explain the problem.
---
 eclass/linux-info.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 177cb7fafcde..5ffaf3f3b898 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -295,7 +295,7 @@ require_configured_kernel() {
 		qeerror "it points to the necessary object directory so that it might find .config."
 		die "Kernel not configured; no .config found in ${KV_OUT_DIR}"
 	fi
-	get_version
+	get_version || die "Unable to determine configured kernel version"
 }
 
 # @FUNCTION: linux_chkconfig_present
-- 
2.16.2

[gentoo-dev] [PATCH 6/8] linux-info.eclass: Ignore check_extra_config on non-Linux

[Michał Górny]

Make the check_extra_config and pkg_setup calls no-op on non-Linux
systems. Those systems don't have a Linux kernel, so they obviously
can't satisfy the requirements. This currently results in a lot of
useless warnings about missing Linux kernel sources on FreeBSD. We could
make it conditional per-package but there is really no point in adding
a lot of conditionals everywhere if this eclass can't ever work
on non-Linux.
---
 eclass/linux-info.eclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 5ffaf3f3b898..0ba7ff7755c7 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -110,6 +110,8 @@ inherit toolchain-funcs versionator
 
 EXPORT_FUNCTIONS pkg_setup
 
+IUSE="kernel_linux"
+
 # Overwritable environment Var's
 # ---------------------------------------
 KERNEL_DIR="${KERNEL_DIR:-${ROOT}usr/src/linux}"
@@ -688,8 +690,10 @@ check_modules_supported() {
 # @FUNCTION: check_extra_config
 # @DESCRIPTION:
 # It checks the kernel config options specified by CONFIG_CHECK. It dies only when a required config option (i.e.
-# the prefix ~ is not used) doesn't satisfy the directive.
+# the prefix ~ is not used) doesn't satisfy the directive. Ignored on non-Linux systems.
 check_extra_config() {
+	use kernel_linux || return
+
 	local config negate die error reworkmodulenames
 	local soft_errors_count=0 hard_errors_count=0 config_required=0
 	# store the value of the QA check, because otherwise we won't catch usages
@@ -902,6 +906,8 @@ check_zlibinflate() {
 # Force a get_version() call when inherited from linux-mod.eclass and then check if the kernel is configured
 # to support the options specified in CONFIG_CHECK (if not null)
 linux-info_pkg_setup() {
+	use kernel_linux || return
+
 	linux-info_get_any_version
 
 	if kernel_is 2 4; then
-- 
2.16.2

[gentoo-dev] [PATCH 7/8] linux-info.eclass: Die in most of public-ish APIs on non-Linux

[Michał Górny]

Add appropriate 'die' calls in most of the seemingly public APIs
of the eclass that could be called by ebuilds and that are going to fail
horribly when used on non-Linux systems. This means that
e.g. 'kernel_is' calls need to be explicitly guarded in ebuilds, as we
can't really reasonably return 'true' or 'false' if there is no Linux
kernel in the first place.
---
 eclass/linux-info.eclass | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 0ba7ff7755c7..554de81d3acc 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -240,6 +240,10 @@ linux_config_qa_check() {
 		ewarn "QA: You called $f before any linux_config_exists!"
 		ewarn "QA: The return value of $f will NOT guaranteed later!"
 	fi
+
+	if ! use kernel_linux; then
+		die "$f called on non-Linux system, please fix the ebuild"
+	fi
 }
 
 # @FUNCTION: linux_config_src_exists
@@ -290,6 +294,10 @@ linux_config_path() {
 # This function verifies that the current kernel is configured (it checks against the existence of .config)
 # otherwise it dies.
 require_configured_kernel() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	if ! linux_config_src_exists; then
 		qeerror "Could not find a usable .config in the kernel source directory."
 		qeerror "Please ensure that ${KERNEL_DIR} points to a configured set of Linux sources."
@@ -369,6 +377,10 @@ linux_chkconfig_string() {
 
 # Note: duplicated in kernel-2.eclass
 kernel_is() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	# if we haven't determined the version yet, we need to.
 	linux-info_get_any_version
 
@@ -439,6 +451,10 @@ get_version_warning_done=
 # KBUILD_OUTPUT (in a decreasing priority list, we look for the env var, makefile var or the
 # symlink /lib/modules/${KV_MAJOR}.${KV_MINOR}.${KV_PATCH}${KV_EXTRA}/build).
 get_version() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	local tmplocal
 
 	# no need to execute this twice assuming KV_FULL is populated.
@@ -592,6 +608,10 @@ get_version() {
 # It gets the version of the current running kernel and the result is the same as get_version() if the
 # function can find the sources.
 get_running_version() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	KV_FULL=$(uname -r)
 
 	if [[ -f ${ROOT}/lib/modules/${KV_FULL}/source/Makefile && -f ${ROOT}/lib/modules/${KV_FULL}/build/Makefile ]]; then
@@ -631,6 +651,10 @@ get_running_version() {
 # This attempts to find the version of the sources, and otherwise falls back to
 # the version of the running kernel.
 linux-info_get_any_version() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	if ! get_version; then
 		ewarn "Unable to calculate Linux Kernel version for build, attempting to use running version"
 		if ! get_running_version; then
@@ -647,6 +671,10 @@ linux-info_get_any_version() {
 # @DESCRIPTION:
 # This function verifies that the current kernel sources have been already prepared otherwise it dies.
 check_kernel_built() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	# if we haven't determined the version yet, we need to
 	require_configured_kernel
 
@@ -676,6 +704,10 @@ check_kernel_built() {
 # @DESCRIPTION:
 # This function verifies that the current kernel support modules (it checks CONFIG_MODULES=y) otherwise it dies.
 check_modules_supported() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	# if we haven't determined the version yet, we need too.
 	require_configured_kernel
 
@@ -832,6 +864,10 @@ check_extra_config() {
 }
 
 check_zlibinflate() {
+	if ! use kernel_linux; then
+		die "${FUNCNAME}() called on non-Linux system, please fix the ebuild"
+	fi
+
 	# if we haven't determined the version yet, we need to
 	require_configured_kernel
 
-- 
2.16.2

[gentoo-dev] [PATCH 8/8] linux-info.eclass: Skip linux_config_*_exists on non-Linux

[Michał Górny]

The linux_config_src_exists and linux_config_bin_exists always return
false on non-Linux systems by design. Short-circuit it via
'kernel_linux' check.
---
 eclass/linux-info.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 554de81d3acc..8ac290979247 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -252,7 +252,7 @@ linux_config_qa_check() {
 # It returns true if .config exists in a build directory otherwise false
 linux_config_src_exists() {
 	export _LINUX_CONFIG_EXISTS_DONE=1
-	[[ -n ${KV_OUT_DIR} && -s ${KV_OUT_DIR}/.config ]]
+	use kernel_linux && [[ -n ${KV_OUT_DIR} && -s ${KV_OUT_DIR}/.config ]]
 }
 
 # @FUNCTION: linux_config_bin_exists
@@ -261,7 +261,7 @@ linux_config_src_exists() {
 # It returns true if .config exists in /proc, otherwise false
 linux_config_bin_exists() {
 	export _LINUX_CONFIG_EXISTS_DONE=1
-	[[ -s /proc/config.gz ]]
+	use kernel_linux && [[ -s /proc/config.gz ]]
 }
 
 # @FUNCTION: linux_config_exists
-- 
2.16.2

Re: [gentoo-dev] [PATCH 3/8] linux-info.eclass: linux-info_get_any_version, die on failure

[Robin H. Johnson]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

On Thu, Mar 08, 2018 at 06:05:43PM +0100, Michał Górny wrote:
> Make linux-info_get_any_version die if it can't determine any version
> of the Linux kernel. This indicates a problem with the eclass code
> (as it should not happen on Linux) and the missing KV_* variables
> are going to cause random misbehavior and failures.
This change worries me a bit. get_running_version calls get_version in
some cases, and there are corner cases there which could trip this up.

linux-info_get_any_version -> get_running_version -> get_version
with get_version returning non-zero.

get_version has ALREADY returned non-zero once in that path, and we're
potentially tweaking KERNEL_DIR/KBUILD_OUTPUT before calling it again.

Most of the breakage cases IIRC are where
/lib/modules/${KV_FULL}/*/Makefile still exist, but point to kernel
sources in weird places that are broken.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 1113 bytes --]

Re: [gentoo-dev] [PATCH 3/8] linux-info.eclass: linux-info_get_any_version, die on failure

[Michał Górny]

W dniu sob, 10.03.2018 o godzinie 01∶05 +0000, użytkownik Robin H.
Johnson napisał:
> On Thu, Mar 08, 2018 at 06:05:43PM +0100, Michał Górny wrote:
> > Make linux-info_get_any_version die if it can't determine any version
> > of the Linux kernel. This indicates a problem with the eclass code
> > (as it should not happen on Linux) and the missing KV_* variables
> > are going to cause random misbehavior and failures.
> 
> This change worries me a bit. get_running_version calls get_version in
> some cases, and there are corner cases there which could trip this up.
> 
> linux-info_get_any_version -> get_running_version -> get_version
> with get_version returning non-zero.
> 
> get_version has ALREADY returned non-zero once in that path, and we're
> potentially tweaking KERNEL_DIR/KBUILD_OUTPUT before calling it again.
> 
> Most of the breakage cases IIRC are where
> /lib/modules/${KV_FULL}/*/Makefile still exist, but point to kernel
> sources in weird places that are broken.

Yes, and this needs to be fixed one way or another. Otherwise you don't
get KV_* exported and a lot of other logic trips on unset variables.

-- 
Best regards,
Michał Górny