Running make install over sshfs is painful now

Running make install over sshfs is painful now

[Dmitry Torokhov]

Hi,

With the following commit running "make modules_install install" over
sshfs (and I suppose nfs) is extremely painful:

commit fb994ecc2b1c214951366c2ba5d8b121f0010d1f
Author: Greg Thelen <gthelen@google.com>
Date:   Wed May 5 10:41:44 2010 -0700

    kbuild: Fix checking of scm-identifier variable
    
    I'm looking Makefile in the -mm branch (dated 2010-04-28-16-53) and
    seeing what looks like a bug in the checking of scm-identifier.  The
    "ifneq ($scm-identifier)" seems to always execute "ifeq
    ($(LOCALVERSION,)) ...".  This patch fixes the checking of
    scm-identifier.
    
    Signed-off-by: Greg Thelen <gthelen@google.com>
    Acked-by: David Rientjes <rientjes@google.com>
    Signed-off-by: Michal Marek <mmarek@suse.cz>

I compile my kernel on my workstation and then mount working tree over
sshfs and install on my laptop over wireless. It worked well for the
last few years but the latest changeset slows this process down to a
crawl - traversing git repository over the network is not the fastest
process out there. Any cfhance we could generate the tag at compile time
as opposed to install time?

Thanks.

-- 
Dmitry

Re: Running make install over sshfs is painful now

[Michal Marek]

On 7.6.2010 08:53, Dmitry Torokhov wrote:
> Hi,
> 
> With the following commit running "make modules_install install" over
> sshfs (and I suppose nfs) is extremely painful:
> 
> commit fb994ecc2b1c214951366c2ba5d8b121f0010d1f
> Author: Greg Thelen <gthelen@google.com>
> Date:   Wed May 5 10:41:44 2010 -0700
> 
>     kbuild: Fix checking of scm-identifier variable
...

Commit 85a256d8 and this one just exposed a problem we had before with
CONFIG_LOCALVERSION_AUTO, namely that the Makefile computes the release
string on _every_ invocation:

