2006-10-28 23:07:33

by Horst Schirmeier

[permalink] [raw]
Subject: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

Hello,

the kbuild-dont-put-temp-files-in-the-source-tree.patch (-mm patches) is
implemented faultily and fails in its intention to put temporary files
in $TMPDIR (or /tmp by default).

This is the code as it results from the patch:

ASTMP = $(shell mktemp ${TMPDIR:-/tmp}/asXXXXXX)
as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o $ASTMP ; \
then echo "$(2)"; else echo "$(3)"; fi; \
rm -f $ASTMP)

1) $ needs to be escaped in the shell function call, otherwise make
tries to substitute with a (in this case not existing) make variable
with this name.

2) Makefile variable names need to be put in parentheses; $ASTMP is
being interpreted as $(A)STMP, resulting in as-instr writing to a file
named after whatever is in $(A), followed by "STMP".

3) ld-option also writes to a temporary file and needs the same
treatment.

Please consider using the corrected patch below instead. Alternatively,
we could also simply use -o /dev/null, as we are not interested in the
output anyways. Daniel Drake already suggested this in a LKML post
(message-id [email protected]).

It would also be nice if this would make it into the mainline kernel
before 2.6.19 gets released; this would help avoiding the sandbox
violation problems on Gentoo Linux [1] [2].

Kind regards,
Horst Schirmeier

[1] http://bugs.gentoo.org/show_bug.cgi?id=149307
[2] LKML Message-ID: <[email protected]>

---
http://bugzilla.kernel.org/show_bug.cgi?id=7261 berates us for putting a
temporary file into the kernel source tree. Use mktemp instead.

Cc: Andi Kleen <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: [email protected],
Signed-off-by: Horst Schirmeier <[email protected]>
---

Kbuild.include | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

--- linux-mm/scripts/Kbuild.include.orig 2006-10-29 00:39:35.000000000 +0200
+++ linux-mm/scripts/Kbuild.include 2006-10-29 00:41:43.000000000 +0200
@@ -66,9 +66,10 @@ as-option = $(shell if $(CC) $(CFLAGS) $
# as-instr
# Usage: cflags-y += $(call as-instr, instr, option1, option2)

-as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o astest$$$$.out ; \
+ASTMP = $(shell mktemp $${TMPDIR:-/tmp}/asXXXXXX)
+as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o $(ASTMP) ; \
then echo "$(2)"; else echo "$(3)"; fi; \
- rm -f astest$$$$.out)
+ rm -f $(ASTMP))

