linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 15/15] kconf: Check for eof from input stream.
@ 2006-01-04 22:01 Ben Collins
  2006-01-08 16:34 ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-04 22:01 UTC (permalink / raw)
  To: linux-kernel

Signed-off-by: Ben Collins <bcollins@ubuntu.com>

---

 scripts/kconfig/conf.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

62bb36dcb307ca53999a14f74da6a803e36d5f62
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 8ba5d29..10eeae5 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -63,6 +63,20 @@ static void check_stdin(void)
 	}
 }
 
+static char *fgets_check_stream(char *s, int size, FILE *stream)
+{
+	char *ret = fgets(s, size, stream);
+
+	if (ret == NULL && feof(stream)) {
+		printf(_("aborted!\n\n"));
+		printf(_("Console input is closed. "));
+		printf(_("Run 'make oldconfig' to update configuration.\n\n"));
+		exit(1);
+	}
+
+	return ret;
+}
+
 static void conf_askvalue(struct symbol *sym, const char *def)
 {
 	enum symbol_type type = sym_get_type(sym);
@@ -100,7 +114,7 @@ static void conf_askvalue(struct symbol 
 		check_stdin();
 	case ask_all:
 		fflush(stdout);
-		fgets(line, 128, stdin);
+		fgets_check_stream(line, 128, stdin);
 		return;
 	case set_default:
 		printf("%s\n", def);
@@ -356,7 +370,7 @@ static int conf_choice(struct menu *menu
 			check_stdin();
 		case ask_all:
 			fflush(stdout);
-			fgets(line, 128, stdin);
+			fgets_check_stream(line, 128, stdin);
 			strip(line);
 			if (line[0] == '?') {
 				printf("\n%s\n", menu->sym->help ?
-- 
1.0.5

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-04 22:01 [PATCH 15/15] kconf: Check for eof from input stream Ben Collins
@ 2006-01-08 16:34 ` Roman Zippel
  2006-01-08 18:53   ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-08 16:34 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Wednesday 04 January 2006 23:01, Ben Collins wrote:

> +static char *fgets_check_stream(char *s, int size, FILE *stream)
> +{
> +	char *ret = fgets(s, size, stream);
> +
> +	if (ret == NULL && feof(stream)) {
> +		printf(_("aborted!\n\n"));
> +		printf(_("Console input is closed. "));
> +		printf(_("Run 'make oldconfig' to update configuration.\n\n"));
> +		exit(1);
> +	}
> +
> +	return ret;
> +}

What problem does this solve? conf should finish normally anyway and just set 
everything to the default.

bye, Roman


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-08 16:34 ` Roman Zippel
@ 2006-01-08 18:53   ` Ben Collins
  2006-01-08 20:59     ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-08 18:53 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Sun, 2006-01-08 at 17:34 +0100, Roman Zippel wrote:
> Hi,
> 
> On Wednesday 04 January 2006 23:01, Ben Collins wrote:
> 
> > +static char *fgets_check_stream(char *s, int size, FILE *stream)
> > +{
> > +	char *ret = fgets(s, size, stream);
> > +
> > +	if (ret == NULL && feof(stream)) {
> > +		printf(_("aborted!\n\n"));
> > +		printf(_("Console input is closed. "));
> > +		printf(_("Run 'make oldconfig' to update configuration.\n\n"));
> > +		exit(1);
> > +	}
> > +
> > +	return ret;
> > +}
> 
> What problem does this solve? conf should finish normally anyway and just set 
> everything to the default.

It shouldn't, and it doesn't (that's what defconfig does, I believe).

Anyway, the problem is that if there is no terminal (e.g. stdout is
redirected to a file, and stdin is closed), then kconf loops forever
trying to get an answer (NULL is not the same as "").

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-08 18:53   ` Ben Collins
@ 2006-01-08 20:59     ` Roman Zippel
  2006-01-08 21:41       ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-08 20:59 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Sun, 8 Jan 2006, Ben Collins wrote:

> Anyway, the problem is that if there is no terminal (e.g. stdout is
> redirected to a file, and stdin is closed), then kconf loops forever
> trying to get an answer (NULL is not the same as "").

Then let's fix the real problem.

bye, Roman

     scripts/kconfig/conf.c |    3 +--
     1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.14/scripts/kconfig/conf.c
===================================================================
--- linux-2.6.14.orig/scripts/kconfig/conf.c	2006-01-08 21:52:54.000000000 +0100
+++ linux-2.6.14/scripts/kconfig/conf.c	2006-01-08 21:54:02.000000000 +0100
@@ -314,8 +314,7 @@ static int conf_choice(struct menu *menu
     		printf("%*s%s\n", indent - 1, "", menu_get_prompt(menu));
     		def_sym = sym_get_choice_value(sym);
     		cnt = def = 0;
-		line[0] = '0';
-		line[1] = 0;
+		line[0] = 0;
     		for (child = menu->list; child; child = child->next) {
     			if (!menu_is_visible(child))
     				continue;

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-08 20:59     ` Roman Zippel
@ 2006-01-08 21:41       ` Ben Collins
  2006-01-09  0:09         ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-08 21:41 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Sun, 2006-01-08 at 21:59 +0100, Roman Zippel wrote:
> Hi,
> 
> On Sun, 8 Jan 2006, Ben Collins wrote:
> 
> > Anyway, the problem is that if there is no terminal (e.g. stdout is
> > redirected to a file, and stdin is closed), then kconf loops forever
> > trying to get an answer (NULL is not the same as "").
> 
> Then let's fix the real problem.

That's not entirely acceptable. The reason this shows up is if an
automatic build is being done, and the config files are not up-to-date,
the prefered action is a build failure, not selecting defaults. The
reason for my fix was that instead of filling up diskspace with a
logfile of kconf's infinite loop, we just exit with an error.

Currently, this is the only way to ensure that these issues don't go
unnoticed.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-08 21:41       ` Ben Collins
@ 2006-01-09  0:09         ` Roman Zippel
  2006-01-09  3:59           ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-09  0:09 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Sunday 08 January 2006 22:41, Ben Collins wrote:

> That's not entirely acceptable. The reason this shows up is if an
> automatic build is being done, and the config files are not up-to-date,
> the prefered action is a build failure, not selecting defaults. The
> reason for my fix was that instead of filling up diskspace with a
> logfile of kconf's infinite loop, we just exit with an error.
>
> Currently, this is the only way to ensure that these issues don't go
> unnoticed.

Then something is wrong with your automatic build. If the config needs to be 
updated and stdin is redirected during a kbuild, it will already abort.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-09  0:09         ` Roman Zippel
@ 2006-01-09  3:59           ` Ben Collins
  2006-01-09 11:32             ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-09  3:59 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Mon, 2006-01-09 at 01:09 +0100, Roman Zippel wrote:
> Hi,
> 
> On Sunday 08 January 2006 22:41, Ben Collins wrote:
> 
> > That's not entirely acceptable. The reason this shows up is if an
> > automatic build is being done, and the config files are not up-to-date,
> > the prefered action is a build failure, not selecting defaults. The
> > reason for my fix was that instead of filling up diskspace with a
> > logfile of kconf's infinite loop, we just exit with an error.
> >
> > Currently, this is the only way to ensure that these issues don't go
> > unnoticed.
> 
> Then something is wrong with your automatic build. If the config needs to be 
> updated and stdin is redirected during a kbuild, it will already abort.

And what should be directed into stdin? Nothing. There should be no
input going into an automated build, exactly because it could produce
incorrect results.

BTW, this is the automatic build that Debian and Ubuntu both use (in
Debian's case, used for quite a number of years). So this isn't
something I whipped up.

Still, the build system shouldn't have to do anything special with stdin
to get the desired result. The code should also not react differently
based on stdin being closed, or being pumped with nul data.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-09  3:59           ` Ben Collins
@ 2006-01-09 11:32             ` Roman Zippel
  2006-01-09 13:42               ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-09 11:32 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Monday 09 January 2006 04:59, Ben Collins wrote:

> > Then something is wrong with your automatic build. If the config needs to
> > be updated and stdin is redirected during a kbuild, it will already
> > abort.
>
> And what should be directed into stdin? Nothing. There should be no
> input going into an automated build, exactly because it could produce
> incorrect results.
>
> BTW, this is the automatic build that Debian and Ubuntu both use (in
> Debian's case, used for quite a number of years). So this isn't
> something I whipped up.

That just means Debian's automatic build for the kernel has been broken for 
years. All normal config targets require user input and no input equals 
default input. Only silentoldconfig will abort if input is not available.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-09 11:32             ` Roman Zippel
@ 2006-01-09 13:42               ` Ben Collins
  2006-01-11 23:26                 ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-09 13:42 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Mon, 2006-01-09 at 12:32 +0100, Roman Zippel wrote:
> Hi,
> 
> On Monday 09 January 2006 04:59, Ben Collins wrote:
> 
> > > Then something is wrong with your automatic build. If the config needs to
> > > be updated and stdin is redirected during a kbuild, it will already
> > > abort.
> >
> > And what should be directed into stdin? Nothing. There should be no
> > input going into an automated build, exactly because it could produce
> > incorrect results.
> >
> > BTW, this is the automatic build that Debian and Ubuntu both use (in
> > Debian's case, used for quite a number of years). So this isn't
> > something I whipped up.
> 
> That just means Debian's automatic build for the kernel has been broken for 
> years. All normal config targets require user input and no input equals 
> default input. Only silentoldconfig will abort if input is not available.

I think that's broken (because I don't see where that behavior is
described). IMO, based on the code, it should only go with defaults when
-n -y or -m is passed. Anything else should detect when stdin is not
valid and abort. If stdin is valid, and empty string is read, then
that's a valid "give me the default" response.

Why is it so hard to error when stdin is closed? It's not like that will
break anything.

Since silentoldconfig does this, then oldconfig should aswell. The only
naming difference is that silentoldconfig is quiet, and oldconfig is
not. Why should the two act any differently with a closed stdin?

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-09 13:42               ` Ben Collins
@ 2006-01-11 23:26                 ` Roman Zippel
  2006-01-12  2:00                   ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-11 23:26 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Mon, 9 Jan 2006, Ben Collins wrote:

> > That just means Debian's automatic build for the kernel has been broken for 
> > years. All normal config targets require user input and no input equals 
> > default input. Only silentoldconfig will abort if input is not available.
> 
> I think that's broken (because I don't see where that behavior is
> described).

I'll accept a patch to fix the documentation.

> IMO, based on the code, it should only go with defaults when
> -n -y or -m is passed.

No.

> Why is it so hard to error when stdin is closed? It's not like that will
> break anything.

oldconfig & co are interactive targets, so don't use them in automatic 
builds. If you some problem with using silentoldconfig, describe it and 
I'll help to solve it.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-11 23:26                 ` Roman Zippel
@ 2006-01-12  2:00                   ` Ben Collins
  2006-01-12 11:08                     ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-12  2:00 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Thu, 2006-01-12 at 00:26 +0100, Roman Zippel wrote:
> Hi,
> 
> On Mon, 9 Jan 2006, Ben Collins wrote:
> 
> > > That just means Debian's automatic build for the kernel has been broken for 
> > > years. All normal config targets require user input and no input equals 
> > > default input. Only silentoldconfig will abort if input is not available.
> > 
> > I think that's broken (because I don't see where that behavior is
> > described).
> 
> I'll accept a patch to fix the documentation.
> 
> > IMO, based on the code, it should only go with defaults when
> > -n -y or -m is passed.
> 
> No.
> 
> > Why is it so hard to error when stdin is closed? It's not like that will
> > break anything.
> 
> oldconfig & co are interactive targets, so don't use them in automatic 
> builds. If you some problem with using silentoldconfig, describe it and 
> I'll help to solve it.

First, we need oldconfig because it allows us to look at the build log
and see exactly what happened in the config stage. Silentoldconfig gives
us no feedback for logs.

Now let me see if I get this right:

1) oldconfig was broken when stdin was closed. Meaning, it went into an
infinite loop. This was obviously not the intended outcome.

2) silentoldconfig will, when faced with a closed stdin, abort, and
notify the user.

3) Obviously since current behavior of oldconfig was broken with a
closed stdin, then it was never doing what anyone wanted in this usage
case. Since no one else noticed it, that means that we are the only use
case for this.

4) I send a patch that fixes oldconfig with closed stdin by making it
duplicate what silentoldconfig does, aborting when it needs input and
stdin is closed. Since oldconfig was always broken in this usage case,
this seems the obvious fix, plus it's consistent. This isn't the same as
an empty string (e.g. when hitting enter), which didn't get changed, and
has always meant to use the default value.

5) My patch did not break anything, nor did it change anything that was
already working.

6) In response you make oldconfig work exactly opposite of
silentoldconfig by using the default value for a config option when
stdin is closed (basically acting like the user hit ENTER), and further
break things for me in this usage case, with no purpose, and no reason
for making your change the way you did. Since it was broken, you aren't
helping anyone. We can't have the build system using default values. We
need it to abort.

Did I get this right, or am I imagining that you are being hard headed
about this?

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12  2:00                   ` Ben Collins
@ 2006-01-12 11:08                     ` Roman Zippel
  2006-01-12 12:27                       ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-12 11:08 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Wed, 11 Jan 2006, Ben Collins wrote:

> First, we need oldconfig because it allows us to look at the build log
> and see exactly what happened in the config stage. Silentoldconfig gives
> us no feedback for logs.

silentoldconfig gives you exactly the same information. Both conf and 
oldconfig will change invisible options without telling you, so it's not 
exact at all.
If you can't trust a silent oldconfig, a more verbose oldconfig can't tell 
you anything else, if it would it's a bug.

> 3) Obviously since current behavior of oldconfig was broken with a
> closed stdin, then it was never doing what anyone wanted in this usage
> case. Since no one else noticed it, that means that we are the only use
> case for this.

This is enough reason to simply hijack conf and use it for your own 
purpose without even asking the maintainer about the intended bahaviour?

> 5) My patch did not break anything, nor did it change anything that was
> already working.

It _was_ working like that, you're breaking it.

> 6) In response you make oldconfig work exactly opposite of
> silentoldconfig by using the default value for a config option when
> stdin is closed (basically acting like the user hit ENTER), and further
> break things for me in this usage case, with no purpose, and no reason
> for making your change the way you did. Since it was broken, you aren't
> helping anyone. We can't have the build system using default values. We
> need it to abort.

So simply use silentoldconfig.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 11:08                     ` Roman Zippel
@ 2006-01-12 12:27                       ` Ben Collins
  2006-01-12 12:48                         ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-12 12:27 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Thu, 2006-01-12 at 12:08 +0100, Roman Zippel wrote:
> Hi,
> 
> On Wed, 11 Jan 2006, Ben Collins wrote:
> 
> > First, we need oldconfig because it allows us to look at the build log
> > and see exactly what happened in the config stage. Silentoldconfig gives
> > us no feedback for logs.
> 
> silentoldconfig gives you exactly the same information. Both conf and 
> oldconfig will change invisible options without telling you, so it's not 
> exact at all.
> If you can't trust a silent oldconfig, a more verbose oldconfig can't tell 
> you anything else, if it would it's a bug.

silentoldconfig tells you a lot less, agreed? I want the verbose
oldconfig, and I want it to fail on closed stdin when it needs input.

> > 3) Obviously since current behavior of oldconfig was broken with a
> > closed stdin, then it was never doing what anyone wanted in this usage
> > case. Since no one else noticed it, that means that we are the only use
> > case for this.
> 
> This is enough reason to simply hijack conf and use it for your own 
> purpose without even asking the maintainer about the intended bahaviour?

Hijack? It was broken, correct? It has always been broken. This problem
has existed for as long as I've been handling kernel builds with Debian
(which seems to be about 6-7 years now). So intended behavior aside, it
has never worked as intended.

> > 5) My patch did not break anything, nor did it change anything that was
> > already working.
> 
> It _was_ working like that, you're breaking it.

At what point did oldconfig use default values when stdin was closed?
Show me the code that actually did this. Yes, it has always used default
values when it was empty input, just like silentoldconfig does. But
silentoldconfig aborts when stdin is closed, why should oldconfig?

> > 6) In response you make oldconfig work exactly opposite of
> > silentoldconfig by using the default value for a config option when
> > stdin is closed (basically acting like the user hit ENTER), and further
> > break things for me in this usage case, with no purpose, and no reason
> > for making your change the way you did. Since it was broken, you aren't
> > helping anyone. We can't have the build system using default values. We
> > need it to abort.
> 
> So simply use silentoldconfig.

That's not the usage I want. Show me where and when oldconfig worked as
you say it was intended to work. And then explain why oldconfig is
intended to be inconsistent with silentoldconfig.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 12:27                       ` Ben Collins
@ 2006-01-12 12:48                         ` Roman Zippel
  2006-01-12 13:31                           ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-12 12:48 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Thu, 12 Jan 2006, Ben Collins wrote:

> > silentoldconfig gives you exactly the same information. Both conf and 
> > oldconfig will change invisible options without telling you, so it's not 
> > exact at all.
> > If you can't trust a silent oldconfig, a more verbose oldconfig can't tell 
> > you anything else, if it would it's a bug.
> 
> silentoldconfig tells you a lot less, agreed?

No.

> Hijack? It was broken, correct? It has always been broken. This problem
> has existed for as long as I've been handling kernel builds with Debian
> (which seems to be about 6-7 years now). So intended behavior aside, it
> has never worked as intended.

And it took you 6 years to report this problem?

> > > 5) My patch did not break anything, nor did it change anything that was
> > > already working.
> > 
> > It _was_ working like that, you're breaking it.
> 
> At what point did oldconfig use default values when stdin was closed?

It broke with this patch:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commit;h=4b518e42f97b96abd84f5106e43711dbff3c5707

> > So simply use silentoldconfig.
> 
> That's not the usage I want.

You only want some specific output, which has zero advantage over the 
silentoldconfig output.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 12:48                         ` Roman Zippel
@ 2006-01-12 13:31                           ` Ben Collins
  2006-01-12 14:00                             ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-12 13:31 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Thu, 2006-01-12 at 13:48 +0100, Roman Zippel wrote:
> Hi,
> 
> On Thu, 12 Jan 2006, Ben Collins wrote:
> 
> > > silentoldconfig gives you exactly the same information. Both conf and 
> > > oldconfig will change invisible options without telling you, so it's not 
> > > exact at all.
> > > If you can't trust a silent oldconfig, a more verbose oldconfig can't tell 
> > > you anything else, if it would it's a bug.
> > 
> > silentoldconfig tells you a lot less, agreed?
> 
> No.

So you are saying that silentoldconfig outputs no less information than
oldconfig? No output compared to a full config output (yes, with some
special cased invisible options, but the same output that a user would
see if manually configuring).

My point is that you are making oldconfig and silentoldconfig operate
differently when they encounter a closed stdin. You are making them
inconsistent. And so far, you have yet to give a valid reason to do so.
I've been giving very valid reasons why they should work the same, and
why the behavior is correct for them to work that way.

What is the reason for silentoldconfig to fail in this way, and for
oldconfig not to? I suspect you have some special usage of your own for
which you depend on this functionality.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 13:31                           ` Ben Collins
@ 2006-01-12 14:00                             ` Roman Zippel
  2006-01-12 14:16                               ` Ben Collins
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-12 14:00 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Thu, 12 Jan 2006, Ben Collins wrote:

> > > silentoldconfig tells you a lot less, agreed?
> > 
> > No.
> 
> So you are saying that silentoldconfig outputs no less information than
> oldconfig? No output compared to a full config output (yes, with some
> special cased invisible options, but the same output that a user would
> see if manually configuring).

You're seriously telling me that you check every line of the oldconfig 
output in case of the problem? 
If it makes you feel better, you can rerun oldconfig after the 
silentoldconfig, but the output is practically useless.
It would actually be a lot better if you ran a diff between the old config 
and the new config and add this to the build output, only the contents of 
the .config file is relevant to kbuild and if something went wrong, the 
real differences would be easily visible.

> My point is that you are making oldconfig and silentoldconfig operate
> differently when they encounter a closed stdin. You are making them
> inconsistent. And so far, you have yet to give a valid reason to do so.
> I've been giving very valid reasons why they should work the same, and
> why the behavior is correct for them to work that way.

Even if they sound similiar they are not the same. e.g. I'm working on 
patches to integrate split config step, so it will do a bit more than 
normal config targets (but it remain a valid make target). The 
silentoldconfig target is an automatic target which is also used by kbuild 
to verify the config consistency.
The situation is very simple, we have automatic config targets (like 
silentoldconfig or all*config) and we have interactive config targets 
(like config, xconfig, oldconfig).
I'm very much interested to improve the situation of the automatic 
targets to help automatic builds, but just printing useless information 
adds no value. If you don't trust that silentoldconfig does the right 
thing, you can't trust oldconfig either.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 14:00                             ` Roman Zippel
@ 2006-01-12 14:16                               ` Ben Collins
  2006-01-13 17:44                                 ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Ben Collins @ 2006-01-12 14:16 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel

On Thu, 2006-01-12 at 15:00 +0100, Roman Zippel wrote:
> > My point is that you are making oldconfig and silentoldconfig operate
> > differently when they encounter a closed stdin. You are making them
> > inconsistent. And so far, you have yet to give a valid reason to do so.
> > I've been giving very valid reasons why they should work the same, and
> > why the behavior is correct for them to work that way.
> 
> Even if they sound similiar they are not the same. e.g. I'm working on 
> patches to integrate split config step, so it will do a bit more than 
> normal config targets (but it remain a valid make target). The 
> silentoldconfig target is an automatic target which is also used by kbuild 
> to verify the config consistency.
> The situation is very simple, we have automatic config targets (like 
> silentoldconfig or all*config) and we have interactive config targets 
> (like config, xconfig, oldconfig).
> I'm very much interested to improve the situation of the automatic 
> targets to help automatic builds, but just printing useless information 
> adds no value. If you don't trust that silentoldconfig does the right 
> thing, you can't trust oldconfig either.

What I don't understand is that if oldconfig is an interactive target,
why make it work when interactivity is not available? Also, if
silentoldconfig is an automated target, why make it abort when
interactivity is not available?

Just seems backwards.

For me, silentoldconfig could work, but I prefer the verbosity of
oldconfig. It's just more convenient. It's not that things are checked
in minute detail, but that our builds may eventually disappear (by
upgraded packages), but our build logs remain. It's the only way we have
to go back and check regressions in the build process (it has saved us
many times).

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux


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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-12 14:16                               ` Ben Collins
@ 2006-01-13 17:44                                 ` Roman Zippel
  0 siblings, 0 replies; 22+ messages in thread
From: Roman Zippel @ 2006-01-13 17:44 UTC (permalink / raw)
  To: Ben Collins; +Cc: linux-kernel

Hi,

On Thu, 12 Jan 2006, Ben Collins wrote:

> What I don't understand is that if oldconfig is an interactive target,
> why make it work when interactivity is not available?

It means it will accept any input and no input (by just pressing enter or 
ctrl-d) means the default answer.

> Also, if
> silentoldconfig is an automated target, why make it abort when
> interactivity is not available?

It's called during a normal kbuild and must work in any situation.

> For me, silentoldconfig could work, but I prefer the verbosity of
> oldconfig. It's just more convenient. It's not that things are checked
> in minute detail, but that our builds may eventually disappear (by
> upgraded packages), but our build logs remain. It's the only way we have
> to go back and check regressions in the build process (it has saved us
> many times).

The output is unlikely to help you, you'll still have the .config file and 
that is a much better source to verify the configuration step.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-19 12:50                               ` Bodo Eggert
@ 2006-01-19 12:55                                 ` Roman Zippel
  0 siblings, 0 replies; 22+ messages in thread
From: Roman Zippel @ 2006-01-19 12:55 UTC (permalink / raw)
  To: Bodo Eggert; +Cc: Ben Collins, linux-kernel

Hi,

On Thu, 19 Jan 2006, Bodo Eggert wrote:

> > That would be ^C. ^D means end-of-file and it's up to the application, 
> > whether it needs further information to continue or not.
> 
> If it does need data, it SHOULD NOT invent it, especially if inventing
> this data has no benefit and causes unexpected beahviour and random 
> breakage.

It doesn't "invent" anything, so I don't see your point.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-19 11:52                             ` Roman Zippel
@ 2006-01-19 12:50                               ` Bodo Eggert
  2006-01-19 12:55                                 ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Bodo Eggert @ 2006-01-19 12:50 UTC (permalink / raw)
  To: Roman Zippel; +Cc: 7eggert, Ben Collins, linux-kernel

On Thu, 19 Jan 2006, Roman Zippel wrote:
> On Wed, 18 Jan 2006, Bodo Eggert wrote:

> > > It means it will accept any input and no input (by just pressing enter or
> > > ctrl-d) means the default answer.
> > 
> > If I press ^D, I'd expect to select the abort option
> 
> That would be ^C. ^D means end-of-file and it's up to the application, 
> whether it needs further information to continue or not.

If it does need data, it SHOULD NOT invent it, especially if inventing
this data has no benefit and causes unexpected beahviour and random 
breakage.

-- 
Top 100 things you don't want the sysadmin to say:
66. What do you mean you needed that directory?

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
  2006-01-18 21:51                           ` Bodo Eggert
@ 2006-01-19 11:52                             ` Roman Zippel
  2006-01-19 12:50                               ` Bodo Eggert
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Zippel @ 2006-01-19 11:52 UTC (permalink / raw)
  To: 7eggert; +Cc: Ben Collins, linux-kernel

Hi,

On Wed, 18 Jan 2006, Bodo Eggert wrote:

> > It means it will accept any input and no input (by just pressing enter or
> > ctrl-d) means the default answer.
> 
> If I press ^D, I'd expect to select the abort option

That would be ^C. ^D means end-of-file and it's up to the application, 
whether it needs further information to continue or not.

bye, Roman

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

* Re: [PATCH 15/15] kconf: Check for eof from input stream.
       [not found]                         ` <5uBdT-2Gn-23@gated-at.bofh.it>
@ 2006-01-18 21:51                           ` Bodo Eggert
  2006-01-19 11:52                             ` Roman Zippel
  0 siblings, 1 reply; 22+ messages in thread
From: Bodo Eggert @ 2006-01-18 21:51 UTC (permalink / raw)
  To: Roman Zippel, Ben Collins, linux-kernel

Roman Zippel <zippel@linux-m68k.org> wrote:
> On Thu, 12 Jan 2006, Ben Collins wrote:

>> What I don't understand is that if oldconfig is an interactive target,
>> why make it work when interactivity is not available?
> 
> It means it will accept any input and no input (by just pressing enter or
> ctrl-d) means the default answer.

If I press ^D, I'd expect to select the abort option (like e.g. all sane
GUIs do if you close yes/no/abort dialogs instead of selecting a button).
I can see no advantage from implementing this unexpected behaviour.

<rant>
I don't understand programmers trying to go ahead even if they know they're
going the wrong way. The header file can't be found - let's see if it wasn't
needed, maybe the programmer inserted it just for fun! There is an unknown
argument '--version'? Just ignore it and start the operation, it won't have
been that important! *grrrrrr*
</rant>
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

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

end of thread, other threads:[~2006-01-19 12:55 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-04 22:01 [PATCH 15/15] kconf: Check for eof from input stream Ben Collins
2006-01-08 16:34 ` Roman Zippel
2006-01-08 18:53   ` Ben Collins
2006-01-08 20:59     ` Roman Zippel
2006-01-08 21:41       ` Ben Collins
2006-01-09  0:09         ` Roman Zippel
2006-01-09  3:59           ` Ben Collins
2006-01-09 11:32             ` Roman Zippel
2006-01-09 13:42               ` Ben Collins
2006-01-11 23:26                 ` Roman Zippel
2006-01-12  2:00                   ` Ben Collins
2006-01-12 11:08                     ` Roman Zippel
2006-01-12 12:27                       ` Ben Collins
2006-01-12 12:48                         ` Roman Zippel
2006-01-12 13:31                           ` Ben Collins
2006-01-12 14:00                             ` Roman Zippel
2006-01-12 14:16                               ` Ben Collins
2006-01-13 17:44                                 ` Roman Zippel
     [not found] <5roZI-5y9-29@gated-at.bofh.it>
     [not found] ` <5sSVt-5Du-1@gated-at.bofh.it>
     [not found]   ` <5sWwg-2Bq-21@gated-at.bofh.it>
     [not found]     ` <5t4kf-5Px-11@gated-at.bofh.it>
     [not found]       ` <5t5zv-7GD-31@gated-at.bofh.it>
     [not found]         ` <5tXA1-3Lh-35@gated-at.bofh.it>
     [not found]           ` <5u04G-7s6-19@gated-at.bofh.it>
     [not found]             ` <5u8Yt-317-41@gated-at.bofh.it>
     [not found]               ` <5u9L8-4gd-19@gated-at.bofh.it>
     [not found]                 ` <5uadH-4TM-1@gated-at.bofh.it>
     [not found]                   ` <5uaQp-5UL-7@gated-at.bofh.it>
     [not found]                     ` <5ubjI-6KH-21@gated-at.bofh.it>
     [not found]                       ` <5ubtB-6Xy-9@gated-at.bofh.it>
     [not found]                         ` <5uBdT-2Gn-23@gated-at.bofh.it>
2006-01-18 21:51                           ` Bodo Eggert
2006-01-19 11:52                             ` Roman Zippel
2006-01-19 12:50                               ` Bodo Eggert
2006-01-19 12:55                                 ` Roman Zippel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).