$ strace -feexecve make help 2>&1 | grep -v ' -1 ' | grep git
[pid 13788] execve("/usr/bin/git", ["git", "rev-parse", "--verify",
"--short", "HEAD"], [/* 88 vars */]) = 0
[pid 13790] execve("/usr/bin/git", ["git", "describe", "--exact-match"],
[/* 88 vars */]) = 0
[pid 13792] execve("/usr/bin/git", ["git", "describe"], [/* 88 vars */]) = 0
[pid 13795] execve("/usr/bin/git", ["git", "config", "--get",
"svn-remote.svn.url"], [/* 88 vars */]) = 0
[pid 13796] execve("/usr/bin/git", ["git", "update-index", "--refresh",
"--unmerged"], [/* 88 vars */]) = 0
[pid 13797] execve("/usr/bin/git", ["git", "diff-index", "--name-only",
"HEAD"], [/* 88 vars */]Process 13786 suspended

> Any cfhance we could generate the tag at compile time
> as opposed to install time?

The Makefile already stores the string in include/config/kernel.release
during 'make prepare', but it is nevertheless recomputed every time.
Need to clean this up.

Michal

[PATCH] kbuild: Clean up and speed up the localversion logic

[Michal Marek]

Now that we run scripts/setlocalversion during every build, it makes
sense to move all the localversion logic there. This cleans up the
toplevel Makefile and also makes sure that the script is called only
once in 'make prepare' (previously, it would be called every time due to
a variable expansion in an ifneq statement). No user-visible change is
intended, unless one runs the setlocalversion script directly.

Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Greg Thelen <gthelen@google.com>
Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---

I tried to test this in various scenarios, but if anyone of you could give
it a try, that would be great. The patch is against 2.6.35-rc3.

---
 Makefile                 |   74 +------------------
 scripts/package/Makefile |    2 +-
 scripts/setlocalversion  |  179 ++++++++++++++++++++++++++++++++++------------
 3 files changed, 136 insertions(+), 119 deletions(-)

diff --git a/Makefile b/Makefile
index d49d96c..f6d2cb2 100644
--- a/Makefile
+++ b/Makefile
@@ -883,80 +883,12 @@ PHONY += $(vmlinux-dirs)
 $(vmlinux-dirs): prepare scripts
 	$(Q)$(MAKE) $(build)=$@
 
-# Build the kernel release string
-#
-# The KERNELRELEASE value built here is stored in the file
-# include/config/kernel.release, and is used when executing several
-# make targets, such as "make install" or "make modules_install."
-#
-# The eventual kernel release string consists of the following fields,
-# shown in a hierarchical format to show how smaller parts are concatenated
-# to form the larger and final value, with values coming from places like
-# the Makefile, kernel config options, make command line options and/or
-# SCM tag information.
-#
-#	$(KERNELVERSION)
-#	  $(VERSION)			eg, 2
-#	  $(PATCHLEVEL)			eg, 6
-#	  $(SUBLEVEL)			eg, 18
-#	  $(EXTRAVERSION)		eg, -rc6
-#	$(localver-full)
-#	  $(localver)
-#	    localversion*		(files without backups, containing '~')
-#	    $(CONFIG_LOCALVERSION)	(from kernel config setting)
-#	  $(LOCALVERSION)		(from make command line, if provided)
-#	  $(localver-extra)
-#	    $(scm-identifier)		(unique SCM tag, if one exists)
-#	      ./scripts/setlocalversion	(only with CONFIG_LOCALVERSION_AUTO)
-#	      .scmversion		(only with CONFIG_LOCALVERSION_AUTO)
-#	    +				(only without CONFIG_LOCALVERSION_AUTO
-#					 and without LOCALVERSION= and
-#					 repository is at non-tagged commit)
-#
-# For kernels without CONFIG_LOCALVERSION_AUTO compiled from an SCM that has
-# been revised beyond a tagged commit, `+' is appended to the version string
-# when not overridden by using "make LOCALVERSION=".  This indicates that the
-# kernel is not a vanilla release version and has been modified.
-
-pattern = ".*/localversion[^~]*"
-string  = $(shell cat /dev/null \
-	   `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort -u`)
-
-localver = $(subst $(space),, $(string) \
-			      $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
-
-# scripts/setlocalversion is called to create a unique identifier if the source
-# is managed by a known SCM and the repository has been revised since the last
-# tagged (release) commit.  The format of the identifier is determined by the
-# SCM's implementation.
-#
-# .scmversion is used when generating rpm packages so we do not loose
-# the version information from the SCM when we do the build of the kernel
-# from the copied source
-ifeq ($(wildcard .scmversion),)
-        scm-identifier = $(shell $(CONFIG_SHELL) \
-                         $(srctree)/scripts/setlocalversion $(srctree))
-else
-        scm-identifier = $(shell cat .scmversion 2> /dev/null)
-endif
-
-ifdef CONFIG_LOCALVERSION_AUTO
-	localver-extra = $(scm-identifier)
-else
-	ifneq ($(scm-identifier),)
-		ifeq ($(LOCALVERSION),)
-			localver-extra = +
-		endif
-	endif
-endif
-
-localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
-
 # Store (new) KERNELRELASE string in include/config/kernel.release
-kernelrelease = $(KERNELVERSION)$(localver-full)
+localversion = $(shell $(CONFIG_SHELL) \
+	       $(srctree)/scripts/setlocalversion $(srctree))
 include/config/kernel.release: include/config/auto.conf FORCE
 	$(Q)rm -f $@
-	$(Q)echo $(kernelrelease) > $@
+	$(Q)echo $(KERNELVERSION)$(localversion) > $@
 
 
 # Things we need to do before we recursively start building the kernel
diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index 62fcc3a..3a681ef 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -44,7 +44,7 @@ rpm-pkg rpm: $(objtree)/kernel.spec FORCE
 	fi
 	$(MAKE) clean
 	$(PREV) ln -sf $(srctree) $(KERNELPATH)
-	$(CONFIG_SHELL) $(srctree)/scripts/setlocalversion > $(objtree)/.scmversion
+	$(CONFIG_SHELL) $(srctree)/scripts/setlocalversion --scm-only > $(objtree)/.scmversion
 	$(PREV) tar -cz $(RCS_TAR_IGNORE) -f $(KERNELPATH).tar.gz $(KERNELPATH)/.
 	$(PREV) rm $(KERNELPATH)
 	rm -f $(objtree)/.scmversion
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 46989b8..d6a866e 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -10,73 +10,158 @@
 #
 
 usage() {
-	echo "Usage: $0 [srctree]" >&2
+	echo "Usage: $0 [--scm-only] [srctree]" >&2
 	exit 1
 }
 
-cd "${1:-.}" || usage
+scm_only=false
+srctree=.
+if test "$1" = "--scm-only"; then
+	scm_only=true
+	shift
+fi
+if test $# -gt 0; then
+	srctree=$1
+	shift
+fi
+if test $# -gt 0 -o ! -d "$srctree"; then
+	usage
+fi
 
-# Check for git and a git repo.
-if head=`git rev-parse --verify --short HEAD 2>/dev/null`; then
+scm_version()
+{
+	local short=false
 
-	# If we are at a tagged commit (like "v2.6.30-rc6"), we ignore it,
-	# because this version is defined in the top level Makefile.
-	if [ -z "`git describe --exact-match 2>/dev/null`" ]; then
+	cd "$srctree"
+	if test -e .scmversion; then
+		cat "$_"
+		return
+	fi
+	if test "$1" = "--short"; then
+		short=true
+	fi
 
-		# If we are past a tagged commit (like "v2.6.30-rc5-302-g72357d5"),
-		# we pretty print it.
-		if atag="`git describe 2>/dev/null`"; then
-			echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),$(NF))}'
+	# Check for git and a git repo.
+	if head=`git rev-parse --verify --short HEAD 2>/dev/null`; then
+
+		# If we are at a tagged commit (like "v2.6.30-rc6"), we ignore
+		# it, because this version is defined in the top level Makefile.
+		if [ -z "`git describe --exact-match 2>/dev/null`" ]; then
+
+			# If only the short version is requested, don't bother
+			# running further git commands
+			if $short; then
+				echo "+"
+				return
+			fi
+			# If we are past a tagged commit (like
+			# "v2.6.30-rc5-302-g72357d5"), we pretty print it.
+			if atag="`git describe 2>/dev/null`"; then
+				echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),$(NF))}'
+
+			# If we don't have a tag at all we print -g{commitish}.
+			else
+				printf '%s%s' -g $head
+			fi
+		fi
 
