[gentoo-kernel] linux-mod / linux-info changes

[gentoo-kernel] linux-mod / linux-info changes

[Daniel Drake]

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

Hi,

I just went over the linux-mod and linux-info stuff, we had at least had some 
preliminary discussion on this beforehand but the stuff that went into portage 
isn't quite how I would like it.

Firstly there isn't enough error checking in the version getting code. 
Especially after a recent bug in the kernel-2 eclass where old sources got 
partially removed, some people have broken kernel source trees lying around 
and do not get a useful error message from linux-info when it can't parse the 
version info.

Secondly, whilst getfilevar looks good, its interface looks quite poor. We 
should be trying to simplify things for ebuilds, not worrying them that the 
config file is called ".config" and is located under $KV_OUT_DIR and not 
$KV_DIR .. (and think about what would happen if .config was renamed to 
something else..)
So I replaced those functions, and they can be used from ebuilds like:
	linux_chkconfig_builtin MTRR
which will check that CONFIG_MTRR=y

If there are no objections then I'll commit this and fix up the ebuilds 
sometime soon (do we have any users other than nvidia-kernel and alsa-driver?)

Daniel

[-- Attachment #2: eclass-diff --]
[-- Type: text/plain, Size: 4518 bytes --]

Index: linux-info.eclass
===================================================================
RCS file: /var/cvsroot/gentoo-x86/eclass/linux-info.eclass,v
retrieving revision 1.6
diff -u -b -B -r1.6 linux-info.eclass
--- linux-info.eclass	1 Dec 2004 23:26:43 -0000	1.6
+++ linux-info.eclass	4 Dec 2004 21:46:01 -0000
@@ -47,24 +47,28 @@
 	fi
 }
 
-getfilevar_isset() {
+linux_chkconfig_present() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "m" -o "${RESULT}" = "y" ] && return 0 || return 1
 }
 
-getfilevar_ismodule() {
+linux_chkconfig_module() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "m" ] && return 0 || return 1
 }
 
-getfilevar_isbuiltin() {
+linux_chkconfig_builtin() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "y" ] && return 0 || return 1
 }
 
+linux_chkconfig_string() {
+	getfilevar "CONFIG_${1}" "${KV_OUT_DIR}/.config"
+}
+
 # Versioning Functions
 # ---------------------------------------
 
@@ -124,7 +128,32 @@
 	if [ -z "${KV_DIR}" ]
 	then
 		eerror "Unable to find kernel sources at ${KERNEL_DIR}"
-		die
+		einfo "This package requires Linux sources."
+		if [ "${KERNEL_DIR}" == "/usr/src/linux" ] ; then
+			einfo "Please make sure that ${KERNEL_DIR} points at your running kernel, "
+			einfo "(or the kernel you wish to build against)."
+			einfo "Alternatively, set the KERNEL_DIR environment variable to the kernel sources location"
+		else
+			einfo "Please ensure that the KERNEL_DIR environment variable points at full Linux sources of the kernel you wish to compile against."
+		fi
+		die "Cannot locate Linux sources at ${KERNEL_DIR}"
+	fi
+
+	einfo "Found kernel source directory:"
+	einfo "    ${KV_DIR}"
+
+	if [ ! -s "${KV_DIR}/Makefile" ]
+	then
+		eerror "Could not find a Makefile in the kernel source directory."
+		einfo "Please ensure that ${KERNEL_DIR} points to a complete set of Linux sources"
+		die "Makefile not found in ${KV_DIR}"
+	fi
+
+	if [ ! -s "${KV_DIR}/.config" ]
+	then
+		eerror "Could not find a usable .config in the kernel source directory."
+		einfo "Please ensure that ${KERNEL_DIR} points to a user-configured set of Linux sources"
+		die ".config not found in ${KV_DIR}"
 	fi
 	
 	# OK so now we know our sources directory, but they might be using
@@ -145,6 +174,13 @@
 	KV_PATCH="$(getfilevar SUBLEVEL ${KV_DIR}/Makefile)"
 	KV_EXTRA="$(getfilevar EXTRAVERSION ${KV_DIR}/Makefile)"
 	
