[patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)

[patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)

[Oleg Verych]

Due to almost zer0 activity, this may be last attempt.

--

[patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Oleg Verych]

[-- Attachment #1: scripts-replace-gawk-and-by-shell-with-some-cleanup_update.patch --]
[-- Type: text/plain, Size: 4871 bytes --]

scripts: replace gawk, head, bc with shell, update

  Replacing overhead of using some (external) programs
  instead of good old `sh'.

Cc: Roman Zippel <zippel@linux-m68k.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: William Stearns <wstearns@pobox.com>
Cc: Martin Schlemmer <azarah@nosferatu.za.org>
Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
---

 Whitespace cleanup included.

 scripts/gen_initramfs_list.sh | 43 +++++++++++++++++++++---------------------
 scripts/makelst               | 34 ++++++++++++++++-----------------
 2 files changed, 39 insertions(+), 38 deletions(-)

Index: linux-2.6.20/scripts/makelst
===================================================================
--- linux-2.6.20.orig/scripts/makelst	2007-02-06 02:12:38.811908000 +0100
+++ linux-2.6.20/scripts/makelst	2007-02-06 02:12:43.688212750 +0100
@@ -1,31 +1,31 @@
-#!/bin/bash
+#!/bin/sh
 # A script to dump mixed source code & assembly
 # with correct relocations from System.map
-# Requires the following lines in Rules.make.
-# Author(s): DJ Barrow (djbarrow@de.ibm.com,barrow_dj@yahoo.com) 
-#            William Stearns <wstearns@pobox.com>
+# Requires the following lines in makefile:
 #%.lst: %.c
 #	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(CFLAGS_$@) -g -c -o $*.o $<
-#	$(TOPDIR)/scripts/makelst $*.o $(TOPDIR)/System.map $(OBJDUMP)
+#	$(srctree)/scripts/makelst $*.o $(objtree)/System.map $(OBJDUMP)
 #
-#    Copyright (C) 2000 IBM Corporation
-#    Author(s): DJ Barrow (djbarrow@de.ibm.com,barrow_dj@yahoo.com) 
+# Copyright (C) 2000 IBM Corporation
+# Author(s): DJ Barrow (djbarrow@de.ibm.com,barrow_dj@yahoo.com)
+#            William Stearns <wstearns@pobox.com>
 #
 
-t1=`$3 --syms $1 | grep .text | grep " F " | head -n 1`
+# awk style field access
+field() {
+  shift $1 ; echo $1
+}
+
+t1=`$3 --syms $1 | grep .text | grep -m1 " F "`
 if [ -n "$t1" ]; then
-  t2=`echo $t1 | gawk '{ print $6 }'`
+  t2=`field 6 $t1`
   if [ ! -r $2 ]; then
     echo "No System.map" >&2
-    t7=0
   else
     t3=`grep $t2 $2`
-    t4=`echo $t3 | gawk '{ print $1 }'`
-    t5=`echo $t1 | gawk '{ print $1 }'`
-    t6=`echo $t4 - $t5 | tr a-f A-F`
-    t7=`( echo  ibase=16 ; echo $t6 ) | bc`
+    t4=`field 1 $t3`
+    t5=`field 1 $t1`
+    t6=`printf "%lu" $((0x$t4 - 0x$t5))`
   fi
-else
-  t7=0
 fi
-$3 -r --source --adjust-vma=$t7 $1
+$3 -r --source --adjust-vma=${t6:-0} $1
Index: linux-2.6.20/scripts/gen_initramfs_list.sh
===================================================================
--- linux-2.6.20.orig/scripts/gen_initramfs_list.sh	2007-02-06 02:12:38.819908500 +0100
+++ linux-2.6.20/scripts/gen_initramfs_list.sh	2007-02-06 02:12:43.688212750 +0100
@@ -1,5 +1,5 @@
 #!/bin/bash
 # Copyright (C) Martin Schlemmer <azarah@nosferatu.za.org>
-# Copyright (c) 2006           Sam Ravnborg <sam@ravnborg.org>
+# Copyright (C) 2006 Sam Ravnborg <sam@ravnborg.org>
 #
 # Released under the terms of the GNU GPL
@@ -18,13 +18,13 @@ Usage:
 $0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
 	-o <file>      Create gzipped initramfs file named <file> using
-	               gen_init_cpio and gzip
+		       gen_init_cpio and gzip
 	-u <uid>       User ID to map to user ID 0 (root).
-	               <uid> is only meaningful if <cpio_source>
-	               is a directory.
+		       <uid> is only meaningful if <cpio_source>
+		       is a directory.
 	-g <gid>       Group ID to map to group ID 0 (root).
-	               <gid> is only meaningful if <cpio_source>
-	               is a directory.
+		       <gid> is only meaningful if <cpio_source>
+		       is a directory.
 	<cpio_source>  File list or directory for cpio archive.
-	               If <cpio_source> is a .cpio file it will be used
+		       If <cpio_source> is a .cpio file it will be used
 		       as direct input to initramfs.
 	-d             Output the default cpio list.
@@ -37,4 +37,10 @@ EOF
 }
 
+# awk style field access
+# $1 - field number; rest is argument string
+field() {
+	shift $1 ; echo $1
+}
+
 list_default_initramfs() {
 	# echo usr/kinit/kinit
@@ -120,20 +126,15 @@ parse() {
 			;;
 		"nod")
-			local dev_type=
-			local maj=$(LC_ALL=C ls -l "${location}" | \
-					gawk '{sub(/,/, "", $5); print $5}')
-			local min=$(LC_ALL=C ls -l "${location}" | \
-					gawk '{print $6}')
-
-			if [ -b "${location}" ]; then
-				dev_type="b"
-			else
-				dev_type="c"
-			fi
-			str="${ftype} ${name} ${str} ${dev_type} ${maj} ${min}"
+			local dev=`LC_ALL=C ls -l "${location}"`
+			local maj=`field 5 ${dev}`
+			local min=`field 6 ${dev}`
+			maj=${maj%,}
+
+			[ -b "${location}" ] && dev="b" || dev="c"
+
+			str="${ftype} ${name} ${str} ${dev} ${maj} ${min}"
 			;;
 		"slink")
-			local target=$(LC_ALL=C ls -l "${location}" | \
-					gawk '{print $11}')
+			local target=`field 11 $(LC_ALL=C ls -l "${location}")`
 			str="${ftype} ${name} ${target} ${str}"
 			;;

--

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Mark Lord]

Oleg Verych wrote:
> scripts: replace gawk, head, bc with shell, update
> 
>   Replacing overhead of using some (external) programs
>   instead of good old `sh'.
...
> -    t4=`echo $t3 | gawk '{ print $1 }'`
> -    t5=`echo $t1 | gawk '{ print $1 }'`
> -    t6=`echo $t4 - $t5 | tr a-f A-F`
> -    t7=`( echo  ibase=16 ; echo $t6 ) | bc`
> +    t4=`field 1 $t3`
> +    t5=`field 1 $t1`
> +    t6=`printf "%lu" $((0x$t4 - 0x$t5))`
...

>From where do we obtain this new "field" command ?

Cheers

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Oleg Verych]