-		# If we don't have a tag at all we print -g{commitish}.
-		else
-			printf '%s%s' -g $head
+		# Is this git on svn?
+		if git config --get svn-remote.svn.url >/dev/null; then
+			printf -- '-svn%s' "`git svn find-rev $head`"
 		fi
-	fi
 
-	# Is this git on svn?
-	if git config --get svn-remote.svn.url >/dev/null; then
-	        printf -- '-svn%s' "`git svn find-rev $head`"
-	fi
+		# Update index only on r/w media
+		[ -w . ] && git update-index --refresh --unmerged > /dev/null
 
-	# Update index only on r/w media
-	[ -w . ] && git update-index --refresh --unmerged > /dev/null
+		# Check for uncommitted changes
+		if git diff-index --name-only HEAD | grep -v "^scripts/package" \
+		    | read dummy; then
+			printf '%s' -dirty
+		fi
 
-	# Check for uncommitted changes
-	if git diff-index --name-only HEAD | grep -v "^scripts/package" \
-	    | read dummy; then
-		printf '%s' -dirty
+		# All done with git
+		return
 	fi
 
-	# All done with git
-	exit
-fi
+	# Check for mercurial and a mercurial repo.
+	if hgid=`hg id 2>/dev/null`; then
+		tag=`printf '%s' "$hgid" | cut -d' ' -f2`
+
+		# Do we have an untagged version?
+		if [ -z "$tag" -o "$tag" = tip ]; then
+			id=`printf '%s' "$hgid" | sed 's/[+ ].*//'`
+			printf '%s%s' -hg "$id"
+		fi
 
-# Check for mercurial and a mercurial repo.
-if hgid=`hg id 2>/dev/null`; then
-	tag=`printf '%s' "$hgid" | cut -d' ' -f2`
+		# Are there uncommitted changes?
+		# These are represented by + after the changeset id.
+		case "$hgid" in
+			*+|*+\ *) printf '%s' -dirty ;;
+		esac
 
