public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] input: JOYSTICK_XPAD build fix
@ 2008-04-30 20:03 Ingo Molnar
  2008-04-30 21:02 ` Dmitry Torokhov
  0 siblings, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-04-30 20:03 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel

x86.git testing found the following build failure in -git:

 ERROR: "led_classdev_register" [drivers/input/joystick/xpad.ko] undefined!
 ERROR: "led_classdev_unregister" [drivers/input/joystick/xpad.ko] undefined!

which triggers with the following config:

 http://redhat.com/~mingo/misc/config-Wed_Apr_30_21_43_17_CEST_2008.bad

the reason is dependency on NEW_LEDS that was not spelled out in the
Kconfig entry of JOYSTICK_XPAD.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/input/joystick/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux/drivers/input/joystick/Kconfig
===================================================================
--- linux.orig/drivers/input/joystick/Kconfig
+++ linux/drivers/input/joystick/Kconfig
@@ -268,6 +268,7 @@ config JOYSTICK_JOYDUMP
 config JOYSTICK_XPAD
 	tristate "X-Box gamepad support"
 	depends on USB_ARCH_HAS_HCD
+	depends on NEW_LEDS
 	select USB
 	help
 	  Say Y here if you want to use the X-Box pad with your computer.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [patch] input: JOYSTICK_XPAD build fix
  2008-04-30 20:03 [patch] input: JOYSTICK_XPAD build fix Ingo Molnar
@ 2008-04-30 21:02 ` Dmitry Torokhov
  2008-04-30 21:13   ` Ingo Molnar
  0 siblings, 1 reply; 32+ messages in thread
From: Dmitry Torokhov @ 2008-04-30 21:02 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel

On Wed, Apr 30, 2008 at 10:03:40PM +0200, Ingo Molnar wrote:
> x86.git testing found the following build failure in -git:
> 
>  ERROR: "led_classdev_register" [drivers/input/joystick/xpad.ko] undefined!
>  ERROR: "led_classdev_unregister" [drivers/input/joystick/xpad.ko] undefined!
> 
> which triggers with the following config:
> 
>  http://redhat.com/~mingo/misc/config-Wed_Apr_30_21_43_17_CEST_2008.bad
> 
> the reason is dependency on NEW_LEDS that was not spelled out in the
> Kconfig entry of JOYSTICK_XPAD.
> 

Xpad can be compiled without LED support so this dependancy is
incorrect. JOYSTICK_XPAD_LEDS has proper dependancy on LEDS_CLASS, the
rest is Kconfig breakage.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [patch] input: JOYSTICK_XPAD build fix
  2008-04-30 21:02 ` Dmitry Torokhov
@ 2008-04-30 21:13   ` Ingo Molnar
  2008-04-30 23:01     ` Ingo, no more kconfig patches Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-04-30 21:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, Rafael J. Wysocki, Andrew Morton


* Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> > which triggers with the following config:
> > 
> >  http://redhat.com/~mingo/misc/config-Wed_Apr_30_21_43_17_CEST_2008.bad
> > 
> > the reason is dependency on NEW_LEDS that was not spelled out in the 
> > Kconfig entry of JOYSTICK_XPAD.
> 
> Xpad can be compiled without LED support so this dependancy is 
> incorrect. JOYSTICK_XPAD_LEDS has proper dependancy on LEDS_CLASS, the 
> rest is Kconfig breakage.

no, you are wrong, read the current Kconfig rules again. If the user can 
create a .config that does not build, it is driver breakage. It always 
was, and has been in the past 15 years.

Kconfig might be extended to make dependencies easier to manage for 
developers but until that is implemented you have to craft your driver's 
dependencies with the current tools in a way that doesnt break the 
build.

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Ingo, no more kconfig patches
  2008-04-30 21:13   ` Ingo Molnar
@ 2008-04-30 23:01     ` Adrian Bunk
  2008-05-01  1:17       ` Ingo Molnar
  2008-05-01  2:52       ` Ingo Molnar
  0 siblings, 2 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-04-30 23:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton

On Wed, Apr 30, 2008 at 11:13:17PM +0200, Ingo Molnar wrote:
> 
> * Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > > which triggers with the following config:
> > > 
> > >  http://redhat.com/~mingo/misc/config-Wed_Apr_30_21_43_17_CEST_2008.bad
> > > 
> > > the reason is dependency on NEW_LEDS that was not spelled out in the 
> > > Kconfig entry of JOYSTICK_XPAD.
> > 
> > Xpad can be compiled without LED support so this dependancy is 
> > incorrect. JOYSTICK_XPAD_LEDS has proper dependancy on LEDS_CLASS, the 
> > rest is Kconfig breakage.
> 
> no, you are wrong, read the current Kconfig rules again. If the user can 
> create a .config that does not build, it is driver breakage. It always 
> was, and has been in the past 15 years.
> 
> Kconfig might be extended to make dependencies easier to manage for 
> developers but until that is implemented you have to craft your driver's 
> dependencies with the current tools in a way that doesnt break the 
> build.

Ingo, there are areas in the kernel you know well.

But kconfig is not among them.

