Re: [patch] 2.5.25 net/core/Makefile

Re: [patch] 2.5.25 net/core/Makefile

[Thunder from the hill]

Hi,

On Jul 9, 2002 02:13:30, Keith Owens wrote:
> +# See p8022 in net/802/Makefile for config options to check
> +ifneq ($(subst n,,$(CONFIG_LLC)$(CONFIG_TR)$(CONFIG_IPX)$(CONFIG_ATALK)),)
>  obj-y += ext8022.o
>  endif

Make's response:

make[4]: Entering directory `/home/thunder/tmp/thunder-2.5-kb24/net/core'
Makefile:20: *** missing separator.  Stop.
make[4]: Leaving directory `/home/thunder/tmp/thunder-2.5-kb24/net/core'
make[3]: *** [core] Error 2

Ideas?

							Regards,
							Thunder
-- 
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

[patch] 2.5.25 net/core/Makefile

[Keith Owens]

The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
builds ext8022.o and gets unresolved references because there is no
network code.  Detected by kbuild 2.5, not detected by the existing
build system.

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o
 
 obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o
 
-ifneq ($(CONFIG_LLC),n)
+ifneq ($(subst n,,$(CONFIG_LLC)),)
 obj-y += ext8022.o
 endif
 

Re: [patch] 2.5.25 net/core/Makefile

[Dave Jones]

On Sat, Jul 06, 2002 at 06:30:29PM +1000, Keith Owens wrote:
 > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
 > builds ext8022.o and gets unresolved references because there is no
 > network code.  Detected by kbuild 2.5, not detected by the existing
 > build system.

And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd

net/network.o: In function Register_8022_client':
net/network.o(.text+0xe8b7): undefined reference to Llc_register_sap'
net/network.o: In function Unregister_8022_client':
net/network.o(.text+0xe8fe): undefined reference to Llc_unregister_sap'


-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [patch] 2.5.25 net/core/Makefile

[Keith Owens]

On Tue, 9 Jul 2002 01:11:35 +0100, 
Dave Jones <davej@suse.de> wrote:
>On Sat, Jul 06, 2002 at 06:30:29PM +1000, Keith Owens wrote:
> > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> > builds ext8022.o and gets unresolved references because there is no
> > network code.  Detected by kbuild 2.5, not detected by the existing
> > build system.
>
>And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd
>
>net/network.o: In function Register_8022_client':
>net/network.o(.text+0xe8b7): undefined reference to Llc_register_sap'
>net/network.o: In function Unregister_8022_client':
>net/network.o(.text+0xe8fe): undefined reference to Llc_unregister_sap'

??? That is the bug that I reported.  My patch fixes that bug.

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o
 
 obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o
 
-ifneq ($(CONFIG_LLC),n)
+ifneq ($(subst n,,$(CONFIG_LLC)),)
 obj-y += ext8022.o
 endif
 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch] 2.5.25 net/core/Makefile

[Dave Jones]

On Tue, Jul 09, 2002 at 10:30:31AM +1000, Keith Owens wrote:
 > > > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
 > >And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd

 > ??? That is the bug that I reported.  My patch fixes that bug.

Same bug maybe, but triggered in different ways.
Note the CONFIG_NET change between your report and mine.

With mainline kbuild this isn't an issue, but your fix breaks it.

*shrug*

        Dave

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [patch] 2.5.25 net/core/Makefile

[Keith Owens]

On Tue, 9 Jul 2002 02:36:28 +0200, 
Dave Jones <davej@suse.de> wrote:
>On Tue, Jul 09, 2002 at 10:30:31AM +1000, Keith Owens wrote:
> > > > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> > >And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd
>
> > ??? That is the bug that I reported.  My patch fixes that bug.
>
>Same bug maybe, but triggered in different ways.
>Note the CONFIG_NET change between your report and mine.

Sorry, missed that change the first time.

The problem is net/802/Makefile which includes p8022 for any of
CONFIG_LLC, CONFIG_TR, CONFIG_IPX or CONFIG_ATALK.  p8022 calls
llc_register_sap which is in ext8022.o, that file is built by
net/core/Makefile but only for CONFIG_LLC.  It worked before because of
the wrong test in net/core/Makefile which always built ext8022.o.

Davem - we could unconditionally build ext8022.o when CONFIG_NET=y or
we could do this

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Tue, 09 Jul 2002 12:10:53 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,8 @@ obj-$(CONFIG_FILTER) += filter.o
 
 obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o
 
-ifneq ($(CONFIG_LLC),n)
+# See p8022 in net/802/Makefile for config options to check
+ifneq ($(subst n,,$(CONFIG_LLC)$(CONFIG_TR)$(CONFIG_IPX)$(CONFIG_ATALK)),)
 obj-y += ext8022.o
 endif
 
Index: 25.1/net/802/Makefile
--- 25.1/net/802/Makefile Fri, 21 Jun 2002 10:09:01 +1000 kaos (linux-2.5/r/c/0_Makefile 1.3 444)
+++ 25.1(w)/net/802/Makefile Tue, 09 Jul 2002 12:10:38 +1000 kaos (linux-2.5/r/c/0_Makefile 1.3 444)
@@ -6,6 +6,7 @@ export-objs		:= llc_macinit.o p8022.o ps
 
 obj-y			:= p8023.o
 
+# Check the p8022 selections against net/core/Makefile.
 obj-$(CONFIG_SYSCTL)	+= sysctl_net_802.o
 obj-$(CONFIG_LLC)	+= p8022.o psnap.o llc_sendpdu.o llc_utility.o \
 			   		   cl2llc.o llc_macinit.o	

Re: [patch] 2.5.25 net/core/Makefile

[Arnaldo Carvalho de Melo]

Em Tue, Jul 09, 2002 at 12:13:30PM +1000, Keith Owens escreveu:
> On Tue, 9 Jul 2002 02:36:28 +0200, 
> Dave Jones <davej@suse.de> wrote:
> >Same bug maybe, but triggered in different ways.
> >Note the CONFIG_NET change between your report and mine.
> 
> Sorry, missed that change the first time.
> 
> The problem is net/802/Makefile which includes p8022 for any of
> CONFIG_LLC, CONFIG_TR, CONFIG_IPX or CONFIG_ATALK.  p8022 calls

This will be moot when I remove p8022.c from the kernel (in fact all of
net/802), which I plan to do before 2.5 freeze 8)

- Arnaldo

Re: [patch] 2.5.25 net/core/Makefile

[Kai Germaschewski]

On Sat, 6 Jul 2002, Keith Owens wrote:

> The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> builds ext8022.o and gets unresolved references because there is no
> network code.  Detected by kbuild 2.5, not detected by the existing
> build system.
> 
> Index: 25.1/net/core/Makefile
> --- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
> +++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
> @@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o
>  
>  obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o
>  
> -ifneq ($(CONFIG_LLC),n)
> +ifneq ($(subst n,,$(CONFIG_LLC)),)
>  obj-y += ext8022.o
>  endif

Makes sense to me. However, the CONFIG_ variables used in the Makefiles 
are never "n", they are "y", "m" or undefined.

In Config.in scripts you have to cater for "n" or "", and I've seen 
various people on l-k carry this behavior into the Makefiles, but there 
it's unnecessary for all I can tell.

So

ifdef CONFIG_LLC
obj-y += ext8022.o
endif

should do the job just fine.

--Kai

Re: [patch] 2.5.25 net/core/Makefile

[Keith Owens]

On Sun, 21 Jul 2002 18:56:24 -0500 (CDT), 
Kai Germaschewski <kai@tp1.ruhr-uni-bochum.de> wrote:
>Makes sense to me. However, the CONFIG_ variables used in the Makefiles 
>are never "n", they are "y", "m" or undefined.
>
>In Config.in scripts you have to cater for "n" or "", and I've seen 
>various people on l-k carry this behavior into the Makefiles, but there 
>it's unnecessary for all I can tell.

It is required if you ever want autoconfigure to work, that
distinguishes between "" (undefined) and "n" (explicitly turned off).
Forward planning.

Re: [patch] 2.5.25 net/core/Makefile

[Russell King]

On Mon, Jul 22, 2002 at 11:08:41AM +1000, Keith Owens wrote:
> It is required if you ever want autoconfigure to work, that
> distinguishes between "" (undefined) and "n" (explicitly turned off).
> Forward planning.

Wouldn't it be better to fix the existing config tools to output "=n"
instead of "# CONFIG_foo is not set" ?  IIRC they do the translation
back and forth internally anyway, so it should be just a matter of
removing some code from the tools.

After all, the earlier we update the config tools, the earlier we can
do something with the makefiles (after a reasonable period for things
like mconfig to catch up...)

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [patch] 2.5.25 net/core/Makefile