+	if [ -z "${KV_MAJOR}" -o -z "${KV_MINOR}" -o -z "${KV_PATCH}" ]
+	then
+		eerror "Could not detect kernel version."
+		einfo "Please ensure that ${KERNEL_DIR} points to a complete set of Linux sources"
+		die "Could not parse version info from ${KV_DIR}/Makefile"
+	fi
+	
 	# and in newer versions we can also pull LOCALVERSION if it is set.
 	# but before we do this, we need to find if we use a different object directory.
 	# This *WILL* break if the user is using localversions, but we assume it was
@@ -165,22 +201,13 @@
 	KV_OUT_DIR="${KV_OUT_DIR:-${KV_DIR}}"
 	
 	KV_LOCAL="${KV_LOCAL}$(cat ${KV_DIR}/localversion* 2>/dev/null)"
-	KV_LOCAL="${KV_LOCAL}$(getfilevar CONFIG_LOCALVERSION ${KV_OUT_DIR}/.config | sed 's:"::g')"
+	KV_LOCAL="${KV_LOCAL}$(linux_chkconfig_string LOCALVERSION | sed 's:"::g')"
 	
 	# And we should set KV_FULL to the full expanded version
 	KV_FULL="${KV_MAJOR}.${KV_MINOR}.${KV_PATCH}${KV_EXTRA}${KV_LOCAL}"
 	
-	if [ -z "${KV_FULL}" ]
-	then
-		eerror "We are unable to find a usable kernel source tree in ${KV_DIR}"
-		eerror "Please check a kernel source exists in this directory."
-		die
-	else
-		einfo "Found kernel source directory:"
-		einfo "    ${KV_DIR}"
-		einfo "with sources for kernel version:"
+	einfo "Found sources for kernel version:"
 		einfo "    ${KV_FULL}"
-	fi
 }
 
 
@@ -212,13 +239,12 @@
 	# if we haven't determined the version yet, we need too.
 	get_version;
 	
-	getfilevar_isset CONFIG_MODULES ${KV_OUT_DIR}/.config
-	if [ "$?" != 0 ]
+	if ! linux_chkconfig_builtin "MODULES"
 	then
 		eerror "These sources do not support loading external modules."
 		eerror "to be able to use this module please enable \"Loadable modules support\""
 		eerror "in your kernel, recompile and then try merging this module again."
-		die No support for external modules in ${KV_FULL} config
+		die "No support for external modules in ${KV_FULL} config"
 	fi
 }
 


[-- Attachment #3: Type: text/plain, Size: 40 bytes --]

--
gentoo-kernel@gentoo.org mailing list

Re: [gentoo-kernel] linux-mod / linux-info changes

[Sam Ravnborg]

On Sat, Dec 04, 2004 at 11:54:28PM +0000, Daniel Drake wrote:
> +		einfo "This package requires Linux sources."
> +		if [ "${KERNEL_DIR}" == "/usr/src/linux" ] ; then
> +			einfo "Please make sure that ${KERNEL_DIR} points at your running kernel, "
> +			einfo "(or the kernel you wish to build against)."
> +			einfo "Alternatively, set the KERNEL_DIR environment variable to the kernel sources location"
> +		else
> +			einfo "Please ensure that the KERNEL_DIR environment variable points at full Linux sources of the kernel you wish to compile against."
> +		fi
> +		die "Cannot locate Linux sources at ${KERNEL_DIR}"

Why not use /lib/modules/`uname -r`/source - it works for 2.6 at least.
It could be your first try - second /usr/src/linux/...

	Sam

--
gentoo-kernel@gentoo.org mailing list

Re: [gentoo-kernel] linux-mod / linux-info changes

[John Mylchreest]

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

this is against kernel policy since we never should be building against
the current running kernel.
we should be building against what we dictate via KERNEL_DIR.
think critical modules which need to run at the next boot of a new
kernel.

On Sat, 2004-12-04 at 23:51 +0100, Sam Ravnborg wrote:
> On Sat, Dec 04, 2004 at 11:54:28PM +0000, Daniel Drake wrote:
> > +		einfo "This package requires Linux sources."
> > +		if [ "${KERNEL_DIR}" == "/usr/src/linux" ] ; then
> > +			einfo "Please make sure that ${KERNEL_DIR} points at your running kernel, "
> > +			einfo "(or the kernel you wish to build against)."
> > +			einfo "Alternatively, set the KERNEL_DIR environment variable to the kernel sources location"
> > +		else
> > +			einfo "Please ensure that the KERNEL_DIR environment variable points at full Linux sources of the kernel you wish to compile against."
> > +		fi
> > +		die "Cannot locate Linux sources at ${KERNEL_DIR}"
> 
> Why not use /lib/modules/`uname -r`/source - it works for 2.6 at least.
> It could be your first try - second /usr/src/linux/...
> 
> 	Sam
> 
> --
> gentoo-kernel@gentoo.org mailing list
> 
-- 
John Mylchreest

Role:            Gentoo Linux Kernel Lead
Gentoo Linux:    http://www.gentoo.org
Public Key:      gpg --recv-keys 0xEAB9E721              
Key fingerprint: 0670 E5E4 F461 806B 860A  2245 A40E 72EB EAB9 E721
Web:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xEAB9E721



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-kernel] linux-mod / linux-info changes