# cc-option
# Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586)
@@ -97,10 +98,11 @@ cc-ifversion = $(shell if [ $(call cc-ve

# ld-option
# Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
+LDTMP = $(shell mktemp $${TMPDIR:-/tmp}/ldXXXXXX)
ld-option = $(shell if $(CC) $(1) \
- -nostdlib -o ldtest$$$$.out -xc /dev/null \
+ -nostdlib -o $(LDTMP) -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi; \
- rm -f ldtest$$$$.out)
+ rm -f $(LDTMP))

###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=


2006-10-29 02:08:00

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Saturday 28 October 2006 16:07, Horst Schirmeier wrote:
> Hello,
>
> the kbuild-dont-put-temp-files-in-the-source-tree.patch (-mm patches) is
> implemented faultily and fails in its intention to put temporary files
> in $TMPDIR (or /tmp by default).
>
> This is the code as it results from the patch:
>
> ASTMP = $(shell mktemp ${TMPDIR:-/tmp}/asXXXXXX)

I'm still against mktemp. It eats random entropy and
temporary files should be in the objdir, not in /tmp

-Andi

2006-10-29 12:09:02

by Horst Schirmeier

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Sat, 28 Oct 2006, Andi Kleen wrote:
> On Saturday 28 October 2006 16:07, Horst Schirmeier wrote:
> > Hello,
> >
> > the kbuild-dont-put-temp-files-in-the-source-tree.patch (-mm patches) is
> > implemented faultily and fails in its intention to put temporary files
> > in $TMPDIR (or /tmp by default).
> >
> > This is the code as it results from the patch:
> >
> > ASTMP = $(shell mktemp ${TMPDIR:-/tmp}/asXXXXXX)
>
> I'm still against mktemp. It eats random entropy and
> temporary files should be in the objdir, not in /tmp

TBH, I don't see the necessity of temporary files at all in this case,
but I assumed there must be a reason for them as the change already made
it into the -mm tree.

Why not use -o /dev/null, as Daniel Drake already suggested in [1]? In
both as-instr and ld-option, the tmp file is being deleted
unconditionally right after its creation anyways.

The attached patch is adapted from the patches proposed in [2], redone
as a replacement for
kbuild-dont-put-temp-files-in-the-source-tree.patch. Comments?

Kind regards,
Horst

[1] LKML Message-ID: <[email protected]>
[2] http://bugs.gentoo.org/show_bug.cgi?id=149307

---
Do not write temporary files in as-instr and ld-option but write to
/dev/null, as the output is not being used anyways.

Cc: Andi Kleen <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: [email protected]
Signed-off-by: Horst Schirmeier <[email protected]>
---

Kbuild.include | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

--- linux-mm/scripts/Kbuild.include.orig 2006-10-29 00:39:35.000000000 +0200
+++ linux-mm/scripts/Kbuild.include 2006-10-29 12:56:39.000000000 +0100
@@ -66,9 +66,8 @@ as-option = $(shell if $(CC) $(CFLAGS) $
# as-instr
# Usage: cflags-y += $(call as-instr, instr, option1, option2)

-as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o astest$$$$.out ; \
- then echo "$(2)"; else echo "$(3)"; fi; \
- rm -f astest$$$$.out)
+as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o /dev/null ; \
+ then echo "$(2)"; else echo "$(3)"; fi)

# cc-option
# Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586)
@@ -98,9 +97,8 @@ cc-ifversion = $(shell if [ $(call cc-ve
# ld-option
# Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
ld-option = $(shell if $(CC) $(1) \
- -nostdlib -o ldtest$$$$.out -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi; \
- rm -f ldtest$$$$.out)
+ -nostdlib -o /dev/null -xc /dev/null \
+ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi)

###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=

2006-10-29 16:17:52

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch


> Why not use -o /dev/null, as Daniel Drake already suggested in [1]? In
> both as-instr and ld-option, the tmp file is being deleted
> unconditionally right after its creation anyways.

Because then when the compilation runs as root some as versions
will delete /dev/null on a error. This has happened in the past.

-Andi

2006-10-29 17:52:34

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On 2006-10-29, Andi Kleen wrote:
>> Why not use -o /dev/null, as Daniel Drake already suggested in [1]? In
>> both as-instr and ld-option, the tmp file is being deleted
>> unconditionally right after its creation anyways.
>
> Because then when the compilation runs as root some as versions
> will delete /dev/null on a error. This has happened in the past.

OK, but let users, who still build kernels as root, alone.

In `19-rc3/include/Kbuild.include', just below `as-instr' i see:
,--
|cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
| > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
|
|# cc-option-yn
|# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
|cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
| > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
`--
so, change to `-o /dev/null' in `as-instr' will just follow this.
____

2006-10-29 22:52:36

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Sun, Oct 29, 2006 at 05:58:56PM +0000, Oleg Verych wrote:
>
> On 2006-10-29, Andi Kleen wrote:
> >> Why not use -o /dev/null, as Daniel Drake already suggested in [1]? In
> >> both as-instr and ld-option, the tmp file is being deleted
> >> unconditionally right after its creation anyways.
> >
> > Because then when the compilation runs as root some as versions
> > will delete /dev/null on a error. This has happened in the past.
>
> OK, but let users, who still build kernels as root, alone.
This needs to work - there are too much people that continue to do so.
And gentoo books recommended this last time I looked.

>
> In `19-rc3/include/Kbuild.include', just below `as-instr' i see:
> ,--
> |cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> | > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> |
> |# cc-option-yn
> |# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
> |cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> | > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
> `--
> so, change to `-o /dev/null' in `as-instr' will just follow this.

gcc does not delete files specified with -o - but binutils does.
So using /dev/null in this case is not an option.

Sam

2006-10-30 08:14:32

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

>> In `19-rc3/include/Kbuild.include', just below `as-instr' i see:
>> ,--
>> |cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>> | > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>> |
>> |# cc-option-yn
>> |# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
>> |cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>> | > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
>> `--
>> so, change to `-o /dev/null' in `as-instr' will just follow this.
>
>gcc does not delete files specified with -o - but binutils does.
>So using /dev/null in this case is not an option.

While I fixed this quite some time ago (after running into it myself), it
obviously still is a problem with older versions. However, using as' -Z
option seems to help here.
On the other hand, I long wanted to compose a patch to do away
with all the .tmp_* things at the build root, and move them into a
single .tmp/ directory - this would also seem to make a nice place to
put all sort of other temporary files in... I just never found the time
to actually do that, sorry.

Jan

2006-10-30 13:17:04

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On 2006-10-30, Jan Beulich wrote:
>
>>> In `19-rc3/include/Kbuild.include', just below `as-instr' i see:
>>> ,--
>>> |cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>>> | > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>>> |
>>> |# cc-option-yn
>>> |# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
>>> |cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>>> | > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
>>> `--
>>> so, change to `-o /dev/null' in `as-instr' will just follow this.
>>
>>gcc does not delete files specified with -o - but binutils does.
>>So using /dev/null in this case is not an option.

Hmm. What's the preblem to invoke `as' via the GNU C compiler, then?

> While I fixed this quite some time ago (after running into it myself), it
> obviously still is a problem with older versions. However, using as' -Z
> option seems to help here.
> On the other hand, I long wanted to compose a patch to do away
> with all the .tmp_* things at the build root, and move them into a
> single .tmp/ directory - this would also seem to make a nice place to
> put all sort of other temporary files in... I just never found the time
> to actually do that, sorry.

Maybe it's good idea, let me try, as i already bound to kbuild fixes.

But now, i'm just using KBUILD_OUTPUT=/tmp/, and /tmp/ is /dev/shm/.
It speeds up things on testing and small amounts of stuff to build.
Source tree is for patching only.
____

2006-10-30 16:00:52

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch


On 2006-10-30, Valdis.Kletnieks wrote:
>
> Hmm.. and even the 'ln' worked even when z9 was chmod 0. ;)
(WTF! I'm no wonder any more, why all that selinux was brought ;)

Well, i've said already about roots in post above.
This fix is for needless mktemp and old binutils.

Fix for roots:
,--
|if [ `id -u` == "0" ]; then echo "Root landed !!!"; ! true; fi
`--
More polite fools-protection, with guaranteed permission from the user:
,--
|if [ `id -u` == "0" ]; then useradd bkernel && su bkernel fi;
`--
____

2006-10-30 22:06:38

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch


On 2006-10-30, <olecom@flower> wrote:
> Fix for roots:
> ,--
>|if [ `id -u` == "0" ]; then echo "Root landed !!!"; ! true; fi
> `--
> More polite fools-protection, with guaranteed permission from the user:
> ,--
>|if [ `id -u` == "0" ]; then useradd bkernel && su bkernel fi;
> `--
> ____

For current state of things, i wish to propose

kbuild-mm-more-option-check-fixes.pre-patch:

Request For Testing.

Interested parties may test this one.
$(ret) is used for debug. In final version it may be removed,
$(objtree)/null must be known for clean targets.

I've replaced one `echo -e' with `printf', because, for example, my shell is
not bash, and built-in `echo' have not `-e' option, `printf' works everywhere.
[trailing spaces killed: +1]

Any comments are appreciated.
____

--- linux-2.6.19-rc3-mm1/scripts/Kbuild.include 2006-10-28 01:26:25.000000000 +0000
+++ linux-2.6.19-rc3-mm1/scripts/Kbuild.include~more-option-check-fixes 2006-10-30 20:39:03.641018805 +0000
@@ -7,6 +7,15 @@ squote := '
empty :=
space := $(empty) $(empty)

+# Immortal null for mortals and roots
+define null
+ $(shell \
+ if test -L null; \
+ then echo null; \
+ else rm -f null; ln -s /dev/null null; \
+ fi)
+endef
+
###
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)
@@ -56,30 +65,46 @@ endef
# gcc support functions
# See documentation in Documentation/kbuild/makefiles.txt

+ret = echo "$(1)" ; echo "$(1)" >> results.txt
# as-option
# Usage: cflags-y += $(call as-option, -Wa$(comma)-isa=foo,)
-
-as-option = $(shell if $(CC) $(CFLAGS) $(1) -Wa,-Z -c -o /dev/null \
- -xassembler /dev/null > /dev/null 2>&1; then echo "$(1)"; \
- else echo "$(2)"; fi ;)
+define as-option
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -c -o $(null) -xassembler null >null 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

# as-instr
# Usage: cflags-y += $(call as-instr, instr, option1, option2)
-
-as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o astest$$$$.out ; \
- then echo "$(2)"; else echo "$(3)"; fi; \
- rm -f astest$$$$.out)
+define as-instr
+ $(shell \
+ if printf "$(1)" | $(AS) >null 2>&1 -W -o $(null); \
+ then $(call ret,"$(2)"); \
+ else $(call ret,"$(3)"); \
+ fi)
+endef

# cc-option
# Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586)
-
-cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+define cc-option
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(null) -xc null >null 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

# cc-option-yn
# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
-cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
+define cc-option-yn
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(null) -xc null >null 2>&1; \
+ then $(call ret,"y"); \
+ else $(call ret,"n"); \
+ fi)
+endef

# cc-option-align
# Prefix align with either -falign or -malign
@@ -97,10 +122,13 @@ cc-ifversion = $(shell if [ $(call cc-ve

# ld-option
# Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
-ld-option = $(shell if $(CC) $(1) \
- -nostdlib -o ldtest$$$$.out -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi; \
- rm -f ldtest$$$$.out)
+define ld-option
+ $(shell \
+ if $(CC) $(1) -nostdlib -o $(null) -xc null >null 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
@@ -120,7 +148,7 @@ cmd = @$(echo-cmd) $(cmd_$(1))
objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o)))

###
-# if_changed - execute command if any prerequisite is newer than
+# if_changed - execute command if any prerequisite is newer than
# target, or command line has changed
# if_changed_dep - as if_changed, but uses fixdep to reveal dependencies
# including used config symbols

2006-10-31 00:12:37

by Horst Schirmeier

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Mon, 30 Oct 2006, Oleg Verych wrote:
> For current state of things, i wish to propose
>
> kbuild-mm-more-option-check-fixes.pre-patch:
>
> Request For Testing.
>
> Interested parties may test this one.
> $(ret) is used for debug. In final version it may be removed,
> $(objtree)/null must be known for clean targets.
>
> I've replaced one `echo -e' with `printf', because, for example, my shell is
> not bash, and built-in `echo' have not `-e' option, `printf' works everywhere.
> [trailing spaces killed: +1]
>
> Any comments are appreciated.

The problem is, this brings us back to the problem where this whole
patch orgy began: Gentoo Portage sandbox violations when writing (the
null symlink) to the kernel tree when building external modules. What
about using $(M) as a base directory if it is defined?

--- linux-mm/scripts/Kbuild.include.orig 2006-10-31 01:06:13.000000000 +0100
+++ linux-mm/scripts/Kbuild.include 2006-10-31 01:07:01.000000000 +0100
@@ -7,6 +7,20 @@ squote := '
empty :=
space := $(empty) $(empty)

+# Immortal null for mortals and roots
+ifdef M
+ null = $(M)/null
+else
+ null = null
+endif
+define createnull
+ $(shell \
+ if test -L $(null); \
+ then echo $(null); \
+ else rm -f $(null); ln -s /dev/null $(null); \
+ fi)
+endef
+
###
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)
@@ -56,30 +70,46 @@ endef
# gcc support functions
# See documentation in Documentation/kbuild/makefiles.txt

+ret = echo "$(1)" ; echo "$(1)" >> results.txt
# as-option
# Usage: cflags-y += $(call as-option, -Wa$(comma)-isa=foo,)
-
-as-option = $(shell if $(CC) $(CFLAGS) $(1) -Wa,-Z -c -o /dev/null \
- -xassembler /dev/null > /dev/null 2>&1; then echo "$(1)"; \
- else echo "$(2)"; fi ;)
+define as-option
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -c -o $(createnull) -xassembler $(null) >$(null) 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

# as-instr
# Usage: cflags-y += $(call as-instr, instr, option1, option2)
-
-as-instr = $(shell if echo -e "$(1)" | $(AS) >/dev/null 2>&1 -W -Z -o astest$$$$.out ; \
- then echo "$(2)"; else echo "$(3)"; fi; \
- rm -f astest$$$$.out)
+define as-instr
+ $(shell \
+ if printf "$(1)" | $(AS) >$(createnull) 2>&1 -W -o $(null); \
+ then $(call ret,"$(2)"); \
+ else $(call ret,"$(3)"); \
+ fi)
+endef

# cc-option
# Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586)
-
-cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+define cc-option
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(createnull) -xc $(null) >$(null) 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

# cc-option-yn
# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
-cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
+define cc-option-yn
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(createnull) -xc $(null) >$(null) 2>&1; \
+ then $(call ret,"y"); \
+ else $(call ret,"n"); \
+ fi)
+endef

# cc-option-align
# Prefix align with either -falign or -malign
@@ -97,10 +127,13 @@ cc-ifversion = $(shell if [ $(call cc-ve

# ld-option
# Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
-ld-option = $(shell if $(CC) $(1) \
- -nostdlib -o ldtest$$$$.out -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi; \
- rm -f ldtest$$$$.out)
+define ld-option
+ $(shell \
+ if $(CC) $(1) -nostdlib -o $(createnull) -xc $(null) >$(null) 2>&1; \
+ then $(call ret,"$(1)"); \
+ else $(call ret,"$(2)"); \
+ fi)
+endef

###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
@@ -120,7 +153,7 @@ cmd = @$(echo-cmd) $(cmd_$(1))
objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o)))

###
-# if_changed - execute command if any prerequisite is newer than
+# if_changed - execute command if any prerequisite is newer than
# target, or command line has changed
# if_changed_dep - as if_changed, but uses fixdep to reveal dependencies
# including used config symbols

2006-10-31 00:19:24

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch


> The problem is, this brings us back to the problem where this whole
> patch orgy began: Gentoo Portage sandbox violations when writing (the
> null symlink) to the kernel tree when building external modules. What
> about using $(M) as a base directory if it is defined?

I think Jan's $(objdir)/.tmp proposal would be cleanest. Just someone
has to implement it :)

