[patch 2/2] kbuild: fix crossbuild base config

[patch 2/2] kbuild: fix crossbuild base config

[blaisorblade_spam]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade_spam@yahoo.it>
Cc: Julian Scheid <julian@sektor37.de>, Michael Richardson <mcr@sandelman.ottawa.on.ca>, Sam Ravnborg <sam@ravnborg.org>

When we are doing "make %config" on a normal build, we search a file to read
default settings from: we take the 1st existing one in a list specified in
scripts/kconfig/confdata.c. This list includes also /boot/config-$VER,
/lib/modules/$VER/.config, which is a wrong choice when we are
cross-compiling. So when cross-compiling we should ignore most files except
.config and arch/$ARCH/defconfig.

This has actually created not-working UML binaries (since UML is always
"cross-compiled" for this purpose), as reported by Julian Scheid.

We all agreed on this kind of general, not UML-only fix, and I (Paolo)
implemented it.

Implementation notes:
- I used a environment var, instead of adding yet another option, because it
  was the least intrusive way. Adding a new option and parsing it in all
  configurators, even the one not yet accepting command line option, was very
  intrusive.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade_spam@yahoo.it>
---

 vanilla-linux-2.6.9-paolo/scripts/kconfig/Makefile   |    8 ++++++++
 vanilla-linux-2.6.9-paolo/scripts/kconfig/confdata.c |   19 +++++++++++++++++--
 vanilla-linux-2.6.9-paolo/scripts/kconfig/lkc.h      |    1 -
 3 files changed, 25 insertions(+), 3 deletions(-)

diff -puN scripts/kconfig/confdata.c~kbuild-fix-crossbuild-defconfig scripts/kconfig/confdata.c
--- vanilla-linux-2.6.9/scripts/kconfig/confdata.c~kbuild-fix-crossbuild-defconfig	2004-11-03 00:18:27.359070256 +0100
+++ vanilla-linux-2.6.9-paolo/scripts/kconfig/confdata.c	2004-11-03 00:18:27.365069344 +0100
@@ -18,7 +18,9 @@ const char conf_def_filename[] = ".confi
 
 const char conf_defname[] = "arch/$ARCH/defconfig";
 
