2007-01-31 07:02:34

by Oleg Verych

[permalink] [raw]
Subject: [patch] kbuild: correctly skip tilded backups in localversion files

kbuild: correctly skip tilded backups in localversion files

Tildes as in path as in filenames are handled correctly now.

Definition of `space' was removed, scripts/Kbuild.include has one.
This definition was taken right from GNU make manual, while Kbuild's
version is original.

Cc: Roman Zippel <[email protected]>
Cc: Bastian Blank <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Signed-off-by: Oleg Verych <[email protected]>
---
Another try.

Original report and fix 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 | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

--- linux-2.6.20-rc6/Makefile~4tilde-backups~ 2007-01-30 23:33:45.781462750 +0100
+++ linux-2.6.20-rc6/Makefile 2007-01-31 07:46:18.696404500 +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,14 +788,9 @@ $(vmlinux-dirs): prepare scripts
# scripts/setlocalversion and add the appropriate checks as needed.

-nullstring :=
-space := $(nullstring) # end of line
-
-___localver = $(objtree)/localversion* $(srctree)/localversion*
-__localver = $(sort $(wildcard $(___localver)))
-# skip backup files (containing '~')
-_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(f)),,$(f)))
-
+localversion = $(objtree)/localversion $(srctree)/localversion
+ext_versions = $(objtree)/localversion[^~]* $(srctree)/localversion[!~]*
+versions = $(localversion) $(ext_versions)
localver = $(subst $(space),, \
- $(shell cat /dev/null $(_localver)) \
+ $(shell cat /dev/null $(sort $(wildcard $(versions)))) \
$(patsubst "%",%,$(CONFIG_LOCALVERSION)))


2007-01-31 09:27:12

by Oleg Verych

[permalink] [raw]
Subject: [patch] update for (kbuild: correctly skip tilded backups in localversion files)

kbuild: finally correctly skip tilded backups in localversion files


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

Final addition. Why i want to maximize usage of shell, rather, than
`make'? Just because i think, it's more portable, clear way.

Thanks.

--- linux-2.6.20-rc6/Makefile~4tilde-backups~ 2007-01-31 07:46:18.696404500 +0100
+++ linux-2.6.20-rc6/Makefile 2007-01-31 10:19:18.406100500 +0100
@@ -788,10 +788,10 @@ $(vmlinux-dirs): prepare scripts
# scripts/setlocalversion and add the appropriate checks as needed.

-localversion = $(objtree)/localversion $(srctree)/localversion
-ext_versions = $(objtree)/localversion[^~]* $(srctree)/localversion[!~]*
-versions = $(localversion) $(ext_versions)
-localver = $(subst $(space),, \
- $(shell cat /dev/null $(sort $(wildcard $(versions)))) \
- $(patsubst "%",%,$(CONFIG_LOCALVERSION)))
+pattern = ".*/localversion[^~]*"
+string = $(shell cat /dev/null \
+ `find $(objtree) $(srctree) -maxdepth 1 -regex $(pattern) | sort`)
+
+localver = $(subst $(space),, $(string) \
+ $(patsubst "%",%,$(CONFIG_LOCALVERSION)))

# If CONFIG_LOCALVERSION_AUTO is set scripts/setlocalversion is called

2007-01-31 23:57:13

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] kbuild: correctly skip tilded backups in localversion files

On Wed, 31 Jan 2007 07:11:04 +0000
Oleg Verych <[email protected]> wrote:

> kbuild: correctly skip tilded backups in localversion files

Does this patch replace Bastian's patch, below?


From: Bastian Blank <[email protected]>

Fix the problem that localversion files were 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.

Cc: Sam Ravnborg <[email protected]>
Cc: Roman Zippel <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

Makefile | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN Makefile~kbuild-dont-ignore-localversion-files-if-the-path-includes-a Makefile
--- a/Makefile~kbuild-dont-ignore-localversion-files-if-the-path-includes-a
+++ a/Makefile
@@ -793,7 +793,7 @@ space := $(nullstring) # end of lin
___localver = $(objtree)/localversion* $(srctree)/localversion*
__localver = $(sort $(wildcard $(___localver)))
# skip backup files (containing '~')
-_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(f)),,$(f)))
+_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(notdir $(f))),,$(f)))