-	# Do we have an untagged version?
-	if [ -z "$tag" -o "$tag" = tip ]; then
-		id=`printf '%s' "$hgid" | sed 's/[+ ].*//'`
-		printf '%s%s' -hg "$id"
+		# All done with mercurial
+		return
 	fi
 
-	# Are there uncommitted changes?
-	# These are represented by + after the changeset id.
-	case "$hgid" in
-		*+|*+\ *) printf '%s' -dirty ;;
-	esac
+	# Check for svn and a svn repo.
+	if rev=`svn info 2>/dev/null | grep '^Last Changed Rev'`; then
+		rev=`echo $rev | awk '{print $NF}'`
+		printf -- '-svn%s' "$rev"
 
-	# All done with mercurial
+		# All done with svn
+		return
+	fi
+}
+
+collect_files()
+{
+	local file res
+
+	for file; do
+		case "$file" in
+		*\~*)
+			continue
+			;;
+		esac
+		if test -e "$file"; then
+			res="$res$(cat "$file")"
+		fi
+	done
+	echo "$res"
+}
+
+if $scm_only; then
+	scm_version
 	exit
 fi
 
-# Check for svn and a svn repo.
-if rev=`svn info 2>/dev/null | grep '^Last Changed Rev'`; then
-	rev=`echo $rev | awk '{print $NF}'`
-	printf -- '-svn%s' "$rev"
+if test -e include/config/auto.conf; then
+	source "$_"
+else
+	echo "Error: kernelrelease not valid - run 'make prepare' to update it"
+	exit 1
+fi
 
-	# All done with svn
-	exit
+# localversion* files in the build and source directory
+res="$(collect_files localversion*)"
+if test ! "$srctree" -ef .; then
+	res="$res$(collect_files "$srctree"/localversion*)"
+fi
+
+# CONFIG_LOCALVERSION and LOCALVERSION (if set)
+res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
+
+# scm version string if not at a tagged commit
+if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
+	# full scm version string
+	res="$res$(scm_version)"
+else
+	# apped a plus sign if the repository is not in a clean tagged
+	# state and  LOCALVERSION= is not specified
+	if test "${LOCALVERSION+set}" != "set"; then
+		scm=$(scm_version --short)
+		res="$res${scm:++}"
+	fi
 fi
+
+echo "$res"
-- 
1.7.1

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[David Rientjes]

On Thu, 17 Jun 2010, Michal Marek wrote:

> Now that we run scripts/setlocalversion during every build, it makes
> sense to move all the localversion logic there. This cleans up the
> toplevel Makefile and also makes sure that the script is called only
> once in 'make prepare' (previously, it would be called every time due to
> a variable expansion in an ifneq statement). No user-visible change is
> intended, unless one runs the setlocalversion script directly.
> 
> Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
> Signed-off-by: Michal Marek <mmarek@suse.cz>
> ---
> 
> I tried to test this in various scenarios, but if anyone of you could give
> it a try, that would be great. The patch is against 2.6.35-rc3.
> 

I agree it would be better to move this to scripts/setlocalversion, thanks 
for looking into it.  I'll put this into my build cycles and see if it 
causes any issues in my workflow that is known to be good with 2.6.35-rc3.  
Since this is 2.6.36 material, it seems like we have some time.

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Dmitry Torokhov]

On Thu, Jun 17, 2010 at 04:05:05PM -0700, David Rientjes wrote:
> On Thu, 17 Jun 2010, Michal Marek wrote:
> 
> > Now that we run scripts/setlocalversion during every build, it makes
> > sense to move all the localversion logic there. This cleans up the
> > toplevel Makefile and also makes sure that the script is called only
> > once in 'make prepare' (previously, it would be called every time due to
> > a variable expansion in an ifneq statement). No user-visible change is
> > intended, unless one runs the setlocalversion script directly.
> > 
> > Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Greg Thelen <gthelen@google.com>
> > Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
> > Signed-off-by: Michal Marek <mmarek@suse.cz>
> > ---
> > 
> > I tried to test this in various scenarios, but if anyone of you could give
> > it a try, that would be great. The patch is against 2.6.35-rc3.
> > 
> 
> I agree it would be better to move this to scripts/setlocalversion, thanks 
> for looking into it.  I'll put this into my build cycles and see if it 
> causes any issues in my workflow that is known to be good with 2.6.35-rc3.  
> Since this is 2.6.36 material, it seems like we have some time.