On Tue, Feb 06, 2007 at 09:49:47AM -0500, Mark Lord wrote:
> Oleg Verych wrote:
> >scripts: replace gawk, head, bc with shell, update
> >
> >  Replacing overhead of using some (external) programs
> >  instead of good old `sh'.
> ...                                  <---------------,
> >-    t4=`echo $t3 | gawk '{ print $1 }'`            |
> >-    t5=`echo $t1 | gawk '{ print $1 }'`            |
> >-    t6=`echo $t4 - $t5 | tr a-f A-F`               |
> >-    t7=`( echo  ibase=16 ; echo $t6 ) | bc`        |
> >+    t4=`field 1 $t3`                               |
> >+    t5=`field 1 $t1`                               |
> >+    t6=`printf "%lu" $((0x$t4 - 0x$t5))`           |
> ...                                                  |
>                                                      |
> >From where do we obtain this new "field" command ?  |
                                       `---- here -----'

Without jokes, it even commented:

+# awk style field access
+field() {
+  shift $1 ; echo $1
+}
+

> Cheers
____

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Mark Lord]

Oleg Verych wrote:
..
> Without jokes, it even commented:
> 
> +# awk style field access
> +field() {
> +  shift $1 ; echo $1
> +}
> +

Ack.

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Jesper Juhl]