localver = $(subst $(space),, \
$(shell cat /dev/null $(_localver)) \
_

2007-02-01 02:29:10

by Oleg Verych

[permalink] [raw]
Subject: Re: [patch] kbuild: correctly skip tilded backups in localversion files

On Wed, Jan 31, 2007 at 03:56:51PM -0800, Andrew Morton wrote:
> On Wed, 31 Jan 2007 07:11:04 +0000
> Oleg Verych <[email protected]> wrote:
>
> > kbuild: correctly skip tilded backups in localversion files
>
> Does this patch replace Bastian's patch, below?

Along with "Message-ID: <[email protected]>" -- yes.

Note, that they also make a little bit of cleanup to top makefile's mess,
And, i think, have more elegant, portable, clean solution.

Roman commented on another my patch, saying, that tilde in the end isn't
the only way to name backups. Filename, containing '~' is a backup in the
sources, thus i'm proposing new patch.

Unfortunately still no more comments from anybody. Roman joined, but
seems busy now, to comment on such minor stuff.

Anyway, thanks.


> From: Bastian Blank <[email protected]>
>
> Fix the problem that localversion files were 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.
>
> Cc: Sam Ravnborg <[email protected]>
> Cc: Roman Zippel <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> Makefile | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> diff -puN Makefile~kbuild-dont-ignore-localversion-files-if-the-path-includes-a Makefile
> --- a/Makefile~kbuild-dont-ignore-localversion-files-if-the-path-includes-a
> +++ a/Makefile
> @@ -793,7 +793,7 @@ space := $(nullstring) # end of lin
> ___localver = $(objtree)/localversion* $(srctree)/localversion*
> __localver = $(sort $(wildcard $(___localver)))
> # skip backup files (containing '~')
> -_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(f)),,$(f)))
> +_localver = $(foreach f, $(__localver), $(if $(findstring ~, $(notdir $(f))),,$(f)))
>
> localver = $(subst $(space),, \
> $(shell cat /dev/null $(_localver)) \
> _
>

2007-02-01 02:43:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] kbuild: correctly skip tilded backups in localversion files

On Thu, 1 Feb 2007 03:37:17 +0100 Oleg Verych <[email protected]> wrote:

> On Wed, Jan 31, 2007 at 03:56:51PM -0800, Andrew Morton wrote:
> > On Wed, 31 Jan 2007 07:11:04 +0000
> > Oleg Verych <[email protected]> wrote:
> >
> > > kbuild: correctly skip tilded backups in localversion files
> >
> > Does this patch replace Bastian's patch, below?
>
> Along with "Message-ID: <[email protected]>" -- yes.
>

OK. Just to avoid mistakes, can you please resend everything, against
2.6.20-rc7?

2007-02-01 15:51:17

by Oleg Verych

[permalink] [raw]
Subject: 2.6.20-rc7 (Re: [patch] kbuild: correctly skip tilded backups in localversion files)

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.
This definition was taken right from GNU make manual, while Kbuild's
version is original.

Cc: Roman Zippel <[email protected]>
Cc: Bastian Blank <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Signed-off-by: Oleg Verych <[email protected]>
---

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.

---
linux-2.6.20-rc7/Makefile | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)

Index: kernel.org/linux-2.6.20-rc7/Makefile
===================================================================
--- kernel.org.orig/linux-2.6.20-rc7/Makefile 2007-02-01 16:20:54.214174250 +0100
+++ kernel.org/linux-2.6.20-rc7/Makefile 2007-02-01 16:21:40.649076250 +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

2007-02-04 13:52:30

by Oleg Verych

[permalink] [raw]
Subject: Re: [patch] kbuild: correctly skip tilded backups in localversion files

On Wed, Jan 31, 2007 at 06:43:29PM -0800, Andrew Morton wrote:
> On Thu, 1 Feb 2007 03:37:17 +0100 Oleg Verych <[email protected]> wrote:
>
> > On Wed, Jan 31, 2007 at 03:56:51PM -0800, Andrew Morton wrote:
> > > On Wed, 31 Jan 2007 07:11:04 +0000
> > > Oleg Verych <[email protected]> wrote:
> > >
> > > > kbuild: correctly skip tilded backups in localversion files
> > >
> > > Does this patch replace Bastian's patch, below?
> >
> > Along with "Message-ID: <[email protected]>" -- yes.
> >
>
> OK. Just to avoid mistakes, can you please resend everything, against
> 2.6.20-rc7?

I've resent patches, that i wanted to be in -mm, even without anyone,
who can ack or even test them. But it seems, they not made it to lkml.

____