No, I am sorry, it is not .36 material as the building on remote box was
messed up in .35 cycle.

-- 
Dmitry

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Dmitry Torokhov]

On Thu, Jun 17, 2010 at 03:40:38PM +0200, Michal Marek wrote:
> Now that we run scripts/setlocalversion during every build, it makes
> sense to move all the localversion logic there. This cleans up the
> toplevel Makefile and also makes sure that the script is called only
> once in 'make prepare' (previously, it would be called every time due to
> a variable expansion in an ifneq statement). No user-visible change is
> intended, unless one runs the setlocalversion script directly.
> 
> Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
> Signed-off-by: Michal Marek <mmarek@suse.cz>
> ---
> 
> I tried to test this in various scenarios, but if anyone of you could give
> it a try, that would be great. The patch is against 2.6.35-rc3.
> 

Seems to be working for me, I do not need to go and fetch coffee while
installig newly compiled kernel anymore.

Thanks for your help Michal.

-- 
Dmitry

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Dmitry Torokhov]

On Thu, Jun 17, 2010 at 10:55:12PM -0700, Dmitry Torokhov wrote:
> On Thu, Jun 17, 2010 at 03:40:38PM +0200, Michal Marek wrote:
> > Now that we run scripts/setlocalversion during every build, it makes
> > sense to move all the localversion logic there. This cleans up the
> > toplevel Makefile and also makes sure that the script is called only
> > once in 'make prepare' (previously, it would be called every time due to
> > a variable expansion in an ifneq statement). No user-visible change is
> > intended, unless one runs the setlocalversion script directly.
> > 
> > Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Greg Thelen <gthelen@google.com>
> > Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
> > Signed-off-by: Michal Marek <mmarek@suse.cz>
> > ---
> > 
> > I tried to test this in various scenarios, but if anyone of you could give
> > it a try, that would be great. The patch is against 2.6.35-rc3.
> > 
> 
> Seems to be working for me, I do not need to go and fetch coffee while
> installig newly compiled kernel anymore.
> 

Michal,

Any chance this could be merged in 2.6.35? Without the patch installing
over the network is not really an option anymore.

Thanks.

-- 
Dmitry

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Michal Marek]

On Tue, Jun 29, 2010 at 02:54:21PM -0700, Dmitry Torokhov wrote:
> On Thu, Jun 17, 2010 at 10:55:12PM -0700, Dmitry Torokhov wrote:
> > On Thu, Jun 17, 2010 at 03:40:38PM +0200, Michal Marek wrote:
> > > Now that we run scripts/setlocalversion during every build, it makes
> > > sense to move all the localversion logic there. This cleans up the
> > > toplevel Makefile and also makes sure that the script is called only
> > > once in 'make prepare' (previously, it would be called every time due to
> > > a variable expansion in an ifneq statement). No user-visible change is
> > > intended, unless one runs the setlocalversion script directly.
> > > 
> > > Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: David Rientjes <rientjes@google.com>
> > > Cc: Greg Thelen <gthelen@google.com>
> > > Cc: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>
> > > Signed-off-by: Michal Marek <mmarek@suse.cz>
> > > ---
> > > 
> > > I tried to test this in various scenarios, but if anyone of you could give
> > > it a try, that would be great. The patch is against 2.6.35-rc3.
> > > 
> > 
> > Seems to be working for me, I do not need to go and fetch coffee while
> > installig newly compiled kernel anymore.
> > 
> 
> Michal,
> 
> Any chance this could be merged in 2.6.35? Without the patch installing
> over the network is not really an option anymore.

I now found that my patch breaks 'make LOCALVERSION=... '. The following
patch should fix it.

Michal


commit 0a564b2645c8766a669c55bde1f1ef5b0518caec
Author: Michal Marek <mmarek@suse.cz>
Date:   Wed Jun 30 16:41:23 2010 +0200

    kbuild: Propagate LOCALVERSION= down to scripts/setlocalversion
    
    Variables given on the make commandline are not exported to $(shell
    ...) commands, so run the setlocalversion script in the make rule
    directly.
    
    Signed-off-by: Michal Marek <mmarek@suse.cz>