This breakage with your .config is *not* caused by an input bug as you 
wrongly claim (I'll send a correct fix after some testing).

And the commit that broke it even contains a
  Signed-off-by: Ingo Molnar <mingo@elte.hu>

I would really appreciate it if you could send the error message and the 
.config but not quick kconfig patches that are often wrong and that you 
try to push through the maintainers as you tried here.

Toralf, who does randconfig testing for some years, does exactly this, 
and the maintainers and/or I usually pick up his bug reports and work on
getting them fixed properly.

> 	Ingo

Thanks in advance
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-04-30 23:01     ` Ingo, no more kconfig patches Adrian Bunk
@ 2008-05-01  1:17       ` Ingo Molnar
  2008-05-01  1:37         ` Adrian Bunk
  2008-05-01  2:52       ` Ingo Molnar
  1 sibling, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-05-01  1:17 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Adrian Bunk <bunk@kernel.org> wrote:

> This breakage with your .config is *not* caused by an input bug as you 
> wrongly claim (I'll send a correct fix after some testing).

I have no opinion about where the bug is (input or leds or elsewhere) - 
my only opinion is that the kernel must not stay build-broken - and the 
discussion with Dmitry was about that.

I'm not interested in trivial patches as those issues are much more 
efficiently handled by the person who maintains that code (i.e. Dmitry) 
and who intimately knows the dependencies and expectations of that code. 
That's why i sent the bugreport and patch to Dmitry.

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-01  1:17       ` Ingo Molnar
@ 2008-05-01  1:37         ` Adrian Bunk
  2008-05-01  2:06           ` Ingo Molnar
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-01  1:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton

On Thu, May 01, 2008 at 03:17:03AM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@kernel.org> wrote:
> 
> > This breakage with your .config is *not* caused by an input bug as you 
> > wrongly claim (I'll send a correct fix after some testing).
> 
> I have no opinion about where the bug is (input or leds or elsewhere) - 
> my only opinion is that the kernel must not stay build-broken - and the 
> discussion with Dmitry was about that.
> 
> I'm not interested in trivial patches as those issues are much more 
> efficiently handled by the person who maintains that code (i.e. Dmitry) 
> and who intimately knows the dependencies and expectations of that code. 
> That's why i sent the bugreport and patch to Dmitry.

Why is Dmitry responsible for a bug introduced by a commit *you* 
Signed-off in a subsystem that lists *you* as the maintainer?

The bug is in arch/x86/Kconfig .

Caused by commit 4cf31841762954ad2868156ccba94d798a16630f
(x86: mach-rdc321x Kconfig fix).

That Dmitrys code broke was just a side effect of your bug.

Roman's patch to remove the need to select NEW_LEDS that just appeared 
in another thread will actually also fix your bug (and makes my idea 
to add a "select NEW_LEDS" to X86_RDC321X obsolete).

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-01  1:37         ` Adrian Bunk
@ 2008-05-01  2:06           ` Ingo Molnar
  2008-05-01  2:12             ` Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-05-01  2:06 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton,
	Florian Fainelli, Thomas Gleixner, H. Peter Anvin


* Adrian Bunk <bunk@kernel.org> wrote:

> The bug is in arch/x86/Kconfig .
> 
> Caused by commit 4cf31841762954ad2868156ccba94d798a16630f (x86: 
> mach-rdc321x Kconfig fix).

ah, indeed - Florian Cc:-ed. Preliminary quick hack below.

	Ingo

--------------->
Subject: rdc: leds hack
From: Ingo Molnar <mingo@elte.hu>
Date: Thu May 01 03:46:22 CEST 2008

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -322,6 +322,7 @@ config X86_RDC321X
 	select GENERIC_GPIO
 	select LEDS_CLASS
 	select LEDS_GPIO
+	select NEW_LEDS
 	help
 	  This option is needed for RDC R-321x system-on-chip, also known
 	  as R-8610-(G).

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-01  2:06           ` Ingo Molnar
@ 2008-05-01  2:12             ` Adrian Bunk
  0 siblings, 0 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-01  2:12 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton,
	Florian Fainelli, Thomas Gleixner, H. Peter Anvin

On Thu, May 01, 2008 at 04:06:45AM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@kernel.org> wrote:
> 
> > The bug is in arch/x86/Kconfig .
> > 
> > Caused by commit 4cf31841762954ad2868156ccba94d798a16630f (x86: 
> > mach-rdc321x Kconfig fix).
> 
> ah, indeed - Florian Cc:-ed. Preliminary quick hack below.

Why couldn't you *read* my email completely before sending yet another 
kconfig patch?

I said:
Roman's patch to remove the need to select NEW_LEDS that just appeared 
in another thread will actually also fix your bug (and makes my idea 
to add a "select NEW_LEDS" to X86_RDC321X obsolete).

> 	Ingo
> 
> --------------->
> Subject: rdc: leds hack
> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu May 01 03:46:22 CEST 2008
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/arch/x86/Kconfig
> ===================================================================
> --- linux.orig/arch/x86/Kconfig
> +++ linux/arch/x86/Kconfig
> @@ -322,6 +322,7 @@ config X86_RDC321X
>  	select GENERIC_GPIO
>  	select LEDS_CLASS
>  	select LEDS_GPIO
> +	select NEW_LEDS
>  	help
>  	  This option is needed for RDC R-321x system-on-chip, also known
>  	  as R-8610-(G).

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-04-30 23:01     ` Ingo, no more kconfig patches Adrian Bunk
  2008-05-01  1:17       ` Ingo Molnar
@ 2008-05-01  2:52       ` Ingo Molnar
  2008-05-01 11:59         ` Adrian Bunk
  1 sibling, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-05-01  2:52 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Adrian Bunk <bunk@kernel.org> wrote:

> I would really appreciate it if you could send the error message and 
> the .config but not quick kconfig patches that are often wrong and 
> that you try to push through the maintainers as you tried here.

hey, sorry about invading your turf of trivial patches ;) I dont see it 
as a problem that the thought process and the initial patch is 
incomplete and ad-hoc. My preference is to work with people out in the 
open, even on trivial issues. Dmitry is a capable maintainer who 
understands his code very well and he'll resist me if i'm full of it. 
Just like i resisted you when you were full of it. That's what 
maintainers do, their job is to know their code.

And, occasionally, as in this case, i might end up being faced with a 
bug in the code i maintain ;)

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-01  2:52       ` Ingo Molnar
@ 2008-05-01 11:59         ` Adrian Bunk
  2008-05-03 19:14           ` Ingo Molnar
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-01 11:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton

On Thu, May 01, 2008 at 04:52:34AM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@kernel.org> wrote:
> 
> > I would really appreciate it if you could send the error message and 
> > the .config but not quick kconfig patches that are often wrong and 
> > that you try to push through the maintainers as you tried here.
> 
> hey, sorry about invading your turf of trivial patches ;) I dont see it 
> as a problem that the thought process and the initial patch is 
> incomplete and ad-hoc. My preference is to work with people out in the 
> open, even on trivial issues. Dmitry is a capable maintainer who 
> understands his code very well and he'll resist me if i'm full of it. 
> Just like i resisted you when you were full of it. That's what 
> maintainers do, their job is to know their code.
> 
> And, occasionally, as in this case, i might end up being faced with a 
> bug in the code i maintain ;)

You completely miss my point.

You wrongly (and loudly) blamed Dmitry for something you broke yourself.

And if I hadn't stopped you pointing to what actually was broken you 
might have annoyed Dmitry until he'd have merged your wrong patch.

I don't claim any knowledge about scheduler or x86 internals, but I 
claim that I might be the person with the second-best understanding
of our kconfig files.

And I'd really prefer being able to take some time searching for a 
correct solution over having to quickly defeat the mess you create
as I had here.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-01 11:59         ` Adrian Bunk
@ 2008-05-03 19:14           ` Ingo Molnar
  2008-05-03 19:17             ` Ingo Molnar
  2008-05-03 20:24             ` Adrian Bunk
  0 siblings, 2 replies; 32+ messages in thread
From: Ingo Molnar @ 2008-05-03 19:14 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Adrian Bunk <bunk@stusta.de> wrote:

> On Thu, May 01, 2008 at 04:52:34AM +0200, Ingo Molnar wrote:
> > 
> > * Adrian Bunk <bunk@kernel.org> wrote:
> > 
> > > I would really appreciate it if you could send the error message 
> > > and the .config but not quick kconfig patches that are often wrong 
> > > and that you try to push through the maintainers as you tried 
> > > here.
> > 
> > hey, sorry about invading your turf of trivial patches ;) I dont see 
> > it as a problem that the thought process and the initial patch is 
> > incomplete and ad-hoc. My preference is to work with people out in 
> > the open, even on trivial issues. Dmitry is a capable maintainer who 
> > understands his code very well and he'll resist me if i'm full of 
> > it. Just like i resisted you when you were full of it. That's what 
> > maintainers do, their job is to know their code.
> > 
> > And, occasionally, as in this case, i might end up being faced with 
> > a bug in the code i maintain ;)
> 
> You completely miss my point.
> 
> You wrongly (and loudly) blamed Dmitry for something you broke 
> yourself.

i didnt. Read what i wrote:

|| no, you are wrong, read the current Kconfig rules again. If the user 
|| can create a .config that does not build, it is driver breakage. It 
|| always was, and has been in the past 15 years.
||
|| Kconfig might be extended to make dependencies easier to manage for 
|| developers but until that is implemented you have to craft your 
|| driver's dependencies with the current tools in a way that doesnt 
|| break the build.

and that's exactly what happens with Roman's patch: a Kconfig subsystem 
design bug (its inability to properly propagate the dependencies of 
select's) is worked around in the driver space: by the LEDS_CORE driver 
config introduction and no user-visible.

Roman's patch is obviously cleaner than my hack (i just fixed a single 
instantiation of the problem, while he changed the LEDS driver 
dependency structure), but it's still a workaround for a Kconfig 
subsystem bug and the same problem could reoccur elsewhere. It could hit 
anytime dual dependencies are introduced in a driver accidentally.

As Sam said it, fixing that Kconfig design bug would be "nice" - but 
unfortunately the Kconfig subsystem is not actively developed anymore. 
Would you like to volunteer for that? It would be a _very_ useful 
contribution. One such fix could avoid hundreds or even thousands of 
trivial problems in the future - it could avoid having to make hundreds 
or thousands of trivial patches in the future.

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 19:14           ` Ingo Molnar
@ 2008-05-03 19:17             ` Ingo Molnar
  2008-05-03 20:24             ` Adrian Bunk
  1 sibling, 0 replies; 32+ messages in thread