-Andi

2006-10-31 00:27:59

by Horst Schirmeier

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Mon, 30 Oct 2006, Oleg Verych wrote:
> +# Immortal null for mortals and roots
> +define null
> + $(shell \
> + if test -L null; \
> + then echo null; \
> + else rm -f null; ln -s /dev/null null; \
> + fi)
> +endef

Another remark: the 'else' branch should echo null, too.

# Immortal null for mortals and roots
define null
$(shell \
if test ! -L null; \
then rm -f null; ln -s /dev/null null; \
fi; \
echo null)
endef

My patch proposal (the $(M) one) has the same bug.

Kind regards,
Horst

--
PGP-Key 0xD40E0E7A

2006-10-31 01:14:17

by Horst Schirmeier

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Tue, 31 Oct 2006, Andi Kleen wrote:
>
> > The problem is, this brings us back to the problem where this whole
> > patch orgy began: Gentoo Portage sandbox violations when writing (the
> > null symlink) to the kernel tree when building external modules. What
> > about using $(M) as a base directory if it is defined?
>
> I think Jan's $(objdir)/.tmp proposal would be cleanest. Just someone
> has to implement it :)
>
> -Andi

I'm not sure what you mean by $(objdir); I just got something to work
which creates the /dev/null symlink in a (newly created if necessary)
directory named