[Sam Ravnborg]

On Sat, Dec 04, 2004 at 10:53:28PM +0000, John Mylchreest wrote:
> this is against kernel policy since we never should be building against
> the current running kernel.
> we should be building against what we dictate via KERNEL_DIR.
> think critical modules which need to run at the next boot of a new
> kernel.

OK, that explains

> > > +			einfo "(or the kernel you wish to build against)."

	Sam

--
gentoo-kernel@gentoo.org mailing list

Re: [gentoo-kernel] linux-mod / linux-info changes

[Daniel Drake]

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

Forgot to fix up check_extra_config, lets try again..


[-- Attachment #2: eclass-diff --]
[-- Type: text/plain, Size: 5522 bytes --]

Index: linux-info.eclass
===================================================================
RCS file: /var/cvsroot/gentoo-x86/eclass/linux-info.eclass,v
retrieving revision 1.6
diff -u -b -B -r1.6 linux-info.eclass
--- linux-info.eclass	1 Dec 2004 23:26:43 -0000	1.6
+++ linux-info.eclass	4 Dec 2004 22:04:20 -0000
@@ -47,24 +47,28 @@
 	fi
 }
 
-getfilevar_isset() {
+linux_chkconfig_present() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "m" -o "${RESULT}" = "y" ] && return 0 || return 1
 }
 
-getfilevar_ismodule() {
+linux_chkconfig_module() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "m" ] && return 0 || return 1
 }
 