From: Ingo Molnar @ 2008-05-03 19:17 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Ingo Molnar <mingo@elte.hu> wrote:

> || Kconfig might be extended to make dependencies easier to manage for 
> || developers but until that is implemented you have to craft your 
> || driver's dependencies with the current tools in a way that doesnt 
> || break the build.
> 
> and that's exactly what happens with Roman's patch: a Kconfig 
> subsystem design bug (its inability to properly propagate the 
> dependencies of select's) is worked around in the driver space: by the 
> LEDS_CORE driver config introduction and no user-visible.
                                       breakage remains --^


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 19:14           ` Ingo Molnar
  2008-05-03 19:17             ` Ingo Molnar
@ 2008-05-03 20:24             ` Adrian Bunk
  2008-05-03 21:03               ` Ingo Molnar
  2008-05-03 21:17               ` Krzysztof Halasa
  1 sibling, 2 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 20:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton

On Sat, May 03, 2008 at 09:14:45PM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@stusta.de> wrote:
> 
> > On Thu, May 01, 2008 at 04:52:34AM +0200, Ingo Molnar wrote:
> > > 
> > > * Adrian Bunk <bunk@kernel.org> wrote:
> > > 
> > > > I would really appreciate it if you could send the error message 
> > > > and the .config but not quick kconfig patches that are often wrong 
> > > > and that you try to push through the maintainers as you tried 
> > > > here.
> > > 
> > > hey, sorry about invading your turf of trivial patches ;) I dont see 
> > > it as a problem that the thought process and the initial patch is 
> > > incomplete and ad-hoc. My preference is to work with people out in 
> > > the open, even on trivial issues. Dmitry is a capable maintainer who 
> > > understands his code very well and he'll resist me if i'm full of 
> > > it. Just like i resisted you when you were full of it. That's what 
> > > maintainers do, their job is to know their code.
> > > 
> > > And, occasionally, as in this case, i might end up being faced with 
> > > a bug in the code i maintain ;)
> > 
> > You completely miss my point.
> > 
> > You wrongly (and loudly) blamed Dmitry for something you broke 
> > yourself.
> 
> i didnt. Read what i wrote:
> 
> || no, you are wrong, read the current Kconfig rules again. If the user 
> || can create a .config that does not build, it is driver breakage. It 
> || always was, and has been in the past 15 years.
> ||
> || Kconfig might be extended to make dependencies easier to manage for 
> || developers but until that is implemented you have to craft your 
> || driver's dependencies with the current tools in a way that doesnt 
> || break the build.
> 
> and that's exactly what happens with Roman's patch: a Kconfig subsystem 
> design bug (its inability to properly propagate the dependencies of 
> select's) is worked around in the driver space: by the LEDS_CORE driver 
> config introduction and no user-visible.
> 
> Roman's patch is obviously cleaner than my hack (i just fixed a single 
> instantiation of the problem, while he changed the LEDS driver 
> dependency structure), but it's still a workaround for a Kconfig 
> subsystem bug and the same problem could reoccur elsewhere. It could hit 
> anytime dual dependencies are introduced in a driver accidentally.

Ingo, perhaps it helps if I put it in caps:

YOU TRIED TO PUSH AN INPUT PATCH FOR AN X86 BUG.

Roman's patch is better than adding a select, but your patch would have 
added the select in the completely wrong subsystem.

Why can't you admit the patch you tried to push was wrong?

> As Sam said it, fixing that Kconfig design bug would be "nice" - but 
> unfortunately the Kconfig subsystem is not actively developed anymore. 

Roman is still active.

> Would you like to volunteer for that? It would be a _very_ useful 
> contribution. One such fix could avoid hundreds or even thousands of 
> trivial problems in the future - it could avoid having to make hundreds 
> or thousands of trivial patches in the future.

Different to you I actually have some basic understanding how kconfig 
works.

I'm not even sure the semantics of "select follows dependencies" 
would actually be better than what we have today.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 20:24             ` Adrian Bunk
@ 2008-05-03 21:03               ` Ingo Molnar
  2008-05-03 21:24                 ` Adrian Bunk
  2008-05-03 21:17               ` Krzysztof Halasa
  1 sibling, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-05-03 21:03 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Adrian Bunk <bunk@kernel.org> wrote:

> > > You wrongly (and loudly) blamed Dmitry for something you broke 
> > > yourself.
> > 
> > i didnt. Read what i wrote:
> > 
> > || no, you are wrong, read the current Kconfig rules again. If the user 
> > || can create a .config that does not build, it is driver breakage. It 
> > || always was, and has been in the past 15 years.
> > ||
> > || Kconfig might be extended to make dependencies easier to manage for 
> > || developers but until that is implemented you have to craft your 
> > || driver's dependencies with the current tools in a way that doesnt 
> > || break the build.
> > 
> > and that's exactly what happens with Roman's patch: a Kconfig subsystem 
> > design bug (its inability to properly propagate the dependencies of 
> > select's) is worked around in the driver space: by the LEDS_CORE driver 
> > config introduction and no user-visible.
> > 
> > Roman's patch is obviously cleaner than my hack (i just fixed a single 
> > instantiation of the problem, while he changed the LEDS driver 
> > dependency structure), but it's still a workaround for a Kconfig 
> > subsystem bug and the same problem could reoccur elsewhere. It could hit 
> > anytime dual dependencies are introduced in a driver accidentally.
> 
> Ingo, perhaps it helps if I put it in caps:
> 
> YOU TRIED TO PUSH AN INPUT PATCH FOR AN X86 BUG.

Adrian, your tone is getting more and more abusive, and while i can 
easily ignore your abuse you are not just doing it towards me but 
towards other kernel developers as well. You need to stop that.

Yes, the incomplete (and buggy) select came from an x86 Kconfig file but 
you missed the real argument. Half a dozen other times similar bugs came 
from other portions of the tree so it's a reoccuring pattern of bugs.

And had you read the exchange more carefully you'd notice that the 
discussion was about the Kconfig subsystem bug which is causing all 
these other bugs - which Kconfig subsystem bug is still unfixed. In the 
discussion Dmitry assumed the obvious: that a select on a component will 
also select the sub-components. The problem is - select does not do 
that.

> Roman's patch is better than adding a select, but your patch would 
> have added the select in the completely wrong subsystem.

sure, and that's exactly what i said above: "Roman's patch is obviously 
cleaner than my hack". It avoids this problem by creating a single 
target to select for.

A wrong/buggy select _somewhere_ (this time the bug indeed was in an x86 
subarch Kconfig - but often it was just in a driver that tried to enable 
LEDS support) breaks the build - instead of propagating dependencies or 
at least warning about the problem. That's a Kconfig subsystem design 
bug and it has been known for years.

it's now worked around by Roman's patch by making the LEDS Kconfig 
structure simpler so there's just a single select target. But the root 
cause was not fixed and similar issues could hit the kernel anytime in 
the future. So it's not a real fix - it just prolonges the real fix some 
more.

> Why can't you admit the patch you tried to push was wrong?

how many times do i have to repeat it that yes it was a hack and that it 
was wrong?

> > As Sam said it, fixing that Kconfig design bug would be "nice" - but 
> > unfortunately the Kconfig subsystem is not actively developed 
> > anymore.
> 
> Roman is still active.

great, does this mean we'll see fixes for select's misbehavior, along 
the lines of Sam's suggestions?

at minimum a warning needs to be emitted by the kconfig tool if such 
incomplete selects are used. I've stopped counting the number of times 
such issues have broken the build and have held up kernel development. 

All the information is already in the Kconfig files for the kconfig 
tool/subsystem to make an intelligent decision. It's just not fully 
used, and the burden of fixing these problems is pushed back to the 
developers who create the Kconfig files.

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 20:24             ` Adrian Bunk
  2008-05-03 21:03               ` Ingo Molnar
@ 2008-05-03 21:17               ` Krzysztof Halasa
  2008-05-03 21:47                 ` Adrian Bunk
  1 sibling, 1 reply; 32+ messages in thread
From: Krzysztof Halasa @ 2008-05-03 21:17 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

Adrian Bunk <bunk@kernel.org> writes:

> I'm not even sure the semantics of "select follows dependencies" 
> would actually be better than what we have today.

I think it would. But how would that actually work? By displaying all
relevant dependency chains and asking the user to select one?
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:03               ` Ingo Molnar
@ 2008-05-03 21:24                 ` Adrian Bunk
  2008-05-03 21:38                   ` Sam Ravnborg
                                     ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 21:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton

On Sat, May 03, 2008 at 11:03:00PM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@kernel.org> wrote:
>...
> > > As Sam said it, fixing that Kconfig design bug would be "nice" - but 
> > > unfortunately the Kconfig subsystem is not actively developed 
> > > anymore.
> > 
> > Roman is still active.
> 
> great, does this mean we'll see fixes for select's misbehavior, along 
> the lines of Sam's suggestions?

In the case of the problem here it would have turned one problem into 
another, and Roman's patch is the correct solution no matter whether 
you change the semantics of select or not.

> at minimum a warning needs to be emitted by the kconfig tool if such 
> incomplete selects are used. I've stopped counting the number of times 
> such issues have broken the build and have held up kernel development. 

It might held up your randconfig compiles.

Actual kernel development isn't much affected.

Before you started your randconfig builds and sending (often buggy) 
kconfig patches like crazy there wasn't a problem (and Toralf's 
randconfig builds already catched these problems in the past).