$(firstword $(obj-dirs) $(M))/.tmp

which seems to be a good place for both normal kernel builds and
external modules. External module builds seem not to set $(obj-dirs)...
Objections?

Kind regards,
Horst

--
PGP-Key 0xD40E0E7A

2006-10-31 13:27:06

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Tue, Oct 31, 2006 at 01:27:57AM +0100, Horst Schirmeier wrote:
[]
> echo null)
> endef
>
> My patch proposal (the $(M) one) has the same bug.

Obviously. That was my wrong optimization step.
____

2006-10-31 13:45:15

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

On Tue, Oct 31, 2006 at 02:14:16AM +0100, Horst Schirmeier wrote:
> On Tue, 31 Oct 2006, Andi Kleen wrote:
> >
> > > The problem is, this brings us back to the problem where this whole
> > > patch orgy began: Gentoo Portage sandbox violations when writing (the
> > > null symlink) to the kernel tree when building external modules. What
> > > about using $(M) as a base directory if it is defined?
> >
> > I think Jan's $(objdir)/.tmp proposal would be cleanest. Just someone
> > has to implement it :)
> >
> > -Andi

$(objtree) here,

> I'm not sure what you mean by $(objdir); I just got something to work
> which creates the /dev/null symlink in a (newly created if necessary)
> directory named