On 06/02/07, Oleg Verych <olecom@flower.upol.cz> wrote:
> scripts: replace gawk, head, bc with shell, update
>
>   Replacing overhead of using some (external) programs
>   instead of good old `sh'.
>
> Cc: Roman Zippel <zippel@linux-m68k.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: William Stearns <wstearns@pobox.com>
> Cc: Martin Schlemmer <azarah@nosferatu.za.org>
> Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
> ---
>
>  Whitespace cleanup included.
>
>  scripts/gen_initramfs_list.sh | 43 +++++++++++++++++++++---------------------
>  scripts/makelst               | 34 ++++++++++++++++-----------------
>  2 files changed, 39 insertions(+), 38 deletions(-)
>
> Index: linux-2.6.20/scripts/makelst
> ===================================================================
> --- linux-2.6.20.orig/scripts/makelst   2007-02-06 02:12:38.811908000 +0100
> +++ linux-2.6.20/scripts/makelst        2007-02-06 02:12:43.688212750 +0100
> @@ -1,31 +1,31 @@
> -#!/bin/bash
> +#!/bin/sh

I don't like this change much.  We don't know what shell /bin/sh is on
a random users system - much better to be explicit and make sure we
use/require bash.
I expect this change to cause pain for users who have zsh, csh, ksh,
ash or some other shell as /bin/sh but also have /bin/bash available.
Previously things would just work, but now it will possibly break due
to using a different shell.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Oleg Verych]

On Wed, Feb 07, 2007 at 09:58:12AM +0100, Jesper Juhl wrote:
> On 06/02/07, Oleg Verych <olecom@flower.upol.cz> wrote:
> >scripts: replace gawk, head, bc with shell, update
> >
> >  Replacing overhead of using some (external) programs
> >  instead of good old `sh'.
> >
> >Cc: Roman Zippel <zippel@linux-m68k.org>
> >Cc: Sam Ravnborg <sam@ravnborg.org>
> >Cc: William Stearns <wstearns@pobox.com>
> >Cc: Martin Schlemmer <azarah@nosferatu.za.org>
> >Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
> >---
> >
> > Whitespace cleanup included.
> >
> > scripts/gen_initramfs_list.sh | 43 
> > +++++++++++++++++++++---------------------
> > scripts/makelst               | 34 ++++++++++++++++-----------------
> > 2 files changed, 39 insertions(+), 38 deletions(-)
> >
> >Index: linux-2.6.20/scripts/makelst
> >===================================================================
> >--- linux-2.6.20.orig/scripts/makelst   2007-02-06 02:12:38.811908000 +0100
> >+++ linux-2.6.20/scripts/makelst        2007-02-06 02:12:43.688212750 +0100
> >@@ -1,31 +1,31 @@
> >-#!/bin/bash
> >+#!/bin/sh
> 
> I don't like this change much.  We don't know what shell /bin/sh is on
> a random users system - much better to be explicit and make sure we
> use/require bash.
> I expect this change to cause pain for users who have zsh, csh, ksh,
> ash or some other shell as /bin/sh but also have /bin/bash available.
> Previously things would just work, but now it will possibly break due
> to using a different shell.


I've tested on dash, that is `sh'. Everybody is welcome to test on his
favorite `/bin/sh' compatible shell.

It's very unclear from bash man page, where are extentions, and where are
common features. That particular script is small and non-bash oriented,
unlike gen_initramfs_list.sh [s/sh/bash].

Thus it's OK, even in scope of SUSv3 "incompatible" "%ul" in `printf'
(see my other reply on that).

____