> All the information is already in the Kconfig files for the kconfig 
> tool/subsystem to make an intelligent decision. It's just not fully 
> used, and the burden of fixing these problems is pushed back to the 
> developers who create the Kconfig files.

That's nonsense.

Describe the semantics you want for "bool selects tristate", and no 
matter which you choose I'll tell you a case where it breaks.

You've shown with another of your recent kconfig patches that you don't 
even understand how "depends on" works, and you are _very_ far from 
making claims like the one you just made.

Please accept the fact that while I'm (at least at the moment) not 
qualified to make any deeply technical remarks on e.g. the CPU scheduler 
or x86 details, you are (at least at the moment) not qualified to make 
any deeply technical remarks on kconfig.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:24                 ` Adrian Bunk
@ 2008-05-03 21:38                   ` Sam Ravnborg
  2008-05-03 22:07                     ` Adrian Bunk
  2008-05-03 21:52                   ` Ingo Molnar
  2008-05-03 23:22                   ` Thomas Gleixner
  2 siblings, 1 reply; 32+ messages in thread
From: Sam Ravnborg @ 2008-05-03 21:38 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, May 04, 2008 at 12:24:29AM +0300, Adrian Bunk wrote:
> On Sat, May 03, 2008 at 11:03:00PM +0200, Ingo Molnar wrote:
> > 
> > * Adrian Bunk <bunk@kernel.org> wrote:
> >...
> > > > As Sam said it, fixing that Kconfig design bug would be "nice" - but 
> > > > unfortunately the Kconfig subsystem is not actively developed 
> > > > anymore.
> > > 
> > > Roman is still active.
> > 
> > great, does this mean we'll see fixes for select's misbehavior, along 
> > the lines of Sam's suggestions?
> 
> In the case of the problem here it would have turned one problem into 
> another, and Roman's patch is the correct solution no matter whether 
> you change the semantics of select or not.

Hi Adrian 
It would be nice to know what was wrong with my suggestion.
You have done your share of kconfig patches so you have a good grip
on the problems we face.
So any input on how we can improve kconfig so we can actually provide
what people often expects or requests or need would be nice.

I have so far not done any hacking on the core of the backend of
kconfig but if I one day find more than one hour in row where I can do
some kernel stuff I may actually try it out.
And then it would be nice to have a sketch ready how to solve the issues.

I would though request you to start a new mail thread for this and
include both linux-kbuild@vger.kernel.org and Roman Zippel in the loop.

	Sam

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:17               ` Krzysztof Halasa
@ 2008-05-03 21:47                 ` Adrian Bunk
  2008-05-03 22:13                   ` Krzysztof Halasa
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 21:47 UTC (permalink / raw)
  To: Krzysztof Halasa
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sat, May 03, 2008 at 11:17:09PM +0200, Krzysztof Halasa wrote:
> Adrian Bunk <bunk@kernel.org> writes:
> 
> > I'm not even sure the semantics of "select follows dependencies" 
> > would actually be better than what we have today.
> 
> I think it would. But how would that actually work? By displaying all
> relevant dependency chains and asking the user to select one?

Let's look at the problem this thread is about:

menuconfig NEW_LEDS
        bool "LED Support"