$(objtree) is a directory for all possible outputs of the build precess,
it's set up by `O=' or `KBUILD_OUTPUT', and this is *not* output for ready
external modules `$(M)'. Try to play with this, please.

I'm looking for Sam to say something, if we must go further with this.
____

2006-11-02 12:46:33

by Jan Peter den Heijer

[permalink] [raw]
Subject: Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch

How about using this:

ASTMP := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)astest$$$$.out

This is also used in the Makefile in the source tree top-level
directory (see line 332)
If KBUILD_EXTMOD is used, temp files are created in the module's
source directory, otherwise in the kernel source top-level directory

Jan Peter

2006-11-15 14:11:10

by Oleg Verych

[permalink] [raw]
Subject: kbuild-mm: $(objtree)/knull vs mktemp (was Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch)

On Tue, Oct 31, 2006 at 01:27:57AM +0100, Horst Schirmeier wrote:
> # Immortal knull for mortals and roots
> define knull
> $(shell \
> if test ! -L knull; \
> then rm -f knull; ln -s /dev/null knull; \
> fi; \
> echo knull)
> endef

So, i think this is much better than mktemp and friends.
Current -mm have +4 temp files after every make help, make, make something.

I'm steel looking forward for comments from (busy) developers, like Sam.

Now i have my laptop back and i can finish this up. Hm?
____