-const char *conf_confnames[] = {
+/*These are used only when conf_read(NULL) is called, i.e. when doing
+ * make config */
+static const char *__conf_confnames[] = {
 	".config",
 	"/lib/modules/$UNAME_RELEASE/.config",
 	"/etc/kernel-config",
@@ -27,6 +29,19 @@ const char *conf_confnames[] = {
 	NULL,
 };
 
+/* If we are doing a cross-compilation, using the host system config is
+ * dangerous (silently broken binaries can be the result).*/
+static const char *__cross_conf_confnames[] = {
+	".config",
+	conf_defname,
+	NULL,
+};
+
+static inline const char **get_conf_confnames(void)
+{
+	return getenv("__KBUILD_CROSS_COMPILING") ? __cross_conf_confnames: __conf_confnames;
+}
+
 static char *conf_expand_value(const signed char *in)
 {
 	struct symbol *sym;
@@ -83,7 +98,7 @@ int conf_read(const char *name)
 	if (name) {
 		in = zconf_fopen(name);
 	} else {
-		const char **names = conf_confnames;
+		const char **names = get_conf_confnames();
 		while ((name = *names++)) {
 			name = conf_expand_value(name);
 			in = zconf_fopen(name);
diff -puN scripts/kconfig/lkc.h~kbuild-fix-crossbuild-defconfig scripts/kconfig/lkc.h
--- vanilla-linux-2.6.9/scripts/kconfig/lkc.h~kbuild-fix-crossbuild-defconfig	2004-11-03 00:18:27.361069952 +0100
+++ vanilla-linux-2.6.9-paolo/scripts/kconfig/lkc.h	2004-11-03 00:18:27.365069344 +0100
@@ -36,7 +36,6 @@ char *zconf_curname(void);
 
 /* confdata.c */
 extern const char conf_def_filename[];
-extern char conf_filename[];
 
 char *conf_get_default_confname(void);
 
diff -puN scripts/kconfig/Makefile~kbuild-fix-crossbuild-defconfig scripts/kconfig/Makefile
--- vanilla-linux-2.6.9/scripts/kconfig/Makefile~kbuild-fix-crossbuild-defconfig	2004-11-03 00:18:27.362069800 +0100
+++ vanilla-linux-2.6.9-paolo/scripts/kconfig/Makefile	2004-11-03 00:18:27.365069344 +0100
@@ -4,6 +4,14 @@
 
 .PHONY: oldconfig xconfig gconfig menuconfig config silentoldconfig
 
+ifneq ($(SUBARCH),$(ARCH))
+  # This is used in scripts/kconfig/confdata.c
+  export __KBUILD_CROSS_COMPILING := 1
+else
+  #Let's prevent interferences from the environment.
+  unexport __KBUILD_CROSS_COMPILING
+endif
+
 xconfig: $(obj)/qconf
 	$< arch/$(ARCH)/Kconfig
 
_

Re: [patch 2/2] kbuild: fix crossbuild base config

[Roman Zippel]

Hi,

On Wed, 3 Nov 2004 blaisorblade_spam@yahoo.it wrote:

> This has actually created not-working UML binaries (since UML is always
> "cross-compiled" for this purpose), as reported by Julian Scheid.

This rather suggests, there is a problem with UML. Either fix your Kconfig 
to prevent nonvalid configurations or detect and report the problem at 
runtime.

> We all agreed on this kind of general, not UML-only fix, and I (Paolo)
> implemented it.

I don't like the two separate lists, it would be easier to just skip all 
absolute path names.
I would also like to avoid this patch at all. If this really should be a 
problem, I'd consider to don't run kconfig at all in this case if there 
is no configuration and instead suggest running defconfig (or one of 
machine specific config targets) first.

bye, Roman

Re: [patch 2/2] kbuild: fix crossbuild base config

[Tom Rini]

On Wed, Nov 03, 2004 at 05:56:46PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Wed, 3 Nov 2004 blaisorblade_spam@yahoo.it wrote:
> 
> > This has actually created not-working UML binaries (since UML is always
> > "cross-compiled" for this purpose), as reported by Julian Scheid.
> 
> This rather suggests, there is a problem with UML. Either fix your Kconfig 
> to prevent nonvalid configurations or detect and report the problem at 
> runtime.

No, this is a damn annoying kbuild problem when cross compiling.  It's
just nice that the UML folks run into this too and found a better fix
than deleting the /boot and /lib files from the list.

> > We all agreed on this kind of general, not UML-only fix, and I (Paolo)
> > implemented it.
> 
> I don't like the two separate lists, it would be easier to just skip all 
> absolute path names.
> I would also like to avoid this patch at all. If this really should be a 
> problem, I'd consider to don't run kconfig at all in this case if there 
> is no configuration and instead suggest running defconfig (or one of 
> machine specific config targets) first.

I have a feeling that changing the behavior of 'make {,x,g,q}config' to
fail if there's no .config will upset a lot of users, possibly even more
than would be upset by never looking in /boot or /lib ever.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [patch 2/2] kbuild: fix crossbuild base config

[Roman Zippel]

Hi,

On Wed, 3 Nov 2004, Tom Rini wrote:

> > > This has actually created not-working UML binaries (since UML is always
> > > "cross-compiled" for this purpose), as reported by Julian Scheid.
> > 
> > This rather suggests, there is a problem with UML. Either fix your Kconfig 
> > to prevent nonvalid configurations or detect and report the problem at 
> > runtime.
> 
> No, this is a damn annoying kbuild problem when cross compiling.

The ability to create a nonworkable UML binary is _not_ a kbuild problem, 
especially in the UML case I would expect it should be possible to avoid 
this.

> > > We all agreed on this kind of general, not UML-only fix, and I (Paolo)
> > > implemented it.
> > 
> > I don't like the two separate lists, it would be easier to just skip all 
> > absolute path names.
> > I would also like to avoid this patch at all. If this really should be a 
> > problem, I'd consider to don't run kconfig at all in this case if there 
> > is no configuration and instead suggest running defconfig (or one of 
> > machine specific config targets) first.
> 
> I have a feeling that changing the behavior of 'make {,x,g,q}config' to
> fail if there's no .config will upset a lot of users, possibly even more
> than would be upset by never looking in /boot or /lib ever.

I'm only talking about cross compiling here. From people who do this, I 
sort of expect, that they know what they do. You can misconfigure a kernel 
in native compiles as well, this patch solves the wrong problem. 
E.g. if someone wrote a patch which stores the arch in .config and warns/ 
refuses to load it for a different configuration, I would accept it 
happily.

bye, Roman

Re: [patch 2/2] kbuild: fix crossbuild base config

[Tom Rini]

On Wed, Nov 03, 2004 at 07:19:37PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Wed, 3 Nov 2004, Tom Rini wrote:
> 
> > > > This has actually created not-working UML binaries (since UML is always
> > > > "cross-compiled" for this purpose), as reported by Julian Scheid.
> > > 
> > > This rather suggests, there is a problem with UML. Either fix your Kconfig 
> > > to prevent nonvalid configurations or detect and report the problem at 
> > > runtime.
> > 
> > No, this is a damn annoying kbuild problem when cross compiling.
> 
> The ability to create a nonworkable UML binary is _not_ a kbuild problem, 
> especially in the UML case I would expect it should be possible to avoid 
> this.

How about how easy it is to create a totally bogus config for any arch?
This isn't a UML problem, this is a cross compiling for any arch
problem.

> > > > We all agreed on this kind of general, not UML-only fix, and I (Paolo)
> > > > implemented it.
> > > 
> > > I don't like the two separate lists, it would be easier to just skip all 
> > > absolute path names.
> > > I would also like to avoid this patch at all. If this really should be a 
> > > problem, I'd consider to don't run kconfig at all in this case if there 
> > > is no configuration and instead suggest running defconfig (or one of 
> > > machine specific config targets) first.
> > 
> > I have a feeling that changing the behavior of 'make {,x,g,q}config' to
> > fail if there's no .config will upset a lot of users, possibly even more
> > than would be upset by never looking in /boot or /lib ever.
> 
> I'm only talking about cross compiling here. From people who do this, I 
> sort of expect, that they know what they do.

The following argument makes less sense, possibly, as 2.6 lives on, but
this is new breakage for people moving up from 2.4 that just looked in
.config and arch/$(ARCH)/defconfig and who don't otherwise know that
things have been changed or broken, depending on how you look at it.

> You can misconfigure a kernel 
> in native compiles as well, this patch solves the wrong problem. 

I disagree.  This solves the "why did the kernel decide to look at
/boot/config when it really should have known better" problem.

> E.g. if someone wrote a patch which stores the arch in .config and warns/ 
> refuses to load it for a different configuration, I would accept it 
> happily.

We already have part of this, except I don't know for certain of
CONFIG_ARCH == CONFIG_$(SUBARCH) (... to mix syntax all the hell up).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [patch 2/2] kbuild: fix crossbuild base config

[Blaisorblade]

On Wednesday 03 November 2004 19:34, Tom Rini wrote:
> On Wed, Nov 03, 2004 at 07:19:37PM +0100, Roman Zippel wrote:
> > Hi,
> >
> > On Wed, 3 Nov 2004, Tom Rini wrote:
> > > > > This has actually created not-working UML binaries (since UML is
> > > > > always "cross-compiled" for this purpose), as reported by Julian
> > > > > Scheid.

> > > > This rather suggests, there is a problem with UML. Either fix your
> > > > Kconfig to prevent nonvalid configurations or detect and report the
> > > > problem at runtime.

> > > No, this is a damn annoying kbuild problem when cross compiling.

> > The ability to create a nonworkable UML binary is _not_ a kbuild problem,
> > especially in the UML case I would expect it should be possible to avoid
> > this.

I agree that this is a UML problem and I'm addressing it, too (it maybe wasn't 
clear in the original mail, but it's obvious). And maybe, it is even 
unrelated from this issue.

That said, indipendently from the UML issue, it is *meaningless* to read a 
config default from /boot or /etc or /lib/modules. And, in fact, you never 
say "it's a good thing to do in that case".

You later say "If possible, I'd avoid this patch at all". Why? Is this code 
too intrusive, or implementing a wrong check, or bloating the source?

> How about how easy it is to create a totally bogus config for any arch?
> This isn't a UML problem, this is a cross compiling for any arch
> problem.

> > > > > We all agreed on this kind of general, not UML-only fix, and I
> > > > > (Paolo) implemented it.

> > > > I don't like the two separate lists, it would be easier to just skip
> > > > all absolute path names.
Ok, fine. I was dubious on this, too (I went the lazy way because it's just 3 
duplicated lines).

Would it be ok for this to use:

struct confsource {
 char * path;
 int * cross_valid;
}

?

Since you didn't comment on this, I also suppose the env.var idea is fine for 
you.

> > > > I would also like to avoid this patch at all.

> > > > If this really should 
> > > > be a problem, I'd consider to don't run kconfig at all in this case
> > > > if there is no configuration and instead suggest running defconfig
> > > > (or one of machine specific config targets) first.

When there is no .config and no other source, the last resort is taking the 
defaults from defconfig, normally, isn't it? So, when cross-compiling, that 
should still be correct.

> > > I have a feeling that changing the behavior of 'make {,x,g,q}config' to
> > > fail if there's no .config will upset a lot of users, possibly even
> > > more than would be upset by never looking in /boot or /lib ever.

> > I'm only talking about cross compiling here. From people who do this, I
> > sort of expect, that they know what they do.

* UML is always cross-compiled (even if the compiler is the host one), and 
most UML user *don't* know what they are doing.

* Looking in /boot, /etc or /lib is not documented at all, and "read the 
flaming source" is not a welcome documentation source.

> The following argument makes less sense, possibly, as 2.6 lives on, but
> this is new breakage for people moving up from 2.4 that just looked in
> .config and arch/$(ARCH)/defconfig and who don't otherwise know that
> things have been changed or broken, depending on how you look at it.

> > You can misconfigure a kernel
> > in native compiles as well, this patch solves the wrong problem.

> I disagree.  This solves the "why did the kernel decide to look at
> /boot/config when it really should have known better" problem.

> > E.g. if someone wrote a patch which stores the arch in .config and warns/
> > refuses to load it for a different configuration, I would accept it
> > happily.
Yes, this is another idea, which is also fine, while not excluding the other 
IMHO.
> We already have part of this, except I don't know for certain of
> CONFIG_ARCH == CONFIG_$(SUBARCH) (... to mix syntax all the hell up).

No warning is output. Or better, yes, you get warnings, but tons of not clear 
ones, like "warning, undefined symbol".

UML uses currently CONFIG_USERMODE rather than CONFIG_UM (is this a bug?). I 
have no idea for other archs. Do you suggest using KCONFIG_ARCH= or 
CONFIG_THIS_ARCH= or what?
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

Re: [patch 2/2] kbuild: fix crossbuild base config

[Roman Zippel]

Hi,

On Wed, 3 Nov 2004, Tom Rini wrote:

> How about how easy it is to create a totally bogus config for any arch?

I'm open to suggestions, but this patch doesn't solve the problem.

> > You can misconfigure a kernel 
> > in native compiles as well, this patch solves the wrong problem. 
> 
> I disagree.  This solves the "why did the kernel decide to look at
> /boot/config when it really should have known better" problem.

In a lot of other cases it does make sense. The kconfig core shouldn't 
behave differently if we are cross compiling.

bye, Roman

Re: [patch 2/2] kbuild: fix crossbuild base config

[Roman Zippel]

Hi,

On Wed, 3 Nov 2004, Blaisorblade wrote:

> You later say "If possible, I'd avoid this patch at all". Why? Is this code 
> too intrusive, or implementing a wrong check, or bloating the source?

It adds a special case to the kconfig core to make it behave differently, 
but it shouldn't behave differently depending on how the kernel is 
compiled.

> > > E.g. if someone wrote a patch which stores the arch in .config and warns/
> > > refuses to load it for a different configuration, I would accept it
> > > happily.
> Yes, this is another idea, which is also fine, while not excluding the other 
> IMHO.

This is the better solution, because it solves the more general problem, 
when a .config doesn't match the Kconfig and not just your special case.

> > We already have part of this, except I don't know for certain of
> > CONFIG_ARCH == CONFIG_$(SUBARCH) (... to mix syntax all the hell up).
> 
> No warning is output. Or better, yes, you get warnings, but tons of not clear 
> ones, like "warning, undefined symbol".

I don't really expect to use CONFIG_$(SUBARCH) and rather add a real 
CONFIG_ARCH to Kconfig.

bye, Roman