config LEDS_CLASS
        tristate "LED Class Support"
	depends on NEW_LEDS              # actually an "if", but that's 
                                           just syntactical sugar

config X86_RDC321X
        bool "RDC R-321x SoC"
        select LEDS_CLASS


If you select LEDS_CLASS "select follows dependencies" would let inherit 
X86_RDC321X the dependencies of LEDS_CLASS, IOW treat it as:

config X86_RDC321X
        bool "RDC R-321x SoC"
        select LEDS_CLASS
	depends on NEW_LEDS


That might make the randconfig crowd happy.

But from an UI perspective it's not an improvement.


And regarding "displaying all relevant dependency chains" to the user - 
I can't see how that would work in the more complicated cases.


> Krzysztof Halasa

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:24                 ` Adrian Bunk
  2008-05-03 21:38                   ` Sam Ravnborg
@ 2008-05-03 21:52                   ` Ingo Molnar
  2008-05-03 22:03                     ` Adrian Bunk
  2008-05-03 23:22                   ` Thomas Gleixner
  2 siblings, 1 reply; 32+ messages in thread
From: Ingo Molnar @ 2008-05-03 21:52 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Dmitry Torokhov, linux-kernel, Rafael J. Wysocki, Andrew Morton


* Adrian Bunk <bunk@kernel.org> wrote:

> > at minimum a warning needs to be emitted by the kconfig tool if such 
> > incomplete selects are used. I've stopped counting the number of 
> > times such issues have broken the build and have held up kernel 
> > development.
> 
> It might held up your randconfig compiles.
> 
> Actual kernel development isn't much affected.

uhm, you are quite wrong - countless times have people been bitten by 
select's breakages in the past, and not via randconfig. That's the main 
reason why select use in Kconfig was not encouraged for a long time.

Select does make sense in some situations but it's a double-edged sword: 
kconfig does not warn at all about the situations where it's "unsafe" to 
use it - while it has all the information in the Kconfig files to emit 
that warning. Instead we get build breakages not visible when an 
incorrect select is added, but much later, if someone happens to stumble 
on the wrong kind of .config. That is obviously harmful.

My larger point is that this kconfig tool bug breeds a constant stream 
of avoidable breakages, which causes lost manpower and causes a stream 
of trivial patches hindering maintainers all around the tree. Because 
every such trivial patch has to be reviewed, tested, it clogs the commit 
logs, etc.

So the more trivial patches we _avoid_ having to do in the future, the 
better. I'm not sure why you are even arguing against this this rather 
simple point - your arguments are rather hard to understand. Wouldnt you 
be happier if a whole category of trivial breakages was avoided and if 
you didnt have to deal with and waste your time on that category of 
trivial patches anymore?

Most of the time reoccuring trivial patches are an indicator of some 
deeper structural problem - as in this case.

	Ingo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:52                   ` Ingo Molnar
@ 2008-05-03 22:03                     ` Adrian Bunk
  2008-05-04  3:54                       ` Valdis.Kletnieks
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 22:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Adrian Bunk, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sat, May 03, 2008 at 11:52:14PM +0200, Ingo Molnar wrote:
> 
> * Adrian Bunk <bunk@kernel.org> wrote:
> 
> > > at minimum a warning needs to be emitted by the kconfig tool if such 
> > > incomplete selects are used. I've stopped counting the number of 
> > > times such issues have broken the build and have held up kernel 
> > > development.
> > 
> > It might held up your randconfig compiles.
> > 
> > Actual kernel development isn't much affected.
> 
> uhm, you are quite wrong - countless times have people been bitten by 
> select's breakages in the past, and not via randconfig. That's the main 
> reason why select use in Kconfig was not encouraged for a long time.

But it has not "held up kernel development".

Send a bug report to linux-kernel or with a Cc to me and I'll look at 
the bug.

I've done this for years.

> Select does make sense in some situations but it's a double-edged sword: 
> kconfig does not warn at all about the situations where it's "unsafe" to 
> use it - while it has all the information in the Kconfig files to emit 
> that warning. Instead we get build breakages not visible when an 
> incorrect select is added, but much later, if someone happens to stumble 
> on the wrong kind of .config. That is obviously harmful.

And how would a warning help?

>From an UI perspective we often want to select options that have 
dependencies.

> My larger point is that this kconfig tool bug breeds a constant stream 
> of avoidable breakages, which causes lost manpower and causes a stream 
> of trivial patches hindering maintainers all around the tree. Because 
> every such trivial patch has to be reviewed, tested, it clogs the commit 
> logs, etc.
> 
> So the more trivial patches we _avoid_ having to do in the future, the 
> better. I'm not sure why you are even arguing against this this rather 
> simple point - your arguments are rather hard to understand. Wouldnt you 
> be happier if a whole category of trivial breakages was avoided and if 
> you didnt have to deal with and waste your time on that category of 
> trivial patches anymore?
> 
> Most of the time reoccuring trivial patches are an indicator of some 
> deeper structural problem - as in this case.

Your conclusions are based on an assumption that isn't true.

"trivial patches" are the patches you send.

But they are often bogus.

Fixing these issues properly often requires a deeper understanding of 
both kconfig and the dependencies of the underlying code.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:38                   ` Sam Ravnborg
@ 2008-05-03 22:07                     ` Adrian Bunk
  2008-05-04  7:36                       ` Sam Ravnborg
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 22:07 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sat, May 03, 2008 at 11:38:22PM +0200, Sam Ravnborg wrote:
> On Sun, May 04, 2008 at 12:24:29AM +0300, Adrian Bunk wrote:
> > On Sat, May 03, 2008 at 11:03:00PM +0200, Ingo Molnar wrote:
> > > 
> > > * Adrian Bunk <bunk@kernel.org> wrote:
> > >...
> > > > > As Sam said it, fixing that Kconfig design bug would be "nice" - but 
> > > > > unfortunately the Kconfig subsystem is not actively developed 
> > > > > anymore.
> > > > 
> > > > Roman is still active.
> > > 
> > > great, does this mean we'll see fixes for select's misbehavior, along 
> > > the lines of Sam's suggestions?
> > 
> > In the case of the problem here it would have turned one problem into 
> > another, and Roman's patch is the correct solution no matter whether 
> > you change the semantics of select or not.
> 
> Hi Adrian 
> It would be nice to know what was wrong with my suggestion.
> You have done your share of kconfig patches so you have a good grip
> on the problems we face.
> So any input on how we can improve kconfig so we can actually provide
> what people often expects or requests or need would be nice.
> 
> I have so far not done any hacking on the core of the backend of
> kconfig but if I one day find more than one hour in row where I can do
> some kernel stuff I may actually try it out.
> And then it would be nice to have a sketch ready how to solve the issues.
> 
> I would though request you to start a new mail thread for this and
> include both linux-kbuild@vger.kernel.org and Roman Zippel in the loop.

If you've sent a suggestion that included a description of the exact 
semantics you want to implement I must have missed it.

Can you send me a pointer or a description of the semantics?
I'll then have a look.

> 	Sam

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:47                 ` Adrian Bunk
@ 2008-05-03 22:13                   ` Krzysztof Halasa
  2008-05-03 22:29                     ` Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Halasa @ 2008-05-03 22:13 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