2006-11-17 05:10:45

by Oleg Verych

[permalink] [raw]
Subject: kbuild: O= with M= (Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch)

On Tue, Oct 31, 2006 at 02:51:36PM +0100, olecom wrote:
[]
> On Tue, Oct 31, 2006 at 02:14:16AM +0100, Horst Schirmeier wrote:
[]
> > I'm not sure what you mean by $(objdir); I just got something to work
> > which creates the /dev/null symlink in a (newly created if necessary)
> > directory named
>
> $(objtree) is a directory for all possible outputs of the build precess,
> it's set up by `O=' or `KBUILD_OUTPUT', and this is *not* output for ready
> external modules `$(M)'. Try to play with this, please.

And for me, they are *not* working together:

,--[shell]--
|olecom@deen:/tmp/linux-source-2.6.18$ make clean
|olecom@deen:/tmp/linux-source-2.6.18$ make M=$a
| LD /mnt/work/app-src-build/kernel.org/_work/ti_usb/built-in.o
| CC [M] /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.o
| Building modules, stage 2.
| MODPOST
| CC /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.mod.o
| LD [M] /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.ko
|olecom@deen:/tmp/linux-source-2.6.18$
|olecom@deen:/tmp/linux-source-2.6.18$ make clean
|olecom@deen:/tmp/linux-source-2.6.18$ make O=/tmp/_build-2.6/ M=$a
| CC [M] /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.o
| Building modules, stage 2.
| MODPOST
|/bin/sh: scripts/mod/modpost: not found
|make[2]: *** [__modpost] Error 127
|make[1]: *** [modules] Error 2
|make: *** [_all] Error 2
|olecom@deen:/tmp/linux-source-2.6.18$ make clean
|olecom@deen:/tmp/linux-source-2.6.18$ make M=$a
| CC [M] /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.o
| Building modules, stage 2.
| MODPOST
| LD [M] /mnt/work/app-src-build/kernel.org/_work/ti_usb/ti_usb_3410_5052.ko
|olecom@deen:/tmp/linux-source-2.6.18$
`--

I'm using 'O=' as good way to have clean kernel source directory,
regardless of any "ignore files" policy. And it seems, must be fixed.

> I'm looking for Sam to say something, if we must go further with this.
> ____
____

2007-01-24 08:47:18

by Oleg Verych

[permalink] [raw]
Subject: [patch] kbuild: improving option checking (Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch)

Hallo. Tmpfiles, root users, external mod-builds again.

On Tue, Oct 31, 2006 at 02:51:36PM +0100, olecom wrote:
> On Tue, Oct 31, 2006 at 02:14:16AM +0100, Horst Schirmeier wrote:
> > On Tue, 31 Oct 2006, Andi Kleen wrote:
> > >
> > > > The problem is, this brings us back to the problem where this whole
> > > > patch orgy began: Gentoo Portage sandbox violations when writing (the
> > > > null symlink) to the kernel tree when building external modules. What
> > > > about using $(M) as a base directory if it is defined?
> > >
> > > I think Jan's $(objdir)/.tmp proposal would be cleanest. Just someone
> > > has to implement it :)
> > >
> > > -Andi
>
> $(objtree) here,
>
> > I'm not sure what you mean by $(objdir); I just got something to work
> > which creates the /dev/null symlink in a (newly created if necessary)
> > directory named
>
> $(objtree) is a directory for all possible outputs of the build precess,
> it's set up by `O=' or `KBUILD_OUTPUT', and this is *not* output for ready
> external modules `$(M)'. Try to play with this, please.

Kind of fix has finally landed. Instead for `O=/$dir' a patch...

