LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Roman Zippel <zippel@linux-m68k.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Oleg Verych <olecom@flower.upol.cz>
Subject: The who needs reviews anyways [PATCH]
Date: Thu, 8 Feb 2007 22:48:51 +0100 (CET)	[thread overview]
Message-ID: <Pine.LNX.4.64.0702082227020.14457@scrub.home> (raw)

Hi,

This adds the remaining changes which should have been part of the review 
process. Oleg could have learned something in process, but who needs that 
if wasting everyones time is so much more fun...

Sorry for the delay, but the git server were gone.

- the define command is inappropriate (it's primarily for rule 
  definitions)
- execute commands in the current dir as all other commands
- .*.tmp (but not .*.null) files are also removed up by "make clean"
- printf has other side effects, instead stop pretending we support 
  something else than bash
- proper quoting
- proper indentation

bye, Roman

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 Makefile               |    9 ++++--
 scripts/Kbuild.include |   72 ++++++++++++++++++++++++-------------------------
 2 files changed, 42 insertions(+), 39 deletions(-)

Index: linux-2.6/Makefile
===================================================================
--- linux-2.6.orig/Makefile
+++ linux-2.6/Makefile
@@ -192,8 +192,11 @@ KCONFIG_CONFIG	?= .config
 
 # SHELL used by kbuild
 CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
-	  else if [ -x /bin/bash ]; then echo /bin/bash; \
-	  else echo sh; fi ; fi)
+	  else if [ -x /bin/bash ]; then echo /bin/bash; fi; fi)
+ifeq ($(CONFIG_SHELL),)
+$(error bash is required to build the kernel)
+endif
+SHELL := $(CONFIG_SHELL)
 
 HOSTCC       = gcc
 HOSTCXX      = g++
@@ -321,7 +324,7 @@ KERNELRELEASE = $(shell cat include/conf
 KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
 
 export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
-export ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
+export ARCH CONFIG_SHELL SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
 
Index: linux-2.6/scripts/Kbuild.include
===================================================================
--- linux-2.6.orig/scripts/Kbuild.include
+++ linux-2.6/scripts/Kbuild.include
@@ -1,7 +1,7 @@
 ####
 # kbuild: Generic definitions
 
-# Convenient constants
+# Convenient variables
 comma   := ,
 squote  := '
 empty   :=
@@ -56,44 +56,48 @@ endef
 # gcc support functions
 # See documentation in Documentation/kbuild/makefiles.txt
 
-# 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; \
-  if $(1) >/dev/null 2>&1; \
-    then echo "$(2)"; \
-    else echo "$(3)"; \
-  fi; \
-  rm -f $$OUT)
-endef
+# output directory for tests below
+TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP")
 
 # as-option
 # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
-as-option = $(call checker-shell,\
-   $(CC) $(CFLAGS) $(1) -c -xassembler /dev/null -o $$OUT,$(1),$(2))
+
+as-option = $(call try-run,\
+	$(CC) $(CFLAGS) $(1) -c -xassembler /dev/null -o "$$TMP",$(1),$(2))
 
 # as-instr
 # Usage: cflags-y += $(call as-instr,instr,option1,option2)
-as-instr = $(call checker-shell,\
-   printf "$(1)" | $(CC) $(AFLAGS) -c -xassembler -o $$OUT -,$(2),$(3))
+
+as-instr = $(call try-run,\
+	echo -e "$(1)" | $(CC) $(AFLAGS) -c -xassembler -o "$$TMP" -,$(2),$(3))
 
 # cc-option
 # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
-cc-option = $(call checker-shell,\
-   $(CC) $(CFLAGS) $(if $(3),$(3),$(1)) -S -xc /dev/null -o $$OUT,$(1),$(2))
+
+cc-option = $(call try-run,\
+	$(CC) $(CFLAGS) $(1) -S -xc /dev/null -o "$$TMP",$(1),$(2))
 
 # cc-option-yn
 # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