Adrian Bunk <bunk@kernel.org> writes:

> If you select LEDS_CLASS "select follows dependencies" would let inherit 
> X86_RDC321X the dependencies of LEDS_CLASS,

I see, "follows" the other way around. That fixes the breakage but I
guess it's not exactly what the people doing make menuconfig want.
Though in absence of the following it would IMHO make sense anyway.

> And regarding "displaying all relevant dependency chains" to the user - 
> I can't see how that would work in the more complicated cases.

I fear it too but perhaps there is some sane way? I don't know,
asking (recursively?) interactively?

Like:
QWERTY requires at least one, select (Y/M):
- ARCH_X86 && NET_ETHERNET, or
- USB4_0 && !SMP, or
- XXX

then:
NET_ETHERNET requires at lest one, select (Y/M):
- PCI128
- ISA256

then (info):
PCI128 requires PCI and it's selected automatically.

I don't know. It could be useful even with a single dependency when
configuring as the module, the user could select Y or M for dependency
interactively (people may want to have E1000E=m and TULIP=m but MII=y).

Not sure if still on the same planet, though :-)
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 22:13                   ` Krzysztof Halasa
@ 2008-05-03 22:29                     ` Adrian Bunk
  2008-05-03 23:37                       ` Krzysztof Halasa
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-03 22:29 UTC (permalink / raw)
  To: Krzysztof Halasa
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, May 04, 2008 at 12:13:29AM +0200, Krzysztof Halasa wrote:
> Adrian Bunk <bunk@kernel.org> writes:
> 
> > If you select LEDS_CLASS "select follows dependencies" would let inherit 
> > X86_RDC321X the dependencies of LEDS_CLASS,
> 
> I see, "follows" the other way around. That fixes the breakage but I
> guess it's not exactly what the people doing make menuconfig want.
> Though in absence of the following it would IMHO make sense anyway.

Kconfig is an interface for users.

>From a developer perspective if might be the easiest to simply remove 
the "select" syntax, but from an UI perspective it would be horrible.

> > And regarding "displaying all relevant dependency chains" to the user - 
> > I can't see how that would work in the more complicated cases.
> 
> I fear it too but perhaps there is some sane way? I don't know,
> asking (recursively?) interactively?
> 
> Like:
> QWERTY requires at least one, select (Y/M):
> - ARCH_X86 && NET_ETHERNET, or
> - USB4_0 && !SMP, or
> - XXX
> 
> then:
> NET_ETHERNET requires at lest one, select (Y/M):
> - PCI128
> - ISA256
> 
> then (info):
> PCI128 requires PCI and it's selected automatically.

FB_SGIVW requires X86_VISWS and it's selected automatically.

(which is not good if you want a kernel that runs on a PC)

> I don't know. It could be useful even with a single dependency when
> configuring as the module, the user could select Y or M for dependency
> interactively (people may want to have E1000E=m and TULIP=m but MII=y).

Why do we have to bother users with the MII option at all?

"E1000E=m and TULIP=m but MII=y" works, but it doesn't really make 
sense.

> Not sure if still on the same planet, though :-)
> -- 
> Krzysztof Halasa

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 21:24                 ` Adrian Bunk
  2008-05-03 21:38                   ` Sam Ravnborg
  2008-05-03 21:52                   ` Ingo Molnar
@ 2008-05-03 23:22                   ` Thomas Gleixner
  2008-05-04  0:34                     ` Adrian Bunk
  2 siblings, 1 reply; 32+ messages in thread
From: Thomas Gleixner @ 2008-05-03 23:22 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, 4 May 2008, Adrian Bunk wrote:
> > at minimum a warning needs to be emitted by the kconfig tool if such 
> > incomplete selects are used. I've stopped counting the number of times 
> > such issues have broken the build and have held up kernel development. 
> 
> It might held up your randconfig compiles.
> 
> Actual kernel development isn't much affected.

Interesting. You confidently define how kernel development works and
what affects it and what not. 

I just caught a build wreckage via randconfig builds on a patch which
was sent through Andrew to x86 for merging into mainline.

So you think that we should stop sharing with the wider kernel
community the fixes that result out of our automated randconfig builds
and should instead wait for Joe User or another kernel developer to
trip into some subtle missing Kconfig dependency, just because a fix
might not be the right one and might need another iteration before it
can go upstream?

You are wrong as usual.

I for one prefer getting a (even wrong) patch which points to the
problem instead of some (often grumpy) "this broke" message.

> Before you started your randconfig builds and sending (often buggy) 
> kconfig patches like crazy there wasn't a problem (and Toralf's 
> randconfig builds already catched these problems in the past).

FYI, Ingo has been doing randconfig build and boot tests for a long,
long time. There are dozens and dozens of (non-build) fixes in the
upstream kernel that resulted from that effort - it's really valuable
in practice.

So what you say is complete nonsense.

