Hi Keith.
I don't agree with your analysis in the first block quoted below, but
that's not relevant as I feel there's a better solution than either of
the ones proposed so far. See below for details...
>> 1. Adam's point is that there are dep_* statements in the config
>> setup that have been used to say that a particular option is
>> dependant upon a particular architecture, but this doesn't work.
>>
>> 3. MY understanding of the situation is that ALL architecture
>> specific config lines are EXPECTED to be in the arch/*/config.in
>> files, where they will only even be seen when the relevant
>> architecture is being compiled for.
>>
>> As a result of this, I would summarise this discussion as saying
>> that there is a bug in the kernel config scripts in that some
>> options that should be located in the architecture-specific
>> config files are in the all-architecture config files instead.
> (1) and (3) are correct but your conclusion is not. The problem is
>
> dep_tristate CONFIG_some_driver $CONFIG_some_arch
>
> where the intention is to allow the driver only if some_arch is
> set.
Can I suggest you re-read your comment and the points you quoted and
said are correct. As a DIRECT logical consequence of "(1) and (3) are
correct" (as you phrased it), we get the conclusion that any config
lines that depend on "if some arch is set" (as you phrased it) are in
the architecture-specific config files. This leads DIRECTLY to the
conclusion that ANY lines dependant on a particular architecture that
are not in the architecture-specific config files are a bug in the
kernel config scripts.
=======================================================================
Personally, I would suggest that the bug in this case is the actual
assumption that (3) is true, and the correct course of action is to
remove that assumption as it leads directly to the spaghetti in the
configuration system that we currently have.
> When you compile for some_other_arch, CONFIG_some_arch is
> undefined. dep_tristate treats undefined variables as "don't
> care"...
Agreed - and whoever did the config lines where the dependancy is an
arch variable made the mistake of misunderstanding that.
> ...and we cannot fix that without changing bash or a major
> rewrite of the shell scripts.
Why is either needed? As I see it, the cure is to define a pair of new
statement types in the config language, as follows:
1. dep_arch_bool CONFIG_var CONFIG_arch [CONFIG_other_var...]
This states that CONFIG_var is a boolean var that depends both on
CONFIG_arch being specifically "y" and on CONFIG_other_var being
such as to satisfy dep_bool as currently.
2. dep_arch_tristate CONFIG_var CONFIG_arch [CONFIG_other_var...]
This is the tristate version of dep_arch_bool as expected.
When we've done that, ALL of the problem lines can be trivially
converted from dep_ to dep_arch_ and the problem vanishes. As a free
bonus, the need for the arch-specific config files vanishes as well,
and each option can be documented as to what architectures it is valid
for. This thus removes a possible source of bugs from the equation.
The enclosed patch (against 2.4.5 raw) adds these two statements to
both the `make config` and `make menuconfig` scripts. I don't
understand TCL/TK so can't add them to the `make xconfig` script, and
as I also don't understand PERL, I can't add it to `make checkconfig`
so somebody else will need to deal with those two scripts.
> There are two solutions, either change all such lines to
>
> if [ "$CONFIG_some_arch" = "y" ];then
> tristate CONFIG_some_driver
> fi
>
> or
>
> define_bool CONFIG_some_arch n
>
> for all architectures at the start, followed by turning on the
> one that is required.
Both are messy, and best described as kludges. I wouldn't support
either.
> Lots of if statements are messy and they will not prevent
> somebody adding new options with exactly the same problem.
Agreed.
> Explicitly setting all but one arch variable to 'n' results in
> cleaner config scripts and new arch dependent driver settings
> will automatically work.
Until somebody adds a new port and fails to upgrade any one of the
various files doing this. That's an open recipe for bugs IMHO, and
should be avoided at all costs.
Best wishes from Riley.
On Mon, 2 Jul 2001 00:04:22 +0100 (BST),
Riley Williams <[email protected]> wrote:
>Can I suggest you re-read your comment and the points you quoted and
>said are correct. As a DIRECT logical consequence of "(1) and (3) are
>correct" (as you phrased it), we get the conclusion that any config
>lines that depend on "if some arch is set" (as you phrased it) are in
>the architecture-specific config files. This leads DIRECTLY to the
>conclusion that ANY lines dependant on a particular architecture that
>are not in the architecture-specific config files are a bug in the
>kernel config scripts.
No. We want network drivers to be in the network menu, SCSI drivers in
the SCSI menu etc. Some of those drivers are restricted to some
architectures but putting them in the arch config splits the logical
flow of selection and duplicates text. Look at the repeated SCSI code
in some of the arch configs.
> 2. dep_arch_tristate CONFIG_var CONFIG_arch [CONFIG_other_var...]
dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A $CONFIG_ARCH_ACORN $CONFIG_NET_ETHERNET
fails your code. $CONFIG_ARCH_ACORN is undefined, $CONFIG_NET_ETHERNET
is 'y'. dep_arch_tristate is invoked with 3 (not 4 parameters), text,
CONFIG_ARM_AM79C961A and 'y'. The intervening undefined variable
disappears in shell scripts.
Does anyone know if there is any code that would break if
we put quotation marks around the $CONFIG_xxxx references in the
dep_xxx commands in all of the Config.in files? In other words,
change all commands of the form
dep_tristate CONFIG_FOO 'Foo on x86/pci machines' $CONFIG_PCI $CONFIG_X86
to
dep_tristate CONFIG_FOO 'Foo on x86/pci machines' "$CONFIG_PCI" "$CONFIG_X86"
Then, we could change dep_{bool,tristate} to only treat "" as "n",
in its dependency parameters without effecting how undefined variables
are treated elsewhere. For example, CONFIG_FOO being undefined would
still cause "make oldconfig" to treat it as "NEW" and ask the user
about it.
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
On Sun, 1 Jul 2001 19:25:11 -0700,
"Adam J. Richter" <[email protected]> wrote:
> Does anyone know if there is any code that would break if
>we put quotation marks around the $CONFIG_xxxx references in the
>dep_xxx commands in all of the Config.in files?
That has the same problem that AC was worried about. Variables that
used to be treated as "undefined, don't care" are now treated as
"undefined, assume n and forbid".
As long as there is any ambiguity about how a rule is meant to treat
undefined variables, treating all undefined variables as 'n' is not
safe. Before making a global change like this, first verify that no
rule treats undefined variables as "don't care". Otherwise something
will break.
Keith Owens <[email protected]> writes:
>On Sun, 1 Jul 2001 19:25:11 -0700,
>"Adam J. Richter" <[email protected]> wrote:
>> Does anyone know if there is any code that would break if
>>we put quotation marks around the $CONFIG_xxxx references in the
>>dep_xxx commands in all of the Config.in files?
>That has the same problem that AC was worried about. Variables that
>used to be treated as "undefined, don't care" are now treated as
>"undefined, assume n and forbid".
What variables? Please show me a real example.
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
On Sun, 1 Jul 2001 21:46:04 -0700,
"Adam J. Richter" <[email protected]> wrote:
>Can you even write a hypothetical example?
if [ "$CONFIG_foo" = "n" -a "$CONFIG_bar" = "n" ]; then
define_bool "$CONFIG_ALLOW_foo_bar n
fi
....
dep_tristate CONFIG_baz $CONFIG_ALLOW_foo_bar
>>>> Does anyone know if there is any code that would break if
>>>>we put quotation marks around the $CONFIG_xxxx references in the
>>>>dep_xxx commands in all of the Config.in files?
>>>That has the same problem that AC was worried about. Variables that
>>>used to be treated as "undefined, don't care" are now treated as
>>>"undefined, assume n and forbid".
>> What variables? Please show me a real example.
>Not my job. If you want to make a global change to the meaning of
>undefined config variables, it is your responsibility to show that the
>change has no unwanted side effects. Assuming there are no side
>effects is unsatisfactory in a stable kernel, especially with all the
>config variables in patch sets outside the kernel.
That's not reasonable in the face of what appears to be
completely fabricated myth. You generally can't prove this
kind of a negative for anything. Either you or Alan Cox should
show an example. Can you even show me the code that such an
example would exercise. Can you even write a hypothetical example?
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
>On Sun, 1 Jul 2001 21:46:04 -0700,
>"Adam J. Richter" <[email protected]> wrote:
>>Can you even write a hypothetical example?
>if [ "$CONFIG_foo" = "n" -a "$CONFIG_bar" = "n" ]; then
> define_bool "$CONFIG_ALLOW_foo_bar n
>fi
>....
>dep_tristate CONFIG_baz $CONFIG_ALLOW_foo_bar
In linux-2.4.6-pre8, there are only three configuration variables
that are defined with an indented 'define_bool' statement
(CONFIG_BLK_DEV_IDE{DMA,PCI}, and CONFIG_PCI), and the conditional
code execute by all "if" statements in all of the config.in files
appears to be indented (or at least the first statement in the block
is indented). None of these three variables has the semantics that
I think you you described above.
If you want to check, I determined this by the following shell
commands:
% find . -iname config.in | xargs egrep dep_tristate | tr ' ' '\n\n' | egrep '^\$CONFIG_' | sort -u > /tmp/config-dependencies
% find . -iname config.in | xargs egrep '^[ ].*define_bool' | fgrep -f /tmp/config-dependencies | awk '{print $(NF-1)}' | sort -u
CONFIG_BLK_DEV_IDEDMA
CONFIG_BLK_DEV_IDEPCI
CONFIG_PCI
% find /usr/src/linux/ -iname config.in | xargs egrep -A 2 ^if | egrep -v -e -- | egrep '^-[^ ]'
%
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
I have minor correction to make. I wrote:
> In linux-2.4.6-pre8, there are only three configuration variables
>that are defined with an indented 'define_bool' statement
>(CONFIG_BLK_DEV_IDE{DMA,PCI}, and CONFIG_PCI),
I meant three configuration variables that are defined with an
indented 'define_bool' statement _and are used as arguments to
a dep_* command_.
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
On Sun, 1 Jul 2001 22:52:04 -0700,
"Adam J. Richter" <[email protected]> wrote:
> In linux-2.4.6-pre8, there are only three configuration variables
>that are defined with an indented 'define_bool' statement
>(CONFIG_BLK_DEV_IDE{DMA,PCI}, and CONFIG_PCI), and the conditional
>code execute by all "if" statements in all of the config.in files
>appears to be indented (or at least the first statement in the block
>is indented). None of these three variables has the semantics that
>I think you you described above.
A conditioned define_bool is not the only possibility, that was just
one example. Undefined variables can be created by any of the ask,
define, dependent or choice statements listed in
Documentation/kbuild/config-language.txt, when that statement is not
executed. The reason for not being executed can be an 'if' statement
or the code can be in a file that is not read, either the 'source'
statement is bypassed or it is for another architecture. Also the
unset statement changes variables to undefined values.
Identifying where variables can have unset values cannot be done by a
grep of the files. You have to follow every branch of the condition
tree, including source statements, eliminate those variables where
every branch assigns a value before it is used then flag the
remainder as "may be used while uninitialized".
Hi Keith.
>> Can I suggest you re-read your comment and the points you quoted
>> and said are correct. As a DIRECT logical consequence of "(1)
>> and (3) are correct" (as you phrased it), we get the conclusion
>> that any config lines that depend on "if some arch is set" (as
>> you phrased it) are in the architecture-specific config files.
>> This leads DIRECTLY to the conclusion that ANY lines dependant
>> on a particular architecture that are not in the
>> architecture-specific config files are a bug in the kernel
>> config scripts.
> No. We want network drivers to be in the network menu, SCSI
> drivers in the SCSI menu etc. Some of those drivers are
> restricted to some architectures but putting them in the arch
> config splits the logical flow of selection and duplicates text.
> Look at the repeated SCSI code in some of the arch configs.
That's a direct consequence of the policy quoted in paragraph (3) and
NOT an error in the logic. It's why I'm against the current policy.
>> 2. dep_arch_tristate CONFIG_var CONFIG_arch [CONFIG_other_var...]
> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
> $CONFIG_ARCH_ACORN $CONFIG_NET_ETHERNET
> fails your code. $CONFIG_ARCH_ACORN is undefined,
> $CONFIG_NET_ETHERNET is 'y'. dep_arch_tristate is invoked with
> 3 (not 4 parameters), text, CONFIG_ARM_AM79C961A and 'y'. The
> intervening undefined variable disappears in shell scripts.
You're not thinking clearly. Try the following simple fix to that:
Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
Q> "$CONFIG_ARCH_ACORN" $CONFIG_NET_ETHERNET
That adds only two extra characters, neither conspicuous, and PASSES
my code.
Best wishes from Riley.
Hi Keith, Adam.
>> Does anyone know if there is any code that would break if we
>> put quotation marks around the $CONFIG_xxxx references in the
>> dep_xxx commands in all of the Config.in files?
> That has the same problem that AC was worried about. Variables
> that used to be treated as "undefined, don't care" are now
> treated as "undefined, assume n and forbid".
Whilst there could easily be problems if we allow that for any of the
variables, it can't be a problem if we restrict it to variables
specifying the architecture in question, as per my previous email.
> As long as there is any ambiguity about how a rule is meant to
> treat undefined variables, treating all undefined variables as
> 'n' is not safe. Before making a global change like this, first
> verify that no rule treats undefined variables as "don't care".
> Otherwise something will break.
Agreed.
Best wishes from Riley.
On Mon, 2 Jul 2001 08:16:50 +0100 (BST),
Riley Williams <[email protected]> wrote:
> Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
> Q> "$CONFIG_ARCH_ACORN" $CONFIG_NET_ETHERNET
>
>That adds only two extra characters, neither conspicuous, and PASSES
>my code.
It relies on everybody writing new dep_arch_... rules to remember the
quotes. You are just introducing another source of human error. That
is how this mess occurred, no automatic validation of input.
Hi Keith.
>> Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
>> Q> "$CONFIG_ARCH_ACORN" $CONFIG_NET_ETHERNET
>> That adds only two extra characters, neither conspicuous, and
>> PASSES my code.
> It relies on everybody writing new dep_arch_... rules to
> remember the quotes. You are just introducing another source of
> human error. That is how this mess occurred, no automatic
> validation of input.
OK then, a simple change to the patch I submitted, and the following
instead...
Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
Q> ACORN $CONFIG_NET_ETHERNET
The enclosed patch implements this syntax, and CHECKS that the syntax
stated has been supplied, for both `make config` and `make menuconfig`
and I would imagine the same would be easily added to `make xconfig`
and `make checkconfig` as well by those fluent in the relevant
languages.
Note specifically that it requires that the CONFIG_ARCH_ part of the
name of the variable is NOT provided, and as a result requires that
config variables naming the architecture begin with that string.
Best wishes from Riley.
Keith,
Copping out with "not my job" and then demanding essentially a
mathematical proof of correctness is unpersuasive, but I have no
objection to your change (some select "define_bool xxx no" lines in
a common file that every architecture reads), as long as we have
established that it seems to get along with "make xconfig" after all.
If that is the case, then I'd be just as happy that Linus apply your patch.
So, with that lame endorsement and nobody else still objecting
to your patch (right?), how about if you submit your patch to Linus,
or is there some other way you want to proceed?
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
Keith Owens wrote:
> On Sun, 1 Jul 2001 21:46:04 -0700,
> "Adam J. Richter" <[email protected]> wrote:
> >Can you even write a hypothetical example?
>
> if [ "$CONFIG_foo" = "n" -a "$CONFIG_bar" = "n" ]; then
> define_bool "$CONFIG_ALLOW_foo_bar n
> fi
> ....
> dep_tristate CONFIG_baz $CONFIG_ALLOW_foo_bar
Unfortunately this does not work as you expect (depends on history) if
any of CONFIG_foo CONFIG_bar can be changed.
When changing one of them to y, the value of CONFIG_ALLOW_foo_bar in
make menuconfig / make xconfig is not undefined...
Andrzej
--
=======================================================================
Andrzej M. Krzysztofowicz [email protected]
phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math., Technical University of Gdansk
On Mon, Jul 02, 2001 at 09:25:50AM +0100, Riley Williams wrote:
> Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
> Q> ACORN $CONFIG_NET_ETHERNET
Before we go and create a patch for Linus to apply, please note that the
above is totally bogus and is in fact 100% wrong. Don't create a patch
yourself. Let me know how you propose to do it, and I will create the
patch using the correct symbols.
Also note that the majority of the machine-dependent symbols for StrongARM
platforms (of which there are around 43) start CONFIG_SA1100_*, not
CONFIG_ARCH_*. Unfortunately, its far too late to get around this
special case (I'm not too happy that we have this special case either,
so don't whinge at me please).
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
> appears to be indented (or at least the first statement in the block
> is indented). None of these three variables has the semantics that
> I think you you described above.
Ok so that just leaves fixing any Xconfig/menuconfig versions of the change
and it can go in
Excellent
Hi Russell.
>> Q> dep_arch_tristate ' AM79C961A support' CONFIG_ARM_AM79C961A \
>> Q> ACORN $CONFIG_NET_ETHERNET
> Before we go and create a patch for Linus to apply...
Who's sent a patch to Linus? I haven't, and don't intend to do so.
> ...please note that the above is totally bogus and is in fact
> 100% wrong.
Please explain? I don't see the problem with the proposed syntax, and
presume you have noticed that it's a new statement type, not a
modification to an existing one?
> Don't create a patch yourself. Let me know how you propose to do
> it, and I will create the patch using the correct symbols.
OK, here's the proposal:
1. Define new config statements that take as their parameters the
following:
1. The prompt text.
2. The name of the config variable to set.
3. The NAME of the config variable that defines the
required architecture.
4. The VALUES of all other config variables this variable
depends on.
2. Define the new `dep_arch_bool` statement to be the same as
`dep_bool` if passed items 1, 2 and 4 when the config var
named in item 3 has the value "y". When the config var named
has any other value, it becomes `define_bool "$2" "n" instead.
3. Define the new `dep_arch_tristate` similarly.
> Also note that the majority of the machine-dependent symbols for
> StrongARM platforms (of which there are around 43) start
> CONFIG_SA1100_*, not CONFIG_ARCH_*. Unfortunately, its far too
> late to get around this special case (I'm not too happy that we
> have this special case either, so don't whinge at me please).
First, why is it "far too late" as you put it? It won't be the first
time config vars have been renamed, and it's unlikely to be the last
either...
Secondly, that isn't a problem to this proposal. The reason for taking
the CONFIG_ARCH_ out of the syntax is to emphasise that it's the name
and not the value that goes in there, thus reducing the likelihood of
problems creeping in in the first place.
Best wishes from Riley.
On Mon, Jul 02, 2001 at 01:40:25PM +0100, Riley Williams wrote:
> Who's sent a patch to Linus? I haven't, and don't intend to do so.
The way this thread is progressing, it won't take much to get there. We've
already propagated the inaccuracies in the "example" depencencies by 3 or
more mails. I'm just trying to stop it before someone forgets that it's
just an example.
> First, why is it "far too late" as you put it? It won't be the first
> time config vars have been renamed, and it's unlikely to be the last
> either...
I'm not going to cause disruption across the board to lots of people just
because someone wants to keep the length of the symbols down.
If you really do want to do this change, then I suggest that you get in
touch with Nicolas Pitre and discuss it with him. When you come to a
conclusion, its not as simple as patching the kernel. You need to
update the database at http://www.arm.linux.org.uk/developer/machines/ to
reflect the new symbols _at the same time_ that you change them in
everyones tree, since anyone can pull down a new copy of the symbols
from the database at any time.
IMHO, a logistical nightmare to perform.
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On Mon, 2 Jul 2001, Russell King wrote:
> > First, why is it "far too late" as you put it? It won't be the first
> > time config vars have been renamed, and it's unlikely to be the last
> > either...
Can we just break everything apart in 2.5 please? Will this still be an
issue with CML2 anyway?
> I'm not going to cause disruption across the board to lots of people just
> because someone wants to keep the length of the symbols down.
>
> If you really do want to do this change, then I suggest that you get in
> touch with Nicolas Pitre and discuss it with him. When you come to a
> conclusion, its not as simple as patching the kernel. You need to
> update the database at http://www.arm.linux.org.uk/developer/machines/ to
> reflect the new symbols _at the same time_ that you change them in
> everyones tree, since anyone can pull down a new copy of the symbols
> from the database at any time.
For instance we might just keep the SA1100 out of the picture until we're
ready to make such change as "atomic" as possible for all people involved.
Nicolas