Anyway i whould like propose my solution of:
-- external modules build without KBUILD_OUTPUT set;
-- /dev/null, GNU binutils and root users;
-- beauty;

For external modules, there must be information after
`make modules_prepare' in shipped linux-sources. Any build output is
put in $(objtree), and i don't understand why you don't using that.
Please, *try* `make O=/tmp/build M=/usr/local/src/nvatia'. Thank you.

As some kind of buga-feature, "null" isn't in any clean/mrproper
target (for a while ;).

-*- patch -*-
kbuild: improving option checking

GNU binutils, root users, tmpfiles, external modules ro builds must
be fixed to do the right thing now.

In "safe" environment new /dev/null replacement may be used as simply as
`echo > null', `gcc -o null'. In aggressive starting with $(null) is
recommended.

Feature: file $(objtree)/null isn't in any "clear" target (yet).

Cc: Roman Zippel <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Signed-off-by: Oleg Verych <[email protected]>
---
-o--=O`C
#oo'L O
<___=E M

--- linux~2.6.20-rc5/scripts/Kbuild.include~blackhole-4-tmpfiles 2007-01-12 19:54:26.000000000 +0100
+++ linux~2.6.20-rc5/scripts/Kbuild.include 2007-01-24 09:19:01.386426000 +0100
@@ -2,5 +2,5 @@
# kbuild: Generic definitions

-# Convinient variables
+# Convinient constants
comma := ,
squote := '
@@ -8,4 +8,7 @@
space := $(empty) $(empty)

+# Immortal black-hole for mortals and roots
+null = $(shell test -L null || (rm -f null; ln -s /dev/null null); echo null)
+
###
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
@@ -57,33 +60,43 @@
# See documentation in Documentation/kbuild/makefiles.txt

-# output directory for tests below
-TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
-
# as-option
# Usage: cflags-y += $(call as-option, -Wa$(comma)-isa=foo,)
-
-as-option = $(shell if $(CC) $(CFLAGS) $(1) -Wa,-Z -c -o /dev/null \
- -xassembler /dev/null > /dev/null 2>&1; then echo "$(1)"; \
- else echo "$(2)"; fi ;)
+define as-option
+ $(shell
+ if $(CC) $(CFLAGS) $(1) -c -o $(null) -xassembler null >null 2>&1; \
+ then echo $(1); \
+ else echo $(2); \
+ fi)
+endef

# as-instr
# Usage: cflags-y += $(call as-instr, instr, option1, option2)
-
-as-instr = $(shell if echo -e "$(1)" | \
- $(CC) $(AFLAGS) -c -xassembler - \
- -o $(TMPOUT)astest$$$$.out > /dev/null 2>&1; \
- then rm $(TMPOUT)astest$$$$.out; echo "$(2)"; \
- else echo "$(3)"; fi)
+define as-instr
+ $(shell \
+ if printf "$(1)" | $(AS) >$(null) 2>&1 -W -o null; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+endef

# cc-option
# Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586)
-
-cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+define cc-option
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(null) -xc null >null 2>&1; \
+ then echo "$(1)"; \
+ else echo "$(2)"; \
+ fi)
+endef

# cc-option-yn
# Usage: flag := $(call cc-option-yn, -march=winchip-c6)
-cc-option-yn = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
- > /dev/null 2>&1; then echo "y"; else echo "n"; fi;)
+define cc-option-yn
+ $(shell \
+ if $(CC) $(CFLAGS) $(1) -S -o $(null) -xc null >null 2>&1; \
+ then echo "y"; \
+ else echo "n"; \
+ fi)
+endef

# cc-option-align
@@ -103,8 +116,11 @@
# ld-option
# Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
-ld-option = $(shell if $(CC) $(1) -nostdlib -xc /dev/null \
- -o $(TMPOUT)ldtest$$$$.out > /dev/null 2>&1; \
- then rm $(TMPOUT)ldtest$$$$.out; echo "$(1)"; \
- else echo "$(2)"; fi)
+define ld-option
+ $(shell \
+ if $(CC) $(1) -nostdlib -o $(null) -xc null >null 2>&1; \
+ then echo "$(1)"; \
+ else echo "$(2)"; \
+ fi)
+endef

###
@@ -116,4 +132,5 @@
# Prefix -I with $(srctree) if it is not an absolute path
addtree = $(if $(filter-out -I/%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1))) $(1)
+
# Find all -I options and call addtree
flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
@@ -126,5 +143,5 @@

###
-# if_changed - execute command if any prerequisite is newer than
+# if_changed - execute command if any prerequisite is newer than
# target, or command line has changed
# if_changed_dep - as if_changed, but uses fixdep to reveal dependencies

2007-01-24 16:11:54

by Randy Dunlap

[permalink] [raw]
Subject: Re: [patch] kbuild: improving option checking (Re: [PATCH -mm] replacement for broken kbuild-dont-put-temp-files-in-the-source-tree.patch)

On Wed, 24 Jan 2007 08:54:30 +0000 Oleg Verych wrote:

> Hallo. Tmpfiles, root users, external mod-builds again.
>
> Kind of fix has finally landed. Instead for `O=/$dir' a patch...
>
> Anyway i whould like propose my solution of:
> -- external modules build without KBUILD_OUTPUT set;
> -- /dev/null, GNU binutils and root users;
> -- beauty;
> ---
>
> --- linux~2.6.20-rc5/scripts/Kbuild.include~blackhole-4-tmpfiles 2007-01-12 19:54:26.000000000 +0100
> +++ linux~2.6.20-rc5/scripts/Kbuild.include 2007-01-24 09:19:01.386426000 +0100
> @@ -2,5 +2,5 @@
> # kbuild: Generic definitions
>
> -# Convinient variables
> +# Convinient constants