-getfilevar_isbuiltin() {
+linux_chkconfig_builtin() {
 local	RESULT
-	RESULT="$(getfilevar ${1} ${2})"
+	RESULT="$(getfilevar CONFIG_${1} ${KV_OUT_DIR}/.config)"
 	[ "${RESULT}" = "y" ] && return 0 || return 1
 }
 
+linux_chkconfig_string() {
+	getfilevar "CONFIG_${1}" "${KV_OUT_DIR}/.config"
+}
+
 # Versioning Functions
 # ---------------------------------------
 
@@ -124,7 +128,32 @@
 	if [ -z "${KV_DIR}" ]
 	then
 		eerror "Unable to find kernel sources at ${KERNEL_DIR}"
-		die
+		einfo "This package requires Linux sources."
+		if [ "${KERNEL_DIR}" == "/usr/src/linux" ] ; then
+			einfo "Please make sure that ${KERNEL_DIR} points at your running kernel, "
+			einfo "(or the kernel you wish to build against)."
+			einfo "Alternatively, set the KERNEL_DIR environment variable to the kernel sources location"
+		else
+			einfo "Please ensure that the KERNEL_DIR environment variable points at full Linux sources of the kernel you wish to compile against."
+		fi
+		die "Cannot locate Linux sources at ${KERNEL_DIR}"
+	fi
+
+	einfo "Found kernel source directory:"
+	einfo "    ${KV_DIR}"
+
+	if [ ! -s "${KV_DIR}/Makefile" ]
+	then
+		eerror "Could not find a Makefile in the kernel source directory."
+		einfo "Please ensure that ${KERNEL_DIR} points to a complete set of Linux sources"
+		die "Makefile not found in ${KV_DIR}"
+	fi
+
+	if [ ! -s "${KV_DIR}/.config" ]
+	then
+		eerror "Could not find a usable .config in the kernel source directory."
+		einfo "Please ensure that ${KERNEL_DIR} points to a user-configured set of Linux sources"
+		die ".config not found in ${KV_DIR}"
 	fi
 	
 	# OK so now we know our sources directory, but they might be using
@@ -145,6 +174,13 @@
 	KV_PATCH="$(getfilevar SUBLEVEL ${KV_DIR}/Makefile)"
 	KV_EXTRA="$(getfilevar EXTRAVERSION ${KV_DIR}/Makefile)"
 	
+	if [ -z "${KV_MAJOR}" -o -z "${KV_MINOR}" -o -z "${KV_PATCH}" ]
+	then
+		eerror "Could not detect kernel version."
+		einfo "Please ensure that ${KERNEL_DIR} points to a complete set of Linux sources"
+		die "Could not parse version info from ${KV_DIR}/Makefile"
+	fi
+	
 	# and in newer versions we can also pull LOCALVERSION if it is set.
 	# but before we do this, we need to find if we use a different object directory.
 	# This *WILL* break if the user is using localversions, but we assume it was
@@ -165,22 +201,13 @@
 	KV_OUT_DIR="${KV_OUT_DIR:-${KV_DIR}}"
 	
 	KV_LOCAL="${KV_LOCAL}$(cat ${KV_DIR}/localversion* 2>/dev/null)"
-	KV_LOCAL="${KV_LOCAL}$(getfilevar CONFIG_LOCALVERSION ${KV_OUT_DIR}/.config | sed 's:"::g')"
+	KV_LOCAL="${KV_LOCAL}$(linux_chkconfig_string LOCALVERSION | sed 's:"::g')"
 	
 	# And we should set KV_FULL to the full expanded version
 	KV_FULL="${KV_MAJOR}.${KV_MINOR}.${KV_PATCH}${KV_EXTRA}${KV_LOCAL}"
 	
-	if [ -z "${KV_FULL}" ]
-	then
-		eerror "We are unable to find a usable kernel source tree in ${KV_DIR}"
-		eerror "Please check a kernel source exists in this directory."
-		die
-	else
-		einfo "Found kernel source directory:"
-		einfo "    ${KV_DIR}"
-		einfo "with sources for kernel version:"
+	einfo "Found sources for kernel version:"
 		einfo "    ${KV_FULL}"
-	fi
 }
 
 
@@ -212,13 +239,12 @@
 	# if we haven't determined the version yet, we need too.
 	get_version;
 	
-	getfilevar_isset CONFIG_MODULES ${KV_OUT_DIR}/.config
-	if [ "$?" != 0 ]
+	if ! linux_chkconfig_builtin "MODULES"
 	then
 		eerror "These sources do not support loading external modules."
 		eerror "to be able to use this module please enable \"Loadable modules support\""
 		eerror "in your kernel, recompile and then try merging this module again."
-		die No support for external modules in ${KV_FULL} config
+		die "No support for external modules in ${KV_FULL} config"
 	fi
 }
 
@@ -235,15 +261,15 @@
 		if [ "${negate}" == "!" ];
 		then
 			config="${config:1}"
-			if getfilevar_isset ${config} ${KV_OUT_DIR}/.config ;
+			if linux_chkconfig_present ${config}
 			then
-				eerror "  ${config}:\tshould not be set in the kernel configuration, but it is."
+				eerror "  CONFIG_${config}:\tshould not be set in the kernel configuration, but it is."
 				error=1
 			fi
 		else
-			if ! getfilevar_isset ${config} ${KV_OUT_DIR}/.config ;
+			if ! linux_chkconfig_present ${config}
 			then
-				eerror "  ${config}:\tshould be set in the kernel configuration, but isn't"
+				eerror "  CONFIG_${config}:\tshould be set in the kernel configuration, but isn't"
 				error=1
 			fi
 		fi
@@ -254,7 +280,7 @@
 		eerror "Please check to make sure these options are set correctly."
 		eerror "Once you have satisfied these options, please try merging"
 		eerror "this package again."
-		die Incorrect kernel configuration options
+		die "Incorrect kernel configuration options"
 	fi
 }
 


[-- Attachment #3: Type: text/plain, Size: 40 bytes --]

--
gentoo-kernel@gentoo.org mailing list