The RDC subarch commit was buggy, but that does not justify your
insulting and self-righteous behaviour at all. If you think that your
behaviour vs. Ingo is improving kernel development, then you are on
the completely wrong track. It might earn you an entry in his
killfile, but nothing else.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 22:29                     ` Adrian Bunk
@ 2008-05-03 23:37                       ` Krzysztof Halasa
  2008-05-04  0:49                         ` Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Halasa @ 2008-05-03 23:37 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

Adrian Bunk <bunk@kernel.org> writes:

> FB_SGIVW requires X86_VISWS and it's selected automatically.
>
> (which is not good if you want a kernel that runs on a PC)

Not sure what do you mean, it currently seem to "depend" on X86_VISWS:

config FB_SGIVW
        tristate "SGI Visual Workstation framebuffer support"
        depends on FB && X86_VISWS
        select FB_CFB_FILLRECT
        select FB_CFB_COPYAREA
        select FB_CFB_IMAGEBLIT

> Why do we have to bother users with the MII option at all?
>
> "E1000E=m and TULIP=m but MII=y" works, but it doesn't really make 
> sense.

(Obviously it assumes both TULIP and E1000E required MII which is not
exactly the case)

But it makes a perfect sense, I can have modular drivers for (few)
hardware devices (just in case I want to rmmod etc) but most of the
kernel may be not modular. It would be nice if the Kconfig ask me if
I want to "select" the dependency Y or M, even if there is only one
way to make "select" dependencies happy (not counting "Y vs "M").
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 23:22                   ` Thomas Gleixner
@ 2008-05-04  0:34                     ` Adrian Bunk
  0 siblings, 0 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-04  0:34 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, May 04, 2008 at 01:22:15AM +0200, Thomas Gleixner wrote:
> On Sun, 4 May 2008, Adrian Bunk wrote:
> > > at minimum a warning needs to be emitted by the kconfig tool if such 
> > > incomplete selects are used. I've stopped counting the number of times 
> > > such issues have broken the build and have held up kernel development. 
> > 
> > It might held up your randconfig compiles.
> > 
> > Actual kernel development isn't much affected.
> 
> Interesting. You confidently define how kernel development works and
> what affects it and what not. 
> 
> I just caught a build wreckage via randconfig builds on a patch which
> was sent through Andrew to x86 for merging into mainline.
> 
> So you think that we should stop sharing with the wider kernel
> community the fixes that result out of our automated randconfig builds
> and should instead wait for Joe User or another kernel developer to
> trip into some subtle missing Kconfig dependency, just because a fix
> might not be the right one and might need another iteration before it
> can go upstream?
> 
> You are wrong as usual.
> 
> I for one prefer getting a (even wrong) patch which points to the
> problem instead of some (often grumpy) "this broke" message.

Ingo' message that caused me to change the subject of this thread was
  http://lkml.org/lkml/2008/4/30/446

Ingo's patch did not point at the actual problem at all (it was a x86 
bug and Ingo's patch was for input). But he tried to push his bogus 
patch through Dmitry who had correctly stated that the patch was wrong.

This way of "sharing fixes" even against a correct maintainer NAK is 
what annoys me.

Am I really wrong as usual on that?

And most issues randconfig finds are some cornercases unlikely to happen 
for users (hey, this thread is about a bug only occuring in 
configurations that include support for an X-Box gamepad in a kernel for 
an RDC R-321x SoC), so there's usually no need to hurry but time to look 
at the problem and fix it properly.

> > Before you started your randconfig builds and sending (often buggy) 
> > kconfig patches like crazy there wasn't a problem (and Toralf's 
> > randconfig builds already catched these problems in the past).
> 
> FYI, Ingo has been doing randconfig build and boot tests for a long,
> long time. There are dozens and dozens of (non-build) fixes in the
> upstream kernel that resulted from that effort - it's really valuable
> in practice.

Since when is he actually doing regular randconfig builds?

The only person from whom I'm aware of regularly seeing reports of build 
breakages with randconfig on linux-kernel for many years is Toralf.

> So what you say is complete nonsense.
> 
> The RDC subarch commit was buggy, but that does not justify your
> insulting and self-righteous behaviour at all. If you think that your
> behaviour vs. Ingo is improving kernel development, then you are on
> the completely wrong track. It might earn you an entry in his
> killfile, but nothing else.

When talking about "self-righteous behaviour" please also talk about how 
Ingo rants about kconfig and makes statements like "All the information 
is already in the Kconfig files for the kconfig tool/subsystem to make 
an intelligent decision." [1] that are simply wrong.

> Thanks,
> 
> 	tglx

cu
Adrian

[1] http://lkml.org/lkml/2008/5/3/215

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 23:37                       ` Krzysztof Halasa
@ 2008-05-04  0:49                         ` Adrian Bunk
  2008-05-04 12:18                           ` Krzysztof Halasa
  0 siblings, 1 reply; 32+ messages in thread
From: Adrian Bunk @ 2008-05-04  0:49 UTC (permalink / raw)
  To: Krzysztof Halasa
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, May 04, 2008 at 01:37:47AM +0200, Krzysztof Halasa wrote:
> Adrian Bunk <bunk@kernel.org> writes:
> 
> > FB_SGIVW requires X86_VISWS and it's selected automatically.
> >
> > (which is not good if you want a kernel that runs on a PC)
> 
> Not sure what do you mean, it currently seem to "depend" on X86_VISWS:
> 
> config FB_SGIVW
>         tristate "SGI Visual Workstation framebuffer support"
>         depends on FB && X86_VISWS
>         select FB_CFB_FILLRECT
>         select FB_CFB_COPYAREA
>         select FB_CFB_IMAGEBLIT

Unless I misunderstand your proposal you want to resolve dependencies 
reversely.

And if you want to only do this for select's: FB_SGIVW could also be 
select'ed.

