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 <[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.
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
--
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
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.
____
On 2/5/07, Oleg Verych <[email protected]> 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
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
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!
____
[]
> > 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"'.
____
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 <[email protected]>
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
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 <[email protected]>
---
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.
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 ;)
____
>
> 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