diff --git a/Makefile b/Makefile
index a86ac8c..12ab175 100644
--- a/Makefile
+++ b/Makefile
@@ -884,11 +884,9 @@ $(vmlinux-dirs): prepare scripts
 	$(Q)$(MAKE) $(build)=$@
 
 # Store (new) KERNELRELASE string in include/config/kernel.release
-localversion = $(shell $(CONFIG_SHELL) \
-	       $(srctree)/scripts/setlocalversion $(srctree))
 include/config/kernel.release: include/config/auto.conf FORCE
 	$(Q)rm -f $@
-	$(Q)echo $(KERNELVERSION)$(localversion) > $@
+	$(Q)echo "$(KERNELVERSION)$$($(CONFIG_SHELL) scripts/setlocalversion $(srctree))" > $@
 
 
 # Things we need to do before we recursively start building the kernel

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Nico Schottelius]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

Michal Marek [Thu, Jun 17, 2010 at 03:40:38PM +0200]:
> Now that we run scripts/setlocalversion during every build, it makes
> sense to move all the localversion logic there. This cleans up the
> toplevel Makefile and also makes sure that the script is called only
> once in 'make prepare' (previously, it would be called every time due to
> a variable expansion in an ifneq statement). No user-visible change is
> intended, unless one runs the setlocalversion script directly.

Sounds good in general.

> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -10,73 +10,158 @@
>  #
>  
>  usage() {
> -	echo "Usage: $0 [srctree]" >&2
> +	echo "Usage: $0 [--scm-only] [srctree]" >&2
>  	exit 1
>  }

If more stuff like this is coming, we should probably rewrite
option handling. No need to change right now, but to keep in mind.

> -cd "${1:-.}" || usage
> +scm_only=false
> +srctree=.
> +if test "$1" = "--scm-only"; then
> +	scm_only=true
> +	shift
> +fi
> +if test $# -gt 0; then
> +	srctree=$1

You probably want

srctree="$1" 

to catch spaces and co. in the path.

Otherwise I just had a quick view over it, but seems to look good.

Nico

-- 
New PGP key: 7ED9 F7D3 6B10 81D7 0EC5  5C09 D7DC C8E4 3187 7DF0
Please resign, if you signed 9885188C or 8D0E27A4.

Currently moving *.schottelius.org to http://www.nico.schottelius.org/ ...

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Michal Marek]

On 22.6.2010 12:44, Nico Schottelius wrote:
> Michal Marek [Thu, Jun 17, 2010 at 03:40:38PM +0200]:
>> -cd "${1:-.}" || usage
>> +scm_only=false
>> +srctree=.
>> +if test "$1" = "--scm-only"; then
>> +	scm_only=true
>> +	shift
>> +fi
>> +if test $# -gt 0; then
>> +	srctree=$1
> 
> You probably want
> 
> srctree="$1" 
> 
> to catch spaces and co. in the path.

That's the same, the value in variable assignment is not subject to word
splitting.


> Otherwise I just had a quick view over it, but seems to look good.

Thanks!

/me is now wondering if it's ok to push this for 2.6.35. At least, there
were no bugreports from linux-next users so far.

Michal

Re: [PATCH] kbuild: Clean up and speed up the localversion logic

[Nico Schottelius]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

Michal Marek [Tue, Jun 22, 2010 at 01:13:55PM +0200]:
> On 22.6.2010 12:44, Nico Schottelius wrote:
> > Michal Marek [Thu, Jun 17, 2010 at 03:40:38PM +0200]:
> > srctree="$1" 
> > 
> > to catch spaces and co. in the path.
> 
> That's the same, the value in variable assignment is not subject to word
> splitting.

Oh, indeed, you're right! Learned something!

> > Otherwise I just had a quick view over it, but seems to look good.
> 
> Thanks!
> 
> /me is now wondering if it's ok to push this for 2.6.35. At least, there
> were no bugreports from linux-next users so far.

Well, sometime you'll have to push it and I'm not sure how many
"no bugreports" you'll want to wait for :-)

Cheers,

Nico

-- 
New PGP key: 7ED9 F7D3 6B10 81D7 0EC5  5C09 D7DC C8E4 3187 7DF0
Please resign, if you signed 9885188C or 8D0E27A4.

Currently moving *.schottelius.org to http://www.nico.schottelius.org/ ...

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]