-cc-option-yn = $(call cc-option,"y","n",$(1))
+cc-option-yn = $(call try-run,\
+	$(CC) $(CFLAGS) $(1) -S -xc /dev/null -o "$$TMP",y,n)
 
 # 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
 # Usage gcc-ver := $(call cc-version,$(CC))
@@ -105,24 +109,22 @@ cc-ifversion = $(shell [ $(call cc-versi
 
 # ld-option
 # Usage: ldflags += $(call ld-option, -Wl$(comma)--hash-style=both)
-ld-option = $(call checker-shell,\
-   $(CC) $(1) -nostdlib -xc /dev/null -o $$OUT,$(1),$(2))
+ld-option = $(call try-run,\
+	$(CC) $(1) -nostdlib -xc /dev/null -o "$$TMP",$(1),$(2))
 
 ######
 
+###
 # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
 # Usage:
 # $(Q)$(MAKE) $(build)=dir
 build := -f $(if $(KBUILD_SRC),$(srctree)/)scripts/Makefile.build obj
 
-# Prefix -I with $(srctree) if it is not an absolute path,
-# add original to the end
-addtree = $(if \
-	$(filter-out -I/%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1))) $(1)
+# 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)))
+flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
 
 # echo command.
 # Short version is used, if $(quiet) equals `quiet_', otherwise full one.
@@ -144,7 +146,7 @@ objectify = $(foreach o,$(1),$(if $(filt
 # See Documentation/kbuild/makefiles.txt for more info
 
 ifneq ($(KBUILD_NOCMDDEP),1)
-# Check if both arguments has same arguments. Result is empty string, if equal.
+# Check if both arguments has same arguments. Result is empty string if equal.
 # User may override this check using make KBUILD_NOCMDDEP=1
 arg-check = $(strip $(filter-out $(cmd_$(1)), $(cmd_$@)) \
                     $(filter-out $(cmd_$@),   $(cmd_$(1))) )
@@ -168,7 +170,6 @@ if_changed = $(if $(strip $(any-prereq) 
 	echo 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
 
 # Execute the command and also postprocess generated .d dependencies file.
-#
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
 	@set -e;                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
@@ -176,10 +177,9 @@ if_changed_dep = $(if $(strip $(any-prer
 	rm -f $(depfile);                                                    \
 	mv -f $(dot-target).tmp $(dot-target).cmd)
 
-# Will check if $(cmd_foo) changed, or any of the prerequisites changed,
-# and if so will execute $(rule_foo).
 # Usage: $(call if_changed_rule,foo)
-#
+# Will check if $(cmd_foo) or any of the prerequisites changed,
+# and if so will execute $(rule_foo).
 if_changed_rule = $(if $(strip $(any-prereq) $(arg-check) ),                 \
 	@set -e;                                                             \
 	$(rule_$(1)))

             reply	other threads:[~2007-02-08 21:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-08 21:48 Roman Zippel [this message]
2007-02-08 22:05 ` Sam Ravnborg
2007-02-08 22:22 ` Linus Torvalds
2007-02-08 22:53   ` Roman Zippel
2007-02-08 23:02     ` Linus Torvalds
2007-02-08 23:20       ` Roman Zippel
2007-02-09  2:29         ` Valdis.Kletnieks
2007-02-08 23:03 ` Kbuild refactoring (Re: The who needs reviews anyways [PATCH]) Oleg Verych
2007-02-09  0:06 ` The who needs reviews anyways [PATCH] Andreas Schwab
2007-02-09  1:21   ` Roman Zippel
2007-02-09  5:22 ` Oleg Verych
2007-02-09 11:35   ` Roman Zippel
2007-02-09 21:42     ` Oleg Verych

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0702082227020.14457@scrub.home \
    --to=zippel@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olecom@flower.upol.cz \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: The who needs reviews anyways [PATCH]' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).