> > Why do we have to bother users with the MII option at all?
> >
> > "E1000E=m and TULIP=m but MII=y" works, but it doesn't really make 
> > sense.
> 
> (Obviously it assumes both TULIP and E1000E required MII which is not
> exactly the case)
> 
> But it makes a perfect sense, I can have modular drivers for (few)
> hardware devices (just in case I want to rmmod etc) but most of the
> kernel may be not modular. It would be nice if the Kconfig ask me if
> I want to "select" the dependency Y or M, even if there is only one
> way to make "select" dependencies happy (not counting "Y vs "M").

Getting the cases that really matter working would already be hard 
enough...

> Krzysztof Halasa

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 22:03                     ` Adrian Bunk
@ 2008-05-04  3:54                       ` Valdis.Kletnieks
  2008-05-04  7:47                         ` Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Valdis.Kletnieks @ 2008-05-04  3:54 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

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

On Sun, 04 May 2008 01:03:30 +0300, Adrian Bunk said:
> On Sat, May 03, 2008 at 11:52:14PM +0200, Ingo Molnar wrote:

> > My larger point is that this kconfig tool bug breeds a constant stream 
> > of avoidable breakages, which causes lost manpower and causes a stream 
> > of trivial patches hindering maintainers all around the tree. Because 
> > every such trivial patch has to be reviewed, tested, it clogs the commit 
> > logs, etc.
> > 
> > So the more trivial patches we _avoid_ having to do in the future, the 
> > better. I'm not sure why you are even arguing against this this rather 
> > simple point - your arguments are rather hard to understand. Wouldnt you 
> > be happier if a whole category of trivial breakages was avoided and if 
> > you didnt have to deal with and waste your time on that category of 
> > trivial patches anymore?
> > 
> > Most of the time reoccuring trivial patches are an indicator of some 
> > deeper structural problem - as in this case.
> 
> Your conclusions are based on an assumption that isn't true.
> 
> "trivial patches" are the patches you send.
> 
> But they are often bogus.
> 
> Fixing these issues properly often requires a deeper understanding of 
> both kconfig and the dependencies of the underlying code.

I suspect that Ingo is however correct - although a *proper* fix of one of
these bugs requires human-intelligence to figure out what's *really* intended,
the kconfig program *does* have enough information available to issue a a clear
warning:

"Yo doodz - I don't know *what* you intended here, but this SELECT is just
waiting to sink its teeth into somebody's posterior.  You might want to fix it
somehow before somebody needs rabies shots..."

There's a long tradition of toolchains issuing warnings about things that a
toolchain can detect, but cannot correct - for instance, the gcc 'variable is
used uninitialized' warning (distinct from the broken "may be used" warning).
Given the truly insane amounts of code we merge every cycle, we should take
advantage of every opportunity for automated detection of things that can't
possibly be right.

(Ingo - flame me if I've misread your position here...)


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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-03 22:07                     ` Adrian Bunk
@ 2008-05-04  7:36                       ` Sam Ravnborg
  2008-05-04  7:49                         ` Adrian Bunk
  0 siblings, 1 reply; 32+ messages in thread
From: Sam Ravnborg @ 2008-05-04  7:36 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

> > Hi Adrian 
> > It would be nice to know what was wrong with my suggestion.
> > You have done your share of kconfig patches so you have a good grip
> > on the problems we face.
> > So any input on how we can improve kconfig so we can actually provide
> > what people often expects or requests or need would be nice.
> > 
> > I have so far not done any hacking on the core of the backend of
> > kconfig but if I one day find more than one hour in row where I can do
> > some kernel stuff I may actually try it out.
> > And then it would be nice to have a sketch ready how to solve the issues.
> > 
> > I would though request you to start a new mail thread for this and
> > include both linux-kbuild@vger.kernel.org and Roman Zippel in the loop.
> 
> If you've sent a suggestion that included a description of the exact 
> semantics you want to implement I must have missed it.
> 
> Can you send me a pointer or a description of the semantics?
> I'll then have a look.

I took the original idea and added a few more details.
Please see mail named "http://lkml.org/lkml/2008/5/4/18"
http://lkml.org/lkml/2008/5/4/18

	Sam

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-04  3:54                       ` Valdis.Kletnieks
@ 2008-05-04  7:47                         ` Adrian Bunk
  0 siblings, 0 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-04  7:47 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sat, May 03, 2008 at 11:54:11PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Sun, 04 May 2008 01:03:30 +0300, Adrian Bunk said:
> > On Sat, May 03, 2008 at 11:52:14PM +0200, Ingo Molnar wrote:
> 
> > > My larger point is that this kconfig tool bug breeds a constant stream 
> > > of avoidable breakages, which causes lost manpower and causes a stream 
> > > of trivial patches hindering maintainers all around the tree. Because 
> > > every such trivial patch has to be reviewed, tested, it clogs the commit 
> > > logs, etc.
> > > 
> > > So the more trivial patches we _avoid_ having to do in the future, the 
> > > better. I'm not sure why you are even arguing against this this rather 
> > > simple point - your arguments are rather hard to understand. Wouldnt you 
> > > be happier if a whole category of trivial breakages was avoided and if 
> > > you didnt have to deal with and waste your time on that category of 
> > > trivial patches anymore?
> > > 
> > > Most of the time reoccuring trivial patches are an indicator of some 
> > > deeper structural problem - as in this case.
> > 
> > Your conclusions are based on an assumption that isn't true.
> > 
> > "trivial patches" are the patches you send.
> > 
> > But they are often bogus.
> > 
> > Fixing these issues properly often requires a deeper understanding of 
> > both kconfig and the dependencies of the underlying code.
> 
> I suspect that Ingo is however correct

Ingo claims the problem was trivial since the patches were trivial.

But fact is they aren't trivial - as you can e.g. see on Ingos patch 
that started this thread, and that was for the completely wrong place.

> - although a *proper* fix of one of
> these bugs requires human-intelligence to figure out what's *really* intended,
> the kconfig program *does* have enough information available to issue a a clear
> warning:
> 
> "Yo doodz - I don't know *what* you intended here, but this SELECT is just
> waiting to sink its teeth into somebody's posterior.  You might want to fix it
> somehow before somebody needs rabies shots..."
>...

And what do you want to do in such a case?

Kconfig is a user interface, and we actually need such cases you want to 
warn for for getting a good UI.

We already know what can cause problems.

But as far as I know there are no such problems users actually ran into 
in recent stable kernels - and most of the problems (like the one in 
this thread) are pathological cornercases you only see with randconfig.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-04  7:36                       ` Sam Ravnborg
@ 2008-05-04  7:49                         ` Adrian Bunk
  0 siblings, 0 replies; 32+ messages in thread
From: Adrian Bunk @ 2008-05-04  7:49 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

On Sun, May 04, 2008 at 09:36:14AM +0200, Sam Ravnborg wrote:
> > > Hi Adrian 
> > > It would be nice to know what was wrong with my suggestion.
> > > You have done your share of kconfig patches so you have a good grip
> > > on the problems we face.
> > > So any input on how we can improve kconfig so we can actually provide
> > > what people often expects or requests or need would be nice.
> > > 
> > > I have so far not done any hacking on the core of the backend of
> > > kconfig but if I one day find more than one hour in row where I can do
> > > some kernel stuff I may actually try it out.
> > > And then it would be nice to have a sketch ready how to solve the issues.
> > > 
> > > I would though request you to start a new mail thread for this and
> > > include both linux-kbuild@vger.kernel.org and Roman Zippel in the loop.
> > 
> > If you've sent a suggestion that included a description of the exact 
> > semantics you want to implement I must have missed it.
> > 
> > Can you send me a pointer or a description of the semantics?
> > I'll then have a look.
> 
> I took the original idea and added a few more details.
> Please see mail named "http://lkml.org/lkml/2008/5/4/18"
> http://lkml.org/lkml/2008/5/4/18

Thanks, I'll answer there.

> 	Sam

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Ingo, no more kconfig patches
  2008-05-04  0:49                         ` Adrian Bunk
@ 2008-05-04 12:18                           ` Krzysztof Halasa
  0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Halasa @ 2008-05-04 12:18 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ingo Molnar, Dmitry Torokhov, linux-kernel, Rafael J. Wysocki,
	Andrew Morton

Adrian Bunk <bunk@kernel.org> writes:

> Unless I misunderstand your proposal you want to resolve dependencies 
> reversely.
>
> And if you want to only do this for select's: FB_SGIVW could also be 
> select'ed.

Right. Sure, if something selects FB_SGIVW then the user would be
presented a list of dependencies and would have to resolve them to be
allowed to continue.

I think I will have to look at it when the so called free time comes
:-)
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2008-05-04 12:18 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-30 20:03 [patch] input: JOYSTICK_XPAD build fix Ingo Molnar
2008-04-30 21:02 ` Dmitry Torokhov
2008-04-30 21:13   ` Ingo Molnar
2008-04-30 23:01     ` Ingo, no more kconfig patches Adrian Bunk
2008-05-01  1:17       ` Ingo Molnar
2008-05-01  1:37         ` Adrian Bunk
2008-05-01  2:06           ` Ingo Molnar
2008-05-01  2:12             ` Adrian Bunk
2008-05-01  2:52       ` Ingo Molnar
2008-05-01 11:59         ` Adrian Bunk
2008-05-03 19:14           ` Ingo Molnar
2008-05-03 19:17             ` Ingo Molnar
2008-05-03 20:24             ` Adrian Bunk
2008-05-03 21:03               ` Ingo Molnar
2008-05-03 21:24                 ` Adrian Bunk
2008-05-03 21:38                   ` Sam Ravnborg
2008-05-03 22:07                     ` Adrian Bunk
2008-05-04  7:36                       ` Sam Ravnborg
2008-05-04  7:49                         ` Adrian Bunk
2008-05-03 21:52                   ` Ingo Molnar
2008-05-03 22:03                     ` Adrian Bunk
2008-05-04  3:54                       ` Valdis.Kletnieks
2008-05-04  7:47                         ` Adrian Bunk
2008-05-03 23:22                   ` Thomas Gleixner
2008-05-04  0:34                     ` Adrian Bunk
2008-05-03 21:17               ` Krzysztof Halasa
2008-05-03 21:47                 ` Adrian Bunk
2008-05-03 22:13                   ` Krzysztof Halasa
2008-05-03 22:29                     ` Adrian Bunk
2008-05-03 23:37                       ` Krzysztof Halasa
2008-05-04  0:49                         ` Adrian Bunk
2008-05-04 12:18                           ` Krzysztof Halasa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox