2001-11-26 13:07:14

by Olaf Hering

[permalink] [raw]
Subject: [PATCH] net/802/Makefile

Hi,

the build stops when cl2llc.c has no write permissions.

diff -urN linuxppc_2_4/net/802/Makefile.broken linuxppc_2_4/net/802/Makefile
--- linuxppc_2_4/net/802/Makefile.broken Mon Nov 26 13:28:56 2001
+++ linuxppc_2_4/net/802/Makefile Mon Nov 26 13:51:10 2001
@@ -57,4 +57,5 @@
include $(TOPDIR)/Rules.make

cl2llc.c: cl2llc.pre
+ chmod u+w cl2llc.c
sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c



Gruss Olaf

--
$ man clone

BUGS
Main feature not yet implemented...


2001-11-26 13:24:47

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, Nov 26, Olaf Hering wrote:

> Hi,
>
> the build stops when cl2llc.c has no write permissions.

Here is a better version, suggested by Rik:

diff -urN linuxppc_2_4/net/802/Makefile.broken
linuxppc_2_4/net/802/Makefile
--- linuxppc_2_4/net/802/Makefile.broken Mon Nov 26 13:28:56 2001
+++ linuxppc_2_4/net/802/Makefile Mon Nov 26 14:22:55 2001
@@ -57,4 +57,6 @@
include $(TOPDIR)/Rules.make

cl2llc.c: cl2llc.pre
+ touch cl2llc.c
+ chmod u+w cl2llc.c
sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c



Gruss Olaf

--
$ man clone

BUGS
Main feature not yet implemented...

2001-11-26 13:32:47

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

In article <[email protected]> you wrote:
> On Mon, Nov 26, Olaf Hering wrote:
>>
>> the build stops when cl2llc.c has no write permissions.
>
> Here is a better version, suggested by Rik:

What about just removing cl2llc.c from the tarball?

Christoph

--
Of course it doesn't work. We've performed a software upgrade.

2001-11-26 13:55:33

by Keith Owens

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, 26 Nov 2001 14:32:26 +0100,
Christoph Hellwig <[email protected]> wrote:
>In article <[email protected]> you wrote:
>> On Mon, Nov 26, Olaf Hering wrote:
>>>
>>> the build stops when cl2llc.c has no write permissions.
>>
>> Here is a better version, suggested by Rik:
>
>What about just removing cl2llc.c from the tarball?

I was going to do that but AC said that all of net/802 was being
rewritten and removing cl2llc.c now would just be noise. The file will
be removed in kbuild 2.5.

2001-11-26 15:14:01

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

Olaf Hering <[email protected]> writes:

|> On Mon, Nov 26, Olaf Hering wrote:
|>
|> > Hi,
|> >
|> > the build stops when cl2llc.c has no write permissions.
|>
|> Here is a better version, suggested by Rik:
|>
|> diff -urN linuxppc_2_4/net/802/Makefile.broken
|> linuxppc_2_4/net/802/Makefile
|> --- linuxppc_2_4/net/802/Makefile.broken Mon Nov 26 13:28:56 2001
|> +++ linuxppc_2_4/net/802/Makefile Mon Nov 26 14:22:55 2001
|> @@ -57,4 +57,6 @@
|> include $(TOPDIR)/Rules.make
|>
|> cl2llc.c: cl2llc.pre
|> + touch cl2llc.c
|> + chmod u+w cl2llc.c
|> sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c

How about this:

@@ -57,4 +57,5 @@
include $(TOPDIR)/Rules.make

cl2llc.c: cl2llc.pre
+ rm -f cl2llc.c
sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c


Andreas.

--
Andreas Schwab "And now for something
[email protected] completely different."
SuSE Labs, SuSE GmbH, Schanz?ckerstr. 10, D-90443 N?rnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5

2001-11-26 16:50:40

by Alan

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

> I was going to do that but AC said that all of net/802 was being
> rewritten and removing cl2llc.c now would just be noise. The file will
> be removed in kbuild 2.5.

The 802.2LLC layer will be replaced when netbeui goes in

2001-11-26 18:34:14

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile


Olaf,


Let the correction come to me through the 802 maintainer, please.

On Mon, 26 Nov 2001, Olaf Hering wrote:

> Hi,
>
> the build stops when cl2llc.c has no write permissions.
>
> diff -urN linuxppc_2_4/net/802/Makefile.broken linuxppc_2_4/net/802/Makefile
> --- linuxppc_2_4/net/802/Makefile.broken Mon Nov 26 13:28:56 2001
> +++ linuxppc_2_4/net/802/Makefile Mon Nov 26 13:51:10 2001
> @@ -57,4 +57,5 @@
> include $(TOPDIR)/Rules.make
>
> cl2llc.c: cl2llc.pre
> + chmod u+w cl2llc.c
> sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c
>
>
>
> Gruss Olaf
>
> --
> $ man clone
>
> BUGS
> Main feature not yet implemented...
>

2001-11-26 19:22:32

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

From: Marcelo Tosatti <[email protected]>
Date: Mon, 26 Nov 2001 15:14:33 -0200 (BRST)

Let the correction come to me through the 802 maintainer, please.

Technically there is no such person currently. At best it comes under
"networking" and that I guess would be me.

The person who originally wrote this 802 stuff is not active anymore
and he is only mentioned in the credits.

2001-11-26 19:36:41

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, Nov 26, David S. Miller wrote:

> From: Marcelo Tosatti <[email protected]>
> Date: Mon, 26 Nov 2001 15:14:33 -0200 (BRST)
>
> Let the correction come to me through the 802 maintainer, please.
>
> Technically there is no such person currently. At best it comes under
> "networking" and that I guess would be me.
>
> The person who originally wrote this 802 stuff is not active anymore
> and he is only mentioned in the credits.

Can you submit that version? ;)

diff -u linuxppc_2_4/net/802/Makefile.broken linuxppc_2_4/net/802/Makefile
--- linuxppc_2_4/net/802/Makefile.broken Mon Nov 26 13:28:56 2001
+++ linuxppc_2_4/net/802/Makefile Mon Nov 26 20:34:22 2001
@@ -57,4 +57,5 @@
include $(TOPDIR)/Rules.make

cl2llc.c: cl2llc.pre
+ rm -f cl2llc.c
sed -f ./pseudo/opcd2num.sed cl2llc.pre >cl2llc.c



Gruss Olaf

--
$ man clone

BUGS
Main feature not yet implemented...

2001-11-26 19:42:31

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

From: Olaf Hering <[email protected]>
Date: Mon, 26 Nov 2001 20:35:27 +0100

Can you submit that version? ;)

No, because it is writable by the user in every kernel source tree I
have ever seen, the change makes no sense.

In the official tarballs it is writable, and even in every CVS tree
(2.2.x, 2.4.x vger) I have looked at it is writable.

And as Alan has stated, this code is pretty irrelevant soon.

2001-11-27 05:10:59

by Tom Rini

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, Nov 26, 2001 at 11:40:45AM -0800, David S. Miller wrote:
> From: Olaf Hering <[email protected]>
> Date: Mon, 26 Nov 2001 20:35:27 +0100
>
> Can you submit that version? ;)
>
> No, because it is writable by the user in every kernel source tree I
> have ever seen, the change makes no sense.

You haven't tried a BitKeeper tree then. You have to explicitly get
write permission.

> In the official tarballs it is writable, and even in every CVS tree
> (2.2.x, 2.4.x vger) I have looked at it is writable.
>
> And as Alan has stated, this code is pretty irrelevant soon.

If this rewrite goes into 2.4, yes. And using 2.2 as an example it
could after a while...

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

2001-11-27 05:27:25

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

From: Tom Rini <[email protected]>
Date: Mon, 26 Nov 2001 22:10:12 -0700

On Mon, Nov 26, 2001 at 11:40:45AM -0800, David S. Miller wrote:
> No, because it is writable by the user in every kernel source tree I
> have ever seen, the change makes no sense.

You haven't tried a BitKeeper tree then. You have to explicitly get
write permission.

That doesn't make any sense to me.

Firstly, the *.c file should be newer than the template it is
generated from, so the rule which tries to write the file should
never run if your file timestamps are setup correctly. So get the
timestamps correct in your bitkeeper trees :)

Secondly, there is no reason the version control system should prevent
a file from being writable to the user who owns the file, especially
in a case like this.

In fact, this is one of the things that makes my balls hurt when I
download source tarballs from some places. I have to chmod +w the
files to edit them just to make hack test local changes that I don't
care if anyone ever sees ever again. At that point I'm fighting
whatever source control system those sources came from.

2001-11-27 06:12:38

by Keith Owens

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, 26 Nov 2001 21:26:58 -0800 (PST),
"David S. Miller" <[email protected]> wrote:
> From: Tom Rini <[email protected]>
> You haven't tried a BitKeeper tree then. You have to explicitly get
> write permission.
>
>That doesn't make any sense to me.
>
>Firstly, the *.c file should be newer than the template it is
>generated from, so the rule which tries to write the file should
>never run if your file timestamps are setup correctly. So get the
>timestamps correct in your bitkeeper trees :)

The source repository (whether BK or any other system) is not the
problem. You can get the timestamps right in the source but the moment
you generate and ship a diff then you lose control of timestamps. See
the long screed below about the problems with shipping generated files,
from kbuild-2.5.txt.

SHIPPING GENERATED FILES

In general it is bad practice to include generated files in the kernel
source tree. The only exceptions are when the generating code is not in
the kernel or when the generating code is in the kernel but it requires
additional tools which not all users have installed. An example of a
generated file without the code to regenerate it is firmware for a network
driver. An example of a generation process that requires extra tools is
drivers/char/defkeymap.c which requires the loadkeys program.

There is no justification for shipping any other generated file as part of
the kernel. This is especially true if the user is always expected to
regenerate the file, even more so if the generated data depends upon the
user's .config. Under these circumstances it is dangerous to ship a
generated file because it can be used when it does not reflect the user's
kernel.

When a generated file has no kernel support to regenerate it, the file can
be shipped as is. The question of whether including generated files in the
kernel without the underlying code is a violation of the kernel license is
outside the scope of this document.

For generated files where the code is included but not all users can run
it, you must be careful about when the generated file is rebuilt. make
looks at the time stamp of the generating and generated files to decide if
the output needs to be regenerated. This works for the person making the
changes because they control when the files are updated.

Time stamps do not work when the change is issued to the rest of the world
as a diff. After patch has run you cannot guarantee which of the updated
files has the more recent time. The patch program does not preserve time
stamps (it must not) so the time stamps on the changed files depend on the
order of the entries in the patch set. There is no defined order for
entries in a patch set, it depends on the program that generated the
difference listing. GNU diff generates patches in alphabetical order but
only when it is given an entire directory. When diff is invoked from a
source repository tool it is typically called once for each file and the
file order is controlled by the repository tool. Even if your repository
generates the patch in the correct order, that order can change when the
patch is merged into the kernel.

Since you cannot rely on time stamps to decide if a generated and shipped
file needs to be regenerated, you must use another mechanism for this case.
There are two basic possibilities, manual or automatic (otherwise known as
lazy or correct).

In the manual case you ignore changes to the generated or generating files
until the user performs some manual operation, such as selecting
CONFIG_REGENERATE_foo. This is easy to implement but it is also the lazy
approach, making the end user do extra work to save the maintainer doing
anything. This goes against the whole idea of *automatically* deciding
what needs to be rebuilt and relies on human intervention.

In the automatic case the maintainer has to do more work but the make rules
can automatically determine if the end user has changed any of the related
files and regenerate automatically. This is the correct method. In the
absence of reliable time stamps you need another method to check if the
generating and generated files are in sync or not. The obvious method is a
checksum of files.


The 2.5 implementation for correctly handling generated files has been
skipped, read Documentation/kbuild/kbuild-2.5.txt for the gory details.

2001-11-27 06:20:59

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

From: Keith Owens <[email protected]>
Date: Tue, 27 Nov 2001 17:12:04 +1100

The source repository (whether BK or any other system) is not the
problem. You can get the timestamps right in the source but the moment
you generate and ship a diff then you lose control of timestamps. See
the long screed below about the problems with shipping generated files,
from kbuild-2.5.txt.

Even after reading this I don't understand why defkeymap.c gets
special treatment just because it requires external tools to generate.

If the timestamps get messed up, it's going to try to regerenerate the
file with loadkeys whether you have it or not.

At best, I'd be happy to take a patch which commented out the rule
which tries to generate net/802/cl2llc.c but not one which will chmod
it. Because the latter only makes sense if we are now deciding to do
this for _every_ such case in the tree.

2001-11-27 06:28:49

by Keith Owens

[permalink] [raw]
Subject: Re: [PATCH] net/802/Makefile

On Mon, 26 Nov 2001 22:18:55 -0800 (PST),
"David S. Miller" <[email protected]> wrote:
> From: Keith Owens <[email protected]>
> Date: Tue, 27 Nov 2001 17:12:04 +1100
>
> The source repository (whether BK or any other system) is not the
> problem. You can get the timestamps right in the source but the moment
> you generate and ship a diff then you lose control of timestamps. See
> the long screed below about the problems with shipping generated files,
> from kbuild-2.5.txt.
>
>Even after reading this I don't understand why defkeymap.c gets
>special treatment just because it requires external tools to generate.
>
>If the timestamps get messed up, it's going to try to regerenerate the
>file with loadkeys whether you have it or not.

When the maintainer did the build on their system, the timestamps were
right. The moment the change is converted to a patch and sent for
inclusion in the kernel, timestamp order cannot be guaranteed, the
final result on kernel.org may have the base file being older or newer
than the generated output. kbuild 2.4 is completely broken for
generated files, tthe only reason it "works" is because most of these
files do not change.

kbuild 2.5 fixes this problem. It knows that you can never rely on
timestamps when you ship both a base file and a file that has been
generated from it. Instead it uses checksums to detect any changes,
that is part of the gory details that were omitted from the previous
mail.

>At best, I'd be happy to take a patch which commented out the rule
>which tries to generate net/802/cl2llc.c but not one which will chmod
>it. Because the latter only makes sense if we are now deciding to do
>this for _every_ such case in the tree.

I have fixed _every_ such case in 2.5. It is unfixable in 2.4, the
order of timestamps on generated and shipped files is not reliable.