[Roman Zippel]

Hi,

On Mon, 22 Jul 2002, Russell King wrote:

> Wouldn't it be better to fix the existing config tools to output "=n"
> instead of "# CONFIG_foo is not set" ?  IIRC they do the translation
> back and forth internally anyway, so it should be just a matter of
> removing some code from the tools.

This would mean, tristate symbols had four states instead of three. The
current shell based config systems simply don't see all symbols.
Depending on the configuration a symbol could be unset or 'n'.

bye, Roman

Re: [patch] 2.5.25 net/core/Makefile

[Andreas Schwab]

Roman Zippel <zippel@linux-m68k.org> writes:

|> Hi,
|> 
|> On Mon, 22 Jul 2002, Russell King wrote:
|> 
|> > Wouldn't it be better to fix the existing config tools to output "=n"
|> > instead of "# CONFIG_foo is not set" ?  IIRC they do the translation
|> > back and forth internally anyway, so it should be just a matter of
|> > removing some code from the tools.
|> 
|> This would mean, tristate symbols had four states instead of three. The
|> current shell based config systems simply don't see all symbols.

They do, see for example load_config_file in scripts/Menuconfig, or around
line 556 in script/Configure.

|> Depending on the configuration a symbol could be unset or 'n'.

A symbol is unset if it does not occur in .config at all.  Having "#
CONFIG_foo is not set" in .config is completely the same as
"CONFIG_foo=n".

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] 2.5.25 net/core/Makefile

[Russell King]

On Mon, Jul 22, 2002 at 10:51:43AM +0200, Andreas Schwab wrote:
> They do, see for example load_config_file in scripts/Menuconfig, or around
> line 556 in script/Configure.

What Roman is meaning is something like this:

if [ "$CONFIG_FOO" = "y" ]; then
   choice ...
fi

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [patch] 2.5.25 net/core/Makefile

[Roman Zippel]

Hi,

On Mon, 22 Jul 2002, Russell King wrote:

> What Roman is meaning is something like this:
>
> if [ "$CONFIG_FOO" = "y" ]; then
>    choice ...
> fi

BTW that's not the only problem, a symbol might be defined in
arch/foo/config.in, but not in arch/bar/config.in, so even xconfig doesn't
see everything.

bye, Roman

Re: [patch] 2.5.25 net/core/Makefile

[Kai Germaschewski]

On Mon, 22 Jul 2002, Russell King wrote:

> On Mon, Jul 22, 2002 at 11:08:41AM +1000, Keith Owens wrote:
> > It is required if you ever want autoconfigure to work, that
> > distinguishes between "" (undefined) and "n" (explicitly turned off).
> > Forward planning.
> 
> Wouldn't it be better to fix the existing config tools to output "=n"
> instead of "# CONFIG_foo is not set" ?  IIRC they do the translation
> back and forth internally anyway, so it should be just a matter of
> removing some code from the tools.

The point is, what would such a change buy us? It needs going through all 
Makefiles, updating 

	ifdef CONFIG_XYZ

to

	ifneq ($(CONFIG_XYZ),n)


(or ifeq ($(CONFIG_XYZ),y) when we now it's a bool)

Actually, now this won't handle the CONFIG_XYZ unset case, which may well 
happen since a part of Config.in which would set or unset the symbols 
may not even get sourced.

So we really have to use

	ifneq ($(subst n,,$(CONFIG_XYZ),)

instead. That's ugly and doesn't have any advantage over what we have 
now, AFAICS.

Inside the Config.in scripts it's annoying that you have to check against
"n" || "" (or ! ("y" || "m") ). The reasons for that lie, for all I can
tell, in the use of sh/bash for the original Configure script. In any
case, if this behavior is considered too annoying, it should be fixed in
the config system, but there's no reason to change the Makefile/.config 
syntax, too.

(I think it may actually possible to fix it even within Configure, by just
always using <undef> instead of "n" - Configure needs to keep track of all
the encountered symbols anyway - but I didn't try. This would also
decrease the confusion, as then both the Makefiles and Config.in would
both use "y","m",<undef>.

W.r.t autoconfigure, I think that can easily be achieved using/extending 
the existing .config format.

Just have

	# CONFIG_FOO is not set
vs
	# CONFIG_FOO=n

or even

	<CONFIG_FOO not mentioned in .config>

vs the current

	# CONFIG_FOO is not set

Should be nicely compatible with all existing tools.

--Kai