Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
On Wed, Feb 07, 2007 at 03:36:47PM +0100, Roman Zippel wrote: [] > Also please don't add random whitespace, Makefiles are no C files, so > different rules apply. Concerning whitespaces. Unfortunately only after installing i386 distro and checking that `-mcpu' error, i've realized what exactly whitespace you mean, doc! cc-option-yn = $(call cc-option, "y", "n", $(1)) vs. cc-option-yn = $(call cc-option,"y","n", $(1)) gives: + gcc ... -mtune=i386 -S -xc /dev/null -o /dev/shm/linux-2.6.20/.22730.null (0) + echo ' y' vs. + gcc ... -mtune=i386 -S -xc /dev/null -o /dev/shm/linux-2.6.20/.22231.null (1) + echo y and here sh1t hits the fun. I always wondered where are all that whitespaces appearing in gcc command line -- somebody adds them in makefiles, somebody don't, like in this comments: # cc-option # Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586) ^ ^ further this is used as this: #-mtune exists since gcc 3.4 HAS_MTUNE := $(call cc-option-yn, -mtune=i386) (2) ifeq ($(HAS_MTUNE),y) tune= $(call cc-option,-mtune=$(1),) [note: no whitespace] else tune= $(call cc-option,-mcpu=$(1),) endif [...] cflags-$(CONFIG_MPENTIUMIII)+= -march=i686 $(call tune,pentium3) cflags-$(CONFIG_MPENTIUMM) += -march=i686 $(call tune,pentium3) (2) conflicts with (0), but OK with (1). Summary: is it better to fix comments, as they are missling? Thanks, Roman! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
On Wed, Feb 07, 2007 at 03:36:47PM +0100, Roman Zippel wrote: > Hi, > > On Tue, 6 Feb 2007, Oleg Verych wrote: > > > -- all checks by shell united in one macro -- checker-shell; > > -- one disposable output sym. link to /dev/null per shell, > >thus no racing, `-Z' is removed; > > -- modules' build output directory is used, if supplied; > > -- every option checking function calls shell wrapper, acquires probe; > > -- `echo -e' bashizm substituted (people with sh != bash have distinct > >CC options!); > > -- some spelling and sense added to the comments; > > -- small shuffle of whitespace. > > This patch is a bit overloaded and it would be better to split it up OK (i though so, but file is too small) > to keep functional changes separate and then please drop the symlink, > it has no advantage to a simple temp file. Heh, i still can't understand issue with symlinks ;) OK. > Also please don't add random whitespace, Makefiles are no C files, so > different rules apply. I didn't know i did, conversely removed some trailing ones (in comments) and some multiline assigments. And i've tried to not touch rules-sections. As my first ever patch was open/close top Makefile with emacs, resulting tabify+trailing-whitespace-delete fail on Andrew's `make'. Finally i didn't know it will be commited, my will was -mm for testing. I'm very sorry, Linus, to bother you with reverting. Thanks. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
Hi, On Tue, 6 Feb 2007, Oleg Verych wrote: > -- all checks by shell united in one macro -- checker-shell; > -- one disposable output sym. link to /dev/null per shell, >thus no racing, `-Z' is removed; > -- modules' build output directory is used, if supplied; > -- every option checking function calls shell wrapper, acquires probe; > -- `echo -e' bashizm substituted (people with sh != bash have distinct >CC options!); > -- some spelling and sense added to the comments; > -- small shuffle of whitespace. This patch is a bit overloaded and it would be better to split it up to keep functional changes separate and then please drop the symlink, it has no advantage to a simple temp file. Also please don't add random whitespace, Makefiles are no C files, so different rules apply. bye, Roman - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
Hi, On Tue, 6 Feb 2007, Oleg Verych wrote: -- all checks by shell united in one macro -- checker-shell; -- one disposable output sym. link to /dev/null per shell, thus no racing, `-Z' is removed; -- modules' build output directory is used, if supplied; -- every option checking function calls shell wrapper, acquires probe; -- `echo -e' bashizm substituted (people with sh != bash have distinct CC options!); -- some spelling and sense added to the comments; -- small shuffle of whitespace. This patch is a bit overloaded and it would be better to split it up to keep functional changes separate and then please drop the symlink, it has no advantage to a simple temp file. Also please don't add random whitespace, Makefiles are no C files, so different rules apply. bye, Roman - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
On Wed, Feb 07, 2007 at 03:36:47PM +0100, Roman Zippel wrote: Hi, On Tue, 6 Feb 2007, Oleg Verych wrote: -- all checks by shell united in one macro -- checker-shell; -- one disposable output sym. link to /dev/null per shell, thus no racing, `-Z' is removed; -- modules' build output directory is used, if supplied; -- every option checking function calls shell wrapper, acquires probe; -- `echo -e' bashizm substituted (people with sh != bash have distinct CC options!); -- some spelling and sense added to the comments; -- small shuffle of whitespace. This patch is a bit overloaded and it would be better to split it up OK (i though so, but file is too small) to keep functional changes separate and then please drop the symlink, it has no advantage to a simple temp file. Heh, i still can't understand issue with symlinks ;) OK. Also please don't add random whitespace, Makefiles are no C files, so different rules apply. I didn't know i did, conversely removed some trailing ones (in comments) and some multiline assigments. And i've tried to not touch rules-sections. As my first ever patch was open/close top Makefile with emacs, resulting tabify+trailing-whitespace-delete fail on Andrew's `make'. Finally i didn't know it will be commited, my will was -mm for testing. I'm very sorry, Linus, to bother you with reverting. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
On Wed, Feb 07, 2007 at 03:36:47PM +0100, Roman Zippel wrote: [] Also please don't add random whitespace, Makefiles are no C files, so different rules apply. Concerning whitespaces. Unfortunately only after installing i386 distro and checking that `-mcpu' error, i've realized what exactly whitespace you mean, doc! cc-option-yn = $(call cc-option, y, n, $(1)) vs. cc-option-yn = $(call cc-option,y,n, $(1)) gives: + gcc ... -mtune=i386 -S -xc /dev/null -o /dev/shm/linux-2.6.20/.22730.null (0) + echo ' y' vs. + gcc ... -mtune=i386 -S -xc /dev/null -o /dev/shm/linux-2.6.20/.22231.null (1) + echo y and here sh1t hits the fun. I always wondered where are all that whitespaces appearing in gcc command line -- somebody adds them in makefiles, somebody don't, like in this comments: # cc-option # Usage: cflags-y += $(call cc-option, -march=winchip-c6, -march=i586) ^ ^ further this is used as this: #-mtune exists since gcc 3.4 HAS_MTUNE := $(call cc-option-yn, -mtune=i386) (2) ifeq ($(HAS_MTUNE),y) tune= $(call cc-option,-mtune=$(1),) [note: no whitespace] else tune= $(call cc-option,-mcpu=$(1),) endif [...] cflags-$(CONFIG_MPENTIUMIII)+= -march=i686 $(call tune,pentium3) cflags-$(CONFIG_MPENTIUMM) += -march=i686 $(call tune,pentium3) (2) conflicts with (0), but OK with (1). Summary: is it better to fix comments, as they are missling? Thanks, Roman! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
kbuild: improve option checking, Kbuild.include cleanup
GNU binutils, root users, tmpfiles, external modules ro builds must
be fixed to do the right thing now.
Cc: Roman Zippel <[EMAIL PROTECTED]>
Cc: Sam Ravnborg <[EMAIL PROTECTED]>
Cc: Horst Schirmeier <[EMAIL PROTECTED]>
Cc: Jan Beulich <[EMAIL PROTECTED]>
Cc: Daniel Drake <[EMAIL PROTECTED]>
Cc: Andi Kleen <[EMAIL PROTECTED]>
Cc: Randy Dunlap <[EMAIL PROTECTED]>
Signed-off-by: Oleg Verych <[EMAIL PROTECTED]>
---
-- all checks by shell united in one macro -- checker-shell;
-- one disposable output sym. link to /dev/null per shell,
thus no racing, `-Z' is removed;
-- modules' build output directory is used, if supplied;
-- every option checking function calls shell wrapper, acquires probe;
-- `echo -e' bashizm substituted (people with sh != bash have distinct
CC options!);
-- some spelling and sense added to the comments;
-- small shuffle of whitespace.
Mostly all people, discussing this back in October are in the Cc list.
Sam Ravnborg have not much time to reply. I've added Roman Zippel, as
a very kind reviewer and commiter of previous fix. Comments and
testing are appreciated. Thanks.
scripts/Kbuild.include | 96 +++--
1 file changed, 53 insertions(+), 43 deletions(-)
Index: linux-2.6.20/scripts/Kbuild.include
===
--- linux-2.6.20.orig/scripts/Kbuild.include2007-02-06 02:12:37.543828750
+0100
+++ linux-2.6.20/scripts/Kbuild.include 2007-02-06 02:14:37.575330250 +0100
@@ -2,5 +2,5 @@
# kbuild: Generic definitions
-# Convinient variables
+# Convenient constants
comma := ,
squote := '
@@ -57,38 +57,44 @@ endef
# See documentation in Documentation/kbuild/makefiles.txt
-# output directory for tests below
-TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
+# checker-shell
+# Usage: option = $(call checker-shell, $(CC)...-o $$OUT, option-ok, otherwise)
+# Exit code chooses option. $$OUT is safe location for needless output.
+define checker-shell
+ $(shell set -e; \
+DIR=$(KBUILD_EXTMOD); \
+cd $${DIR:-$(objtree)}; \
+OUT=$$PWD/..null; \
+\
+ln -s /dev/null $$OUT; \
+if $(1) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+fi; \
+rm -f $$OUT)
+endef
# 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 ;)
+as-option = $(call checker-shell, \
+ $(CC) $(CFLAGS) $(1) -c -xassembler /dev/null -o $$OUT, $(1), $(2))
# 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)
+as-instr = $(call checker-shell, \
+ printf "$(1)" | $(CC) $(AFLAGS) -c -xassembler -o $$OUT -, $(2), $(3))
# 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 ;)
+cc-option = $(call checker-shell, \
+ $(CC) $(CFLAGS) $(if $(3),$(3),$(1)) -S -xc /dev/null -o $$OUT, $(1), $(2))
# 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;)
+cc-option-yn = $(call cc-option, "y", "n", $(1))
# cc-option-align
# Prefix align with either -falign or -malign
cc-option-align = $(subst -functions=0,,\
- $(call cc-option,-falign-functions=0,-malign-functions=0))
+ $(call cc-option,-falign-functions=0,-malign-functions=0))
# cc-version
@@ -98,15 +104,13 @@ cc-version = $(shell $(CONFIG_SHELL) $(s
# cc-ifversion
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
-cc-ifversion = $(shell if [ $(call cc-version, $(CC)) $(1) $(2) ]; then \
- echo $(3); fi;)
+cc-ifversion = $(shell [ $(call cc-version, $(CC)) $(1) $(2) ] && echo $(3))
# 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)
+ld-option = $(call checker-shell, \
+ $(CC) $(1) -nostdlib -xc /dev/null -o $$OUT, $(1), $(2))
+
+##
-###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
# Usage:
@@ -114,17 +118,26 @@ ld-option = $(shell if $(CC) $(1) -nostd
build := -f $(if
[patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup
kbuild: improve option checking, Kbuild.include cleanup
GNU binutils, root users, tmpfiles, external modules ro builds must
be fixed to do the right thing now.
Cc: Roman Zippel [EMAIL PROTECTED]
Cc: Sam Ravnborg [EMAIL PROTECTED]
Cc: Horst Schirmeier [EMAIL PROTECTED]
Cc: Jan Beulich [EMAIL PROTECTED]
Cc: Daniel Drake [EMAIL PROTECTED]
Cc: Andi Kleen [EMAIL PROTECTED]
Cc: Randy Dunlap [EMAIL PROTECTED]
Signed-off-by: Oleg Verych [EMAIL PROTECTED]
---
-- all checks by shell united in one macro -- checker-shell;
-- one disposable output sym. link to /dev/null per shell,
thus no racing, `-Z' is removed;
-- modules' build output directory is used, if supplied;
-- every option checking function calls shell wrapper, acquires probe;
-- `echo -e' bashizm substituted (people with sh != bash have distinct
CC options!);
-- some spelling and sense added to the comments;
-- small shuffle of whitespace.
Mostly all people, discussing this back in October are in the Cc list.
Sam Ravnborg have not much time to reply. I've added Roman Zippel, as
a very kind reviewer and commiter of previous fix. Comments and
testing are appreciated. Thanks.
scripts/Kbuild.include | 96 +++--
1 file changed, 53 insertions(+), 43 deletions(-)
Index: linux-2.6.20/scripts/Kbuild.include
===
--- linux-2.6.20.orig/scripts/Kbuild.include2007-02-06 02:12:37.543828750
+0100
+++ linux-2.6.20/scripts/Kbuild.include 2007-02-06 02:14:37.575330250 +0100
@@ -2,5 +2,5 @@
# kbuild: Generic definitions
-# Convinient variables
+# Convenient constants
comma := ,
squote := '
@@ -57,38 +57,44 @@ endef
# See documentation in Documentation/kbuild/makefiles.txt
-# output directory for tests below
-TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
+# checker-shell
+# Usage: option = $(call checker-shell, $(CC)...-o $$OUT, option-ok, otherwise)
+# Exit code chooses option. $$OUT is safe location for needless output.
+define checker-shell
+ $(shell set -e; \
+DIR=$(KBUILD_EXTMOD); \
+cd $${DIR:-$(objtree)}; \
+OUT=$$PWD/..null; \
+\
+ln -s /dev/null $$OUT; \
+if $(1) /dev/null 21; \
+ then echo $(2); \
+ else echo $(3); \
+fi; \
+rm -f $$OUT)
+endef
# 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 21; then echo $(1); \
-else echo $(2); fi ;)
+as-option = $(call checker-shell, \
+ $(CC) $(CFLAGS) $(1) -c -xassembler /dev/null -o $$OUT, $(1), $(2))
# 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 21; \
- then rm $(TMPOUT)astest.out; echo $(2); \
- else echo $(3); fi)
+as-instr = $(call checker-shell, \
+ printf $(1) | $(CC) $(AFLAGS) -c -xassembler -o $$OUT -, $(2), $(3))
# 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 21; then echo $(1); else echo $(2); fi ;)
+cc-option = $(call checker-shell, \
+ $(CC) $(CFLAGS) $(if $(3),$(3),$(1)) -S -xc /dev/null -o $$OUT, $(1), $(2))
# 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 21; then echo y; else echo n; fi;)
+cc-option-yn = $(call cc-option, y, n, $(1))
# cc-option-align
# Prefix align with either -falign or -malign
cc-option-align = $(subst -functions=0,,\
- $(call cc-option,-falign-functions=0,-malign-functions=0))
+ $(call cc-option,-falign-functions=0,-malign-functions=0))
# cc-version
@@ -98,15 +104,13 @@ cc-version = $(shell $(CONFIG_SHELL) $(s
# cc-ifversion
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
-cc-ifversion = $(shell if [ $(call cc-version, $(CC)) $(1) $(2) ]; then \
- echo $(3); fi;)
+cc-ifversion = $(shell [ $(call cc-version, $(CC)) $(1) $(2) ] echo $(3))
# 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 21; \
- then rm $(TMPOUT)ldtest.out; echo $(1); \
- else echo $(2); fi)
+ld-option = $(call checker-shell, \
+ $(CC) $(1) -nostdlib -xc /dev/null -o $$OUT, $(1), $(2))
+
+##
-###
# Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
# Usage:
@@ -114,17 +118,26 @@ ld-option = $(shell if $(CC) $(1) -nostd
build := -f $(if $(KBUILD_SRC),$(srctree)/)scripts/Makefile.build obj
-# Prefix -I