Convenient

---
~Randy

2007-01-24 19:19:54

by Oleg Verych

[permalink] [raw]
Subject: [patch] spell 4 kbuild: improving option checking

Signed-off-by: Oleg Verych <[email protected]>

--- linux~2.6.20-rc5/scripts/Kbuild.include~ 2007-01-24 09:19:01.386426000 +0100
+++ linux~2.6.20-rc5/scripts/Kbuild.include 2007-01-24 20:07:11.152820000 +0100
@@ -2,5 +2,5 @@
# kbuild: Generic definitions

-# Convinient constants
+# Convenient constants
comma := ,
squote := '
@@ -171,5 +171,5 @@
any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)

-# Execute command if command has changed or prerequisitei(s) are updated
+# Execute command if command has changed or prerequisite(s) are updated
#
if_changed = $(if $(strip $(any-prereq) $(arg-check)), \
@@ -188,5 +188,5 @@

# Usage: $(call if_changed_rule,foo)
-# will check if $(cmd_foo) changed, or any of the prequisites changed,
+# will check if $(cmd_foo) changed, or any of the prerequisites changed,
# and if so will execute $(rule_foo)
if_changed_rule = $(if $(strip $(any-prereq) $(arg-check) ), \
@@ -233,2 +233,4 @@
echo-why = $(call escsq, $(strip $(why)))
endif
+
+ LocalWords: prequisites