Re: [patch 1/3, resend] scripts: replace gawk, head, bc with shell, update

[Roman Zippel]

Hi,

On Tue, 6 Feb 2007, Oleg Verych wrote:

> scripts: replace gawk, head, bc with shell, update
> 
>   Replacing overhead of using some (external) programs
>   instead of good old `sh'.

awk is indeed a bit of overkill.

> 
> Cc: Roman Zippel <zippel@linux-m68k.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: William Stearns <wstearns@pobox.com>
> Cc: Martin Schlemmer <azarah@nosferatu.za.org>
> Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
> ---

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

bye, Roman

[patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup

[Oleg Verych]

[-- Attachment #1: kbuild-improve-option-checking.patch --]
[-- Type: text/plain, Size: 8370 bytes --]

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 <zippel@linux-m68k.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Horst Schirmeier <horst@schirmeier.com>
Cc: Jan Beulich <jbeulich@novell.com>
Cc: Daniel Drake <dsd@gentoo.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
---

-- 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.include	2007-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 $(KBUILD_SRC),$(srctree)/)scripts/Makefile.build obj
 
-# Prefix -I with $(srctree) if it is not an absolute path
-addtree = $(if $(filter-out -I/%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1))) $(1)
+# 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)
+
 # 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)))
 
-# If quiet is set, only print short version of command
+# echo command.
+# Short version is used, if $(quiet) equals `quiet_', otherwise full one.
+echo-cmd = $(if $($(quiet)cmd_$(1)), \
+	echo '  $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
+
+# printing commands
 cmd = @$(echo-cmd) $(cmd_$(1))
 
-# Add $(obj)/ for paths that is not absolute
-objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o)))
+# Add $(obj)/ for paths that are not absolute
+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
@@ -134,5 +147,5 @@ objectify = $(foreach o,$(1),$(if $(filt
 
 ifneq ($(KBUILD_NOCMDDEP),1)
-# Check if both arguments has same arguments. Result in 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_$@)) \
@@ -140,8 +153,4 @@ arg-check = $(strip $(filter-out $(cmd_$
 endif
 
-# echo command. Short version is $(quiet) equals quiet, otherwise full command
-echo-cmd = $(if $($(quiet)cmd_$(1)), \
-	echo '  $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
-
 # >'< substitution is for echo to work,
 # >$< substitution to preserve $ when reloading .cmd file
@@ -154,5 +163,5 @@ make-cmd = $(subst \#,\\\#,$(subst $$,$$
 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)),                       \
@@ -161,6 +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
+# Execute the command and also postprocess generated .d dependencies file.
+#
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
 	@set -e;                                                             \
@@ -170,7 +179,8 @@ if_changed_dep = $(if $(strip $(any-prer
 	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) changed, or any of the prequisites changed,
-# and if so will execute $(rule_foo)
+#
 if_changed_rule = $(if $(strip $(any-prereq) $(arg-check) ),                 \
 	@set -e;                                                             \

--

Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup

[Roman Zippel]

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

Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup

[Oleg Verych]

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.
____

Re: [patch 2/3, resend] kbuild: improve option checking, Kbuild.include cleanup

[Oleg Verych]

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!
____

[patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Oleg Verych]

[-- Attachment #1: kbuild-localversion-correctly-skip-backups.patch --]
[-- Type: text/plain, Size: 2590 bytes --]

kbuild: correctly skip tilded backups in localversion files

 Tildes as in path as in filenames are handled correctly now:
 only files, containing tilde '~', are backups, thus are not valid.

 [KJ]:
 Definition of `space' was removed, scripts/Kbuild.include has one.
 That definition was taken right from the GNU make manual, while Kbuild's
 version is original.

Cc: Roman Zippel <zippel@linux-m68k.org>
Cc: Bastian Blank <bastian@waldi.eu.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
---

 My using of the `sh' rides from willing to have more portable,
 understandable implementation (and due to GFDL make's docs ;)

Original report by Bastian Blank:

 The following patch fixes the problem that localversion files where
 ignored if the tree lives in a path which contains a ~. It changes the
 test to apply to the filename only.

 Debian allows versions which contains ~ in it. The upstream part of the
 version is in the directory name of the build tree and we got weird
 results because the localversion files was just got ignored in this
 case.

 Makefile |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Index: linux-2.6.20/Makefile
===================================================================
--- linux-2.6.20.orig/Makefile	2007-02-06 02:12:35.703713750 +0100
+++ linux-2.6.20/Makefile	2007-02-06 02:17:06.612644500 +0100
@@ -777,5 +777,5 @@ $(vmlinux-dirs): prepare scripts
 #	$(localver-full)
 #	  $(localver)
-#	    localversion*		(all localversion* files)
+#	    localversion*		(files without backups, containing '~')
 #	    $(CONFIG_LOCALVERSION)	(from kernel config setting)
 #	  $(localver-auto)		(only if CONFIG_LOCALVERSION_AUTO is set)
@@ -788,15 +788,10 @@ $(vmlinux-dirs): prepare scripts
 # scripts/setlocalversion and add the appropriate checks as needed.
 
-nullstring :=
-space      := $(nullstring) # end of line
+pattern = ".*/localversion[^~]*"
+string  = $(shell cat /dev/null \
+	   `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort`)
 
-___localver = $(objtree)/localversion* $(srctree)/localversion*
-__localver  = $(sort $(wildcard $(___localver)))
-# skip backup files (containing '~')
-_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(f)),,$(f)))
-
-localver = $(subst $(space),, \
-	   $(shell cat /dev/null $(_localver)) \
-	   $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
+localver = $(subst $(space),, $(string) \
+			      $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
 
 # If CONFIG_LOCALVERSION_AUTO is set scripts/setlocalversion is called

--

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Roman Zippel]

Hi,

On Tue, 6 Feb 2007, Oleg Verych wrote:

> -nullstring :=
> -space      := $(nullstring) # end of line
> +pattern = ".*/localversion[^~]*"
> +string  = $(shell cat /dev/null \
> +	   `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort`)

Calling find here is overkill, if the same can be done with standard make 
functions. I very much prefer to just add the damn $(notdir ...) and be 
done with it.

bye, Roman

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Oleg Verych]

On Wed, Feb 07, 2007 at 03:39:38PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Tue, 6 Feb 2007, Oleg Verych wrote:
> 
> > -nullstring :=
> > -space      := $(nullstring) # end of line
> > +pattern = ".*/localversion[^~]*"
> > +string  = $(shell cat /dev/null \
> > +	   `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort`)
> 
> Calling find here is overkill, if the same can be done with standard make 
> functions. I very much prefer to just add the damn $(notdir ...) and be 
> done with it.

Yes? I whould insist on it as separation of "meat" from "flies", i.e
paths from files, instead of (ugly) reparsing. But you know better.

Thanks.

____

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Tony Luck]

On 2/5/07, Oleg Verych <olecom@flower.upol.cz> wrote:
> kbuild: correctly skip tilded backups in localversion files
>
>  Tildes as in path as in filenames are handled correctly now:
>  only files, containing tilde '~', are backups, thus are not valid.

Git bisect fingers this patch (which is in Linus' tree as commit
76c329563c5b8663ef27eb1bd195885ab826cbd0) as the culprit
for double adding the contents of the localversion file.  E.g.

$ echo -tiger-smp > localversion
$ make prepare
$ make kernelrelease
2.6.20-tiger-smp-tiger-smp

-Tony

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Linus Torvalds]

On Mon, 12 Feb 2007, Tony Luck wrote:
> 
> Git bisect fingers this patch (which is in Linus' tree as commit
> 76c329563c5b8663ef27eb1bd195885ab826cbd0) as the culprit
> for double adding the contents of the localversion file.  E.g.
> 
> $ echo -tiger-smp > localversion
> $ make prepare
> $ make kernelrelease
> 2.6.20-tiger-smp-tiger-smp

Heh. It's because we search for the localversion files in both $objtree 
and $srctree, and normally they are one and the same - so it finds the 
same file twice.

The old code did the same thing, but with the "make" $(sort ..) function, 
which apparently removes duplicates. We should use "sort -u" here.

Both the old code *and* the new code is just horribly complex. The old 
code appears to suffer from GNU $(wildcard ..), the new code is almost as 
ugly in doing an unnecessarily complex "find".

Oh well.

		Linus

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Oleg Verych]

On Mon, Feb 12, 2007 at 02:53:29PM -0800, Linus Torvalds wrote:

Hallo.

> On Mon, 12 Feb 2007, Tony Luck wrote:
> > 
> > Git bisect fingers this patch (which is in Linus' tree as commit
> > 76c329563c5b8663ef27eb1bd195885ab826cbd0) as the culprit
> > for double adding the contents of the localversion file.  E.g.
> > 
> > $ echo -tiger-smp > localversion
> > $ make prepare
> > $ make kernelrelease
> > 2.6.20-tiger-smp-tiger-smp
> 
> Heh. It's because we search for the localversion files in both $objtree 
> and $srctree, and normally they are one and the same - so it finds the 
> same file twice.
> 
> The old code did the same thing, but with the "make" $(sort ..) function, 
> which apparently removes duplicates. We should use "sort -u" here.

Heh. Why one ever going to bloat $(srctree) to add more "dontdiff" and
such, where build is supporting dirty output?

While this is my mis-tesing,

> Both the old code *and* the new code is just horribly complex. The old 
> code appears to suffer from GNU $(wildcard ..), the new code is almost as 
> ugly in doing an unnecessarily complex "find".

any reasons to have multiple files for localversion(s), in $(objtree)
also? Exept one, that somebody used to use it, mm-tree doesn't btw.

As i opposed to Romans's "make" solution, because it wasn't obvious, i
agree current has heavy `find', but what else you can propose? I think,
it's not make's job after all.

> Oh well.

Thanks for testing!
____

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Oleg Verych]

[]
> > The old code did the same thing, but with the "make" $(sort ..) function, 
> > which apparently removes duplicates. We should use "sort -u" here.
> 
> Heh. Why one ever going to bloat $(srctree) to add more "dontdiff" and
> such, where build is supporting dirty output?

I mean, all by-hand modifications must be in the $(srctree) (let's get
this term), $(objtree) is output *only*. Thus, i would propose to remove
it from the path. Even dynamic SCM mechanism of adding local version
doesn't use `localversion' files.

> any reasons to have multiple files for localversion(s), in $(objtree)
> also? Exept one, that somebody used to use it, mm-tree doesn't btw.

I know it maybe another my "change it all" proposition, but i can't find
nothing against `GNU $(wildcard ..)' and `unnecessarily complex "find"'.

____

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Gerd Hoffmann]

  Hi,

> I mean, all by-hand modifications must be in the $(srctree) (let's get
> this term), $(objtree) is output *only*. Thus, i would propose to remove
> it from the path. Even dynamic SCM mechanism of adding local version
> doesn't use `localversion' files.

I use localversion-$foo files in both srctree (scm tags such as
"-hg<changeset>") and objtree (config tag, "-default", "-pae", ...) and
find it quite useful.  The login prompt usually includes $(uname -r), so
you easily see what kernel the machine runs at the moment ...

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Linus Torvalds]

On Tue, 13 Feb 2007, Oleg Verych wrote:
> 
> I mean, all by-hand modifications must be in the $(srctree) (let's get
> this term), $(objtree) is output *only*.

No. Especially for things like localversion, the object tree (if it is 
different) is very much where you'd put that marker.

You might have several object trees for the same source tree, with 
different configurations. Exactly to remember which one is which, you'd 
have a "localversion" file in each object tree.

> I know it maybe another my "change it all" proposition, but i can't find
> nothing against `GNU $(wildcard ..)' and `unnecessarily complex "find"'.

It's the regexp in both cases. $(wildcard ) doesn't do regexp's (only the 
normal path rules), and traditional 'find' doesn't either. The fact that 
GNU find does is another matter. I don't think we require GNU find 
normally.

And I don't even much like the "backup" thing. Some programs will use 
other things than "~" as a backup marker. Patch more often uses ".orig", 
for example. So both methods are fairly complex, but at the same time not 
quite complex enough.

It would probably have been a better idea had we made the rule be that the 
file is called "*localversion" rather than "localversion*", exactly 
because that way it's unambiguous (people normally use _suffixes_ for 
filetypes, not prefixes). That would have avoided the whole complexity in 
wildcarding, but it's too late now..

	$(sort $(wildcard $(srctree)/*localversion $(objtree)/*localversion)

should have worked.

		Linus

Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files

[Roman Zippel]

Hi,

On Tue, 13 Feb 2007, Linus Torvalds wrote:

> > I know it maybe another my "change it all" proposition, but i can't find
> > nothing against `GNU $(wildcard ..)' and `unnecessarily complex "find"'.
> 
> It's the regexp in both cases. $(wildcard ) doesn't do regexp's (only the 
> normal path rules), and traditional 'find' doesn't either. The fact that 
> GNU find does is another matter. I don't think we require GNU find 
> normally.
> 
> And I don't even much like the "backup" thing. Some programs will use 
> other things than "~" as a backup marker. Patch more often uses ".orig", 
> for example. So both methods are fairly complex, but at the same time not 
> quite complex enough.
> 
> It would probably have been a better idea had we made the rule be that the 
> file is called "*localversion" rather than "localversion*", exactly 
> because that way it's unambiguous (people normally use _suffixes_ for 
> filetypes, not prefixes). That would have avoided the whole complexity in 
> wildcarding, but it's too late now..
> 
> 	$(sort $(wildcard $(srctree)/*localversion $(objtree)/*localversion)
> 
> should have worked.

For now I think it's easier to just revert the change to use find, I 
posted the patch for this already a few days ago.
I don't know if it really makes sense to change the rules for this now, 
apparently people are using this, it may not be perfect, but I think in 
the end it's just a matter of taste.

bye, Roman



[PATCH] refix localversion handling

This reverts part of the localversion patch, which now already got into
git. It removes the unnecessary find call, with the simpler $(notdir ...)
fix.

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

---
 Makefile |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Index: linux-2.6/Makefile
===================================================================
--- linux-2.6.orig/Makefile
+++ linux-2.6/Makefile
@@ -787,12 +787,14 @@ $(vmlinux-dirs): prepare scripts
 # moment, only git is supported but other SCMs can edit the script
 # scripts/setlocalversion and add the appropriate checks as needed.
 
-pattern = ".*/localversion[^~]*"
-string  = $(shell cat /dev/null \
-	   `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort`)
-
-localver = $(subst $(space),, $(string) \
-			      $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
+___localver = $(objtree)/localversion* $(srctree)/localversion*
+__localver  = $(sort $(wildcard $(___localver)))
+# skip files containing '~' (like backup files)
+_localver = $(foreach f,$(__localver),$(if $(findstring ~,$(notdir $(f))),,$(f)))
+
+localver = $(subst $(space),, \
+	   $(shell cat /dev/null $(_localver)) \
+	   $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
 
 # If CONFIG_LOCALVERSION_AUTO is set scripts/setlocalversion is called
 # and if the SCM is know a tag from the SCM is appended.

kbuild, localversion (Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files)

[Oleg Verych]

Hallo!

On Tue, Feb 13, 2007 at 05:09:47PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Tue, 13 Feb 2007, Linus Torvalds wrote:
> 
> > > I know it maybe another my "change it all" proposition, but i can't find
> > > nothing against `GNU $(wildcard ..)' and `unnecessarily complex "find"'.
> > 
> > It's the regexp in both cases. $(wildcard ) doesn't do regexp's (only the 
> > normal path rules), and traditional 'find' doesn't either. The fact that 
> > GNU find does is another matter. I don't think we require GNU find 
> > normally.
> > 
> > And I don't even much like the "backup" thing. Some programs will use 
> > other things than "~" as a backup marker. Patch more often uses ".orig", 
> > for example. So both methods are fairly complex, but at the same time not 
> > quite complex enough.
> > 
> > It would probably have been a better idea had we made the rule be that the 
> > file is called "*localversion" rather than "localversion*", exactly 
> > because that way it's unambiguous (people normally use _suffixes_ for 
> > filetypes, not prefixes). That would have avoided the whole complexity in 
> > wildcarding, but it's too late now..
> > 
> > 	$(sort $(wildcard $(srctree)/*localversion $(objtree)/*localversion)
> > 
> > should have worked.

As part of my personal preparation for "a new kind of things" i
finally went to the same idea with suffixes.

What i currently have is:

-- top file 'Linux.version', with first line:

3.0.0-rcX
which can be parsed to fill variables, used in build process (how many
`.' and/or `-' in it -- doesn't really matter), second line is the name;

-- 'MM.version' for MM tree;

-- '[a-z]*\.version' for anything else.

usual sort will place files in right order.

> For now I think it's easier to just revert the change to use find, I 
> posted the patch for this already a few days ago.
> I don't know if it really makes sense to change the rules for this now, 
> apparently people are using this, it may not be perfect, but I think in 
> the end it's just a matter of taste.

At least we have some discussion. Unless Linus use `sed' for patching
Makefile for versions, i think, to edit one or two lines in 50kB monster,
isn't so much pleasure ;)

____

Re: kbuild, localversion (Re: [patch 3/3, resend] kbuild: correctly skip tilded backups in localversion files)

[Sam Ravnborg]

> 
> What i currently have is:
> 
> -- top file 'Linux.version', with first line:
> 
> 3.0.0-rcX
> which can be parsed to fill variables, used in build process (how many
> `.' and/or `-' in it -- doesn't really matter), second line is the name;
> 
> -- 'MM.version' for MM tree;
> 
> -- '[a-z]*\.version' for anything else.
> 
> usual sort will place files in right order.

Too much depend on having version in Makefile so this is a no-go to change.
This has been like this since very early days so we shall not change it.

	Sam

Re: [patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)

[Randy Dunlap]

On Tue, 06 Feb 2007 02:18:19 +0100 Oleg Verych wrote:

> Due to almost zer0 activity, this may be last attempt.

Are all of these patches in -mm for testing?

---
~Randy

Re: [patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)

[Oleg Verych]

On Tue, Feb 06, 2007 at 09:56:30AM -0800, Randy Dunlap wrote:
> On Tue, 06 Feb 2007 02:18:19 +0100 Oleg Verych wrote:
> 
> > Due to almost zer0 activity, this may be last attempt.
> 
> Are all of these patches in -mm for testing?

Nope.

Roman Zippel joined, when i've posted one of them for testing. His
comments were (only) available at that time. And they led me to better
solution as well (for option checking in particular). Nothing from him
since then.

Patch with localversion, where i assumed wrongly "localversion~is-ok" are
OK to be valid, changed to handle, what it should handle (see patched
comments).

Last one -- gawk with frieds, got strange comments about SUSv3
compliance. But, since i've posted my tests,

Message-Id: <20070203131011.GA26632@flower.upol.cz>
Archived-At: <http://permalink.gmane.org/gmane.linux.kernel/489286>

nobody replied.

> ---
> ~Randy
____

Re: [patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)

[Oleg Verych]

> From: Oleg Verych
> Newsgroups: gmane.linux.kernel
> Subject: [patch 0/3, resend] scripts, kbuild updates/fixes (against 2.6.20)
> Date: Tue, 06 Feb 2007 02:18:19 +0100

[]
> Due to almost zer0 activity, this may be last attempt.
>

Landed in -mm tree. Thank you, Andrew.

Happy testing, everyone. Your `acks' are required!

If something wrong with
-- external modules building on `ro' source tree (distros),
-- wrong CC options (arch maintainers),
-- wrong localversion(s) included (Debian ;),
-- something with initramfs or object list (makelst) generation,

i'm listening.

Thanks !

--
-o--=O`C  /. .\
 #oo'L O      o
<___=E M    ^--