[patch] kbuild: improve version string logic

[patch] kbuild: improve version string logic

[David Rientjes]

The LOCALVERSION= string passed to "make" will now always be appended to
the kernel version after CONFIG_LOCALVERSION, if it exists, regardless of
whether CONFIG_LOCALVERSION_AUTO is set or not.  This allows users to
uniquely identify their kernel builds with a string.

scripts/setlocalversion will now always be called to determine whether
the repository is beyond a tagged (release) commit.  When at a tagged
commit, this script will now suppress all output.

If CONFIG_LOCALVERSION_AUTO is enabled, the unique SCM tag reported by 
setlocalversion (or .scmversion) is appended to the kernel version, if it 
exists.  When CONFIG_LOCALVERSION_AUTO is not enabled, a `+' is appended
to the kernel version to represent that the kernel has been revised since
the last release unless "make LOCALVERSION=" was used to uniquely identify
the build.

The end result is this:

 - when LOCALVERSION= is passed to "make", it is appended to the kernel
   version,

 - when CONFIG_LOCALVERSION_AUTO is enabled, a unique SCM identifier is
   appended if the respository has been revised beyond a tagged commit,
   and

 - when CONFIG_LOCALVERSION_AUTO is disabled, a `+' is appended if the 
   repository has been revised beyond a tagged commit and LOCALVERSION=
   was not passed to "make".

Examples:

With CONFIG_LOCALVERSION_AUTO: "make" results in
v2.6.32-rc4-00149-ga3ccf63.  If there are uncommited changes to the
respository, it results in v2.6.32-rc4-00149-ga3ccf63-dirty.  If
"make LOCALVERSION=kbuild" were used, it results in
v2.6.32-rc4-kbuild-00149-ga3ccf63-dirty.

Without CONFIG_LOCALVERSION_AUTO, "make" results in v2.6.32-rc4+
unless the repository is at the Linux v2.6.32-rc4 commit (in which
case the version would be v2.6.32-rc4).  If "make LOCALVERSION=kbuild"
were used, it results in v2.6.32-rc4-kbuild.

Also renames variables such as localver-auto and _localver-auto to more 
accurately describe what they represent: localver-extra and
scm-identifier, respectively.

Acked-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 Makefile                |   43 +++++++++++++++++++++++++++----------------
 scripts/setlocalversion |    3 ++-
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -898,14 +898,19 @@ $(vmlinux-dirs): prepare scripts
 #	  $(localver)
 #	    localversion*		(files without backups, containing '~')
 #	    $(CONFIG_LOCALVERSION)	(from kernel config setting)
-#	  $(localver-auto)		(only if CONFIG_LOCALVERSION_AUTO is set)
-#	    ./scripts/setlocalversion	(SCM tag, if one exists)
-#	    $(LOCALVERSION)		(from make command line if provided)
+#	    $(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)
 #
-#  Note how the final $(localver-auto) string is included *only* if the
-# kernel config option CONFIG_LOCALVERSION_AUTO is selected.  Also, at the
-# moment, only git is supported but other SCMs can edit the script
-# scripts/setlocalversion and add the appropriate checks as needed.
+# 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 \
@@ -914,26 +919,32 @@ string  = $(shell cat /dev/null \
 localver = $(subst $(space),, $(string) \
 			      $(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.
-# The appended tag is determined by the SCM used.
+# 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
-ifdef CONFIG_LOCALVERSION_AUTO
-
 ifeq ($(wildcard .scmversion),)
-        _localver-auto = $(shell $(CONFIG_SHELL) \
+        scm-identifier = $(shell $(CONFIG_SHELL) \
                          $(srctree)/scripts/setlocalversion $(srctree))
 else
-        _localver-auto = $(shell cat .scmversion 2> /dev/null)
+        scm-identifier = $(shell cat .scmversion 2> /dev/null)
 endif
 
-	localver-auto  = $(LOCALVERSION)$(_localver-auto)
+ifdef CONFIG_LOCALVERSION_AUTO
+	localver-extra = $(scm-identifier)
+else
+	ifneq ($scm-identifier,)
+		ifeq ($(LOCALVERSION),)
+			localver-extra = +
+		endif
+	endif
 endif
 
-localver-full = $(localver)$(localver-auto)
+localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
 
 # Store (new) KERNELRELASE string in include/config/kernel.release
 kernelrelease = $(KERNELVERSION)$(localver-full)
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -26,7 +26,8 @@ if head=`git rev-parse --verify --short HEAD 2>/dev/null`; then
 		# 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))}'
+			echo "$atag" | awk -F- 'int($(NF-1)) > 0	\
+					{printf("-%05d-%s", $(NF-1),$(NF))}'
 
 		# If we don't have a tag at all we print -g{commitish}.
 		else

Re: [patch] kbuild: improve version string logic

[Michal Marek]

On 4.1.2010 22:31, David Rientjes wrote:
> The LOCALVERSION= string passed to "make" will now always be appended to
> the kernel version after CONFIG_LOCALVERSION, if it exists, regardless of
> whether CONFIG_LOCALVERSION_AUTO is set or not.  This allows users to
> uniquely identify their kernel builds with a string.
> 
> scripts/setlocalversion will now always be called to determine whether
> the repository is beyond a tagged (release) commit.  When at a tagged
> commit, this script will now suppress all output.
> 
> If CONFIG_LOCALVERSION_AUTO is enabled, the unique SCM tag reported by 
> setlocalversion (or .scmversion) is appended to the kernel version, if it 
> exists.  When CONFIG_LOCALVERSION_AUTO is not enabled, a `+' is appended
> to the kernel version to represent that the kernel has been revised since
> the last release unless "make LOCALVERSION=" was used to uniquely identify
> the build.
> 
> The end result is this:
> 
>  - when LOCALVERSION= is passed to "make", it is appended to the kernel
>    version,
> 
>  - when CONFIG_LOCALVERSION_AUTO is enabled, a unique SCM identifier is
>    appended if the respository has been revised beyond a tagged commit,
>    and
> 
>  - when CONFIG_LOCALVERSION_AUTO is disabled, a `+' is appended if the 
>    repository has been revised beyond a tagged commit and LOCALVERSION=
>    was not passed to "make".

I've read the thread from last October where this was discussed
(http://thread.gmane.org/gmane.linux.kernel/897768/focus=898233). I'm
not entirely sure if all people will be happy about the plus sign that
can't be turned off, but from the thread I got the impression that the
plus sign is wanted, so let's add this to linux-next now, so that people
doing automated builds can adjust their scripts if needed. Just in case:
Stephen, won't this break your linux-next scripts in any way? The final
build won't change, because of the next-YYYYMMDD tag, but the builds in
between will have the '+' appended to their version.


Two small comments are below.

> Examples:
> 
> With CONFIG_LOCALVERSION_AUTO: "make" results in
> v2.6.32-rc4-00149-ga3ccf63.  If there are uncommited changes to the
> respository, it results in v2.6.32-rc4-00149-ga3ccf63-dirty.  If
> "make LOCALVERSION=kbuild" were used, it results in
> v2.6.32-rc4-kbuild-00149-ga3ccf63-dirty.
> 
> Without CONFIG_LOCALVERSION_AUTO, "make" results in v2.6.32-rc4+
> unless the repository is at the Linux v2.6.32-rc4 commit (in which
> case the version would be v2.6.32-rc4).  If "make LOCALVERSION=kbuild"
> were used, it results in v2.6.32-rc4-kbuild.
> 
> Also renames variables such as localver-auto and _localver-auto to more 
> accurately describe what they represent: localver-extra and
> scm-identifier, respectively.
> 
> Acked-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  Makefile                |   43 +++++++++++++++++++++++++++----------------
>  scripts/setlocalversion |    3 ++-
>  2 files changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -898,14 +898,19 @@ $(vmlinux-dirs): prepare scripts
>  #	  $(localver)
>  #	    localversion*		(files without backups, containing '~')
>  #	    $(CONFIG_LOCALVERSION)	(from kernel config setting)
> -#	  $(localver-auto)		(only if CONFIG_LOCALVERSION_AUTO is set)
> -#	    ./scripts/setlocalversion	(SCM tag, if one exists)
> -#	    $(LOCALVERSION)		(from make command line if provided)
> +#	    $(LOCALVERSION)		(from make command line, if provided)

$(LOCALVERSION) is no longer part of $(localver), it should be indented
at the same level.


> +#	  $(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)
>  #
> -#  Note how the final $(localver-auto) string is included *only* if the
> -# kernel config option CONFIG_LOCALVERSION_AUTO is selected.  Also, at the
> -# moment, only git is supported but other SCMs can edit the script
> -# scripts/setlocalversion and add the appropriate checks as needed.
> +# 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 \
> @@ -914,26 +919,32 @@ string  = $(shell cat /dev/null \
>  localver = $(subst $(space),, $(string) \
>  			      $(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.
> -# The appended tag is determined by the SCM used.
> +# 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
> -ifdef CONFIG_LOCALVERSION_AUTO
> -
>  ifeq ($(wildcard .scmversion),)
> -        _localver-auto = $(shell $(CONFIG_SHELL) \
> +        scm-identifier = $(shell $(CONFIG_SHELL) \
>                           $(srctree)/scripts/setlocalversion $(srctree))
>  else
> -        _localver-auto = $(shell cat .scmversion 2> /dev/null)
> +        scm-identifier = $(shell cat .scmversion 2> /dev/null)
>  endif
>  
> -	localver-auto  = $(LOCALVERSION)$(_localver-auto)
> +ifdef CONFIG_LOCALVERSION_AUTO
> +	localver-extra = $(scm-identifier)
> +else
> +	ifneq ($scm-identifier,)
> +		ifeq ($(LOCALVERSION),)
> +			localver-extra = +
> +		endif
> +	endif
>  endif
>  
> -localver-full = $(localver)$(localver-auto)
> +localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
>  
>  # Store (new) KERNELRELASE string in include/config/kernel.release
>  kernelrelease = $(KERNELVERSION)$(localver-full)
> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -26,7 +26,8 @@ if head=`git rev-parse --verify --short HEAD 2>/dev/null`; then
>  		# 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))}'
> +			echo "$atag" | awk -F- 'int($(NF-1)) > 0	\
> +					{printf("-%05d-%s", $(NF-1),$(NF))}'
>
>  		# If we don't have a tag at all we print -g{commitish}.
>  		else

Why is this needed? There is

	# 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

a few lines above and even without your patch ./scripts/setlocalversion
returns nothing when HEAD is tagged. Do you have an old git version that
doesn't undestand --exact-match? In that case, the if around this should
be removed.

thanks,
Michal

Re: [patch] kbuild: improve version string logic

[Stephen Rothwell]

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

Hi Michal,

On Tue, 05 Jan 2010 14:59:11 +0100 Michal Marek <mmarek@suse.cz> wrote:
>
> Stephen, won't this break your linux-next scripts in any way? The final
> build won't change, because of the next-YYYYMMDD tag, but the builds in
> between will have the '+' appended to their version.

I think it should be OK - I don't depend on the kernel version for
anything until the very end.  If it does cause a problem, I am now
forwarned and can fix my scripts.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

[patch] kbuild: improve version string logic

[David Rientjes]

The LOCALVERSION= string passed to "make" will now always be appended to
the kernel version after CONFIG_LOCALVERSION, if it exists, regardless of
whether CONFIG_LOCALVERSION_AUTO is set or not.  This allows users to
uniquely identify their kernel builds with a string.

If CONFIG_LOCALVERSION_AUTO is enabled, the unique SCM tag reported by 
setlocalversion (or .scmversion) is appended to the kernel version, if it 
exists.  When CONFIG_LOCALVERSION_AUTO is not enabled, a `+' is appended
to the kernel version to represent that the kernel has been revised since
the last release unless "make LOCALVERSION=" was used to uniquely identify
the build.

The end result is this:

 - when LOCALVERSION= is passed to "make", it is appended to the kernel
   version,

 - when CONFIG_LOCALVERSION_AUTO is enabled, a unique SCM identifier is
   appended if the respository has been revised beyond a tagged commit,
   and

 - when CONFIG_LOCALVERSION_AUTO is disabled, a `+' is appended if the 
   repository has been revised beyond a tagged commit and LOCALVERSION=
   was not passed to "make".

Examples:

With CONFIG_LOCALVERSION_AUTO: "make" results in
v2.6.32-rc4-00149-ga3ccf63.  If there are uncommited changes to the
respository, it results in v2.6.32-rc4-00149-ga3ccf63-dirty.  If
"make LOCALVERSION=kbuild" were used, it results in
v2.6.32-rc4-kbuild-00149-ga3ccf63-dirty.

Without CONFIG_LOCALVERSION_AUTO, "make" results in v2.6.32-rc4+
unless the repository is at the Linux v2.6.32-rc4 commit (in which
case the version would be v2.6.32-rc4).  If "make LOCALVERSION=kbuild"
were used, it results in v2.6.32-rc4-kbuild.

Also renames variables such as localver-auto and _localver-auto to more 
accurately describe what they represent: localver-extra and
scm-identifier, respectively.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Makefile |   43 +++++++++++++++++++++++++++----------------
 1 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -908,14 +908,19 @@ endif
 #	  $(localver)
 #	    localversion*		(files without backups, containing '~')
 #	    $(CONFIG_LOCALVERSION)	(from kernel config setting)
-#	  $(localver-auto)		(only if CONFIG_LOCALVERSION_AUTO is set)
-#	    ./scripts/setlocalversion	(SCM tag, if one exists)
-#	    $(LOCALVERSION)		(from make command line if provided)
+#	  $(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)
 #
-#  Note how the final $(localver-auto) string is included *only* if the
-# kernel config option CONFIG_LOCALVERSION_AUTO is selected.  Also, at the
-# moment, only git is supported but other SCMs can edit the script
-# scripts/setlocalversion and add the appropriate checks as needed.
+# 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 \
@@ -924,26 +929,32 @@ string  = $(shell cat /dev/null \
 localver = $(subst $(space),, $(string) \
 			      $(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.
-# The appended tag is determined by the SCM used.
+# 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
-ifdef CONFIG_LOCALVERSION_AUTO
-
 ifeq ($(wildcard .scmversion),)
-        _localver-auto = $(shell $(CONFIG_SHELL) \
+        scm-identifier = $(shell $(CONFIG_SHELL) \
                          $(srctree)/scripts/setlocalversion $(srctree))
 else
-        _localver-auto = $(shell cat .scmversion 2> /dev/null)
+        scm-identifier = $(shell cat .scmversion 2> /dev/null)
 endif
 
-	localver-auto  = $(LOCALVERSION)$(_localver-auto)
+ifdef CONFIG_LOCALVERSION_AUTO
+	localver-extra = $(scm-identifier)
+else
+	ifneq ($scm-identifier,)
+		ifeq ($(LOCALVERSION),)
+			localver-extra = +
+		endif
+	endif
 endif
 
-localver-full = $(localver)$(localver-auto)
+localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
 
 # Store (new) KERNELRELASE string in include/config/kernel.release
 kernelrelease = $(KERNELVERSION)$(localver-full)

Re: [patch] kbuild: improve version string logic

[Ingo Molnar]

* David Rientjes <rientjes@google.com> wrote:

> The LOCALVERSION= string passed to "make" will now always be appended to the 
> kernel version after CONFIG_LOCALVERSION, if it exists, regardless of 
> whether CONFIG_LOCALVERSION_AUTO is set or not.  This allows users to 
> uniquely identify their kernel builds with a string.

I've been using Linus's earlier patch for 3 months meanwhile in -tip and the 
'+' version string differentiator is very useful in practice. Your version of 
the patch should address some of the observations offered in the original 
discussion as well. Would be nice to have your patch upstream (in the next 
merge window or so).

Given that Sam doesnt maintain the kbuild tree anymore, would this patch be a 
good fit for -mm perhaps?

	Ingo

Re: [patch] kbuild: improve version string logic

[Stephen Rothwell]

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

Hi Ingo,

On Sat, 16 Jan 2010 07:37:54 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> Given that Sam doesnt maintain the kbuild tree anymore, would this patch be a 
> good fit for -mm perhaps?

There is a new kbuild tree maintainer: Michal Marek <mmarek@suse.cz>

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [patch] kbuild: improve version string logic

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Ingo,
> 
> On Sat, 16 Jan 2010 07:37:54 +0100 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > Given that Sam doesnt maintain the kbuild tree anymore, would this patch be a 
> > good fit for -mm perhaps?
> 
> There is a new kbuild tree maintainer: Michal Marek <mmarek@suse.cz>

Good. Michal, what do you think about David's patch?

	Ingo

Re: [patch] kbuild: improve version string logic

[Michal Marek]

On 16.1.2010 10:26, Ingo Molnar wrote:
> 
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
>> Hi Ingo,
>>
>> On Sat, 16 Jan 2010 07:37:54 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>> Given that Sam doesnt maintain the kbuild tree anymore, would this patch be a 
>>> good fit for -mm perhaps?
>>
>> There is a new kbuild tree maintainer: Michal Marek <mmarek@suse.cz>
> 
> Good. Michal, what do you think about David's patch?

The patch is OK, I already commented on a previous version and this
addresses the two minor issues I had. I was just too busy last week to
apply it. I will do so ASAP now.

Michal

Re: [patch] kbuild: improve version string logic

[David Rientjes]

On Mon, 18 Jan 2010, Michal Marek wrote:

> >>> Given that Sam doesnt maintain the kbuild tree anymore, would this patch be a 
> >>> good fit for -mm perhaps?
> >>
> >> There is a new kbuild tree maintainer: Michal Marek <mmarek@suse.cz>
> > 
> > Good. Michal, what do you think about David's patch?
> 
> The patch is OK, I already commented on a previous version and this
> addresses the two minor issues I had. I was just too busy last week to
> apply it. I will do so ASAP now.
> 

Michal, where are you hosting your git tree?  Are you going to be taking 
over kbuild-next on git.kernel.org?

Re: [patch] kbuild: improve version string logic

[Michal Marek]

On 18.1.2010 11:45, David Rientjes wrote:
> Michal, where are you hosting your git tree?  Are you going to be taking 
> over kbuild-next on git.kernel.org?

git://repo.or.cz/linux-kbuild.git for-next
(http://repo.or.cz/w/linux-kbuild.git). Maybe I should ask for an
account in git.kernel.org account and move it there, you're not the
first one to wonder where to find the tree :).

Michal

Re: [patch] kbuild: improve version string logic

[Jiri Kosina]

On Mon, 18 Jan 2010, Michal Marek wrote:

> > Michal, where are you hosting your git tree?  Are you going to be taking 
> > over kbuild-next on git.kernel.org?
> 
> git://repo.or.cz/linux-kbuild.git for-next
> (http://repo.or.cz/w/linux-kbuild.git). Maybe I should ask for an
> account in git.kernel.org account and move it there, you're not the
> first one to wonder where to find the tree :).

Yes, I think it is preferred.

I know, I know, the proper URL is listed in MAINTAINERS file ... still, I 
believe that what most people do when looking for particular git tree is 
going to http://git.kernel.org/ and looking around there.

So having the tree there really helps to give it much more exposure than 
on some random git hosting.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [patch] kbuild: Improve version string logic

[David Rientjes]

On Tue, 6 Oct 2009, Ingo Molnar wrote:

> Subject: kbuild: Improve version string logic
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Tue, 6 Oct 2009 09:31:03 -0700 (PDT)
> 
> It changes how CONFIG_LOCALVERSION_AUTO works, in the following trivial
> way:
> 
>  - if it is set, things work the way they always have, and you get a
>    extended kernel release like:
> 
> 	2.6.32-rc3+00052-g0eca52a-dirty
> 
>    ( with the difference that the extra version string is separated via
>      '+' not via '-'. This improves visibility when we have additional
>      changes over a vanilla tag. )
> 

s/changes/commits/, we'd have to look at the -dirty suffix for uncommited 
changes.

The '+' doesn't necessarily always mean there are additional changes since 
scripts/setlocalversion will return -00000-EXTRAVERSION when we're at a 
release.  So when CONFIG_LOCALVERSION_AUTO is enabled, this patch would, 
for example, create the version 2.6.32-rc3+00000-rc3.

Perhaps we should suppress setlocalversion's output if git describe 
doesn't have a 7-character trailing hash prefixed with 'g'?

Re: [patch] kbuild: Improve version string logic

[Frans Pop]

This is a bit OT, but one last reply.

On Tuesday 06 October 2009, Johannes Berg wrote:
> > AFAIK it would be propagated into the package _name_, where
> > it is allowed. And even if it were part of the package version, it
> > would be in the "upstream" part of the version number, where it is
> > also allowed.
>
> That may all well be true. I wasn't even aware of the distinction
> between debian and upstream part of the version :)

The Debian part is everything after the last dash in a package version.

> I do, however, remember no end to trouble with autoversion and make-kpkg
> so eventually gave up on using it entirely.

Yes, problems with autoversion and make-kpkg I can understand as it makes 
the revision ID part of the package name, which is very simply not where 
you want it [1].

I stopped using make-kpkg for 2 reasons. First of all it is insanely 
complex. And second it used to always do a 'make clean' for every build, 
which meant a huge inefficiency during bisections and builds after minor 
changes; I understand that has been fixed though.

I was happy to discover the kernel's native deb-pkg target. There've been 
some improvements to that in recent kernels that make it quite powerful 
and flexible for building custom kernels as Debian packages.

Cheers,
FJP

[1] The deb-pkg target does the same, so autoversion is not usable with 
that either. What I do is manipulate the output of 'git describe' to set 
an ENVVAR that gets used as package version.

Re: [patch] kbuild: Improve version string logic

[Johannes Berg]

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

On Tue, 2009-10-06 at 21:45 +0200, Frans Pop wrote:
> Johannes Berg wrote:
> > I think adding the + unconditionally will also break things like
> > debian's make-kpkg, for instance, since the + would get propagated into
> > the package version which reserves the + for something else.
> 
> Are you sure? 

No, I'm not.

> AFAIK it would be propagated into the package _name_, where 
> it is allowed. And even if it were part of the package version, it would 
> be in the "upstream" part of the version number, where it is also allowed.
> 
> The only reserved use of "+" I'm aware of is in the Debian part of the 
> package version.
> 
> But I have not used make-kpkg in ages, so I'm not 100% sure how that builds 
> the package version. I could be that it duplicates the kernel version in 
> both the package name and the upstream part of the package version, but 
> that should still be OK.

That may all well be true. I wasn't even aware of the distinction
between debian and upstream part of the version :)

I do, however, remember no end to trouble with autoversion and make-kpkg
so eventually gave up on using it entirely.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [patch] kbuild: Improve version string logic

[Frans Pop]

Johannes Berg wrote:
> I think adding the + unconditionally will also break things like
> debian's make-kpkg, for instance, since the + would get propagated into
> the package version which reserves the + for something else.

Are you sure? AFAIK it would be propagated into the package _name_, where 
it is allowed. And even if it were part of the package version, it would 
be in the "upstream" part of the version number, where it is also allowed.

The only reserved use of "+" I'm aware of is in the Debian part of the 
package version.

But I have not used make-kpkg in ages, so I'm not 100% sure how that builds 
the package version. I could be that it duplicates the kernel version in 
both the package name and the upstream part of the package version, but 
that should still be OK.

Cheers,
FJP

Re: [patch] kbuild: Improve version string logic

[Theodore Tso]

On Tue, Oct 06, 2009 at 08:37:07PM +0200, Johannes Berg wrote:
> 
> I really don't see how it improves visibility (unless you're oblivious
> to the difference between long and short strings), but I do know that I
> have a few parsers that this will break.
> 
> I think adding the + unconditionally will also break things like
> debian's make-kpkg, for instance, since the + would get propagated into
> the package version which reserves the + for something else.
> 
> Etc. Can we please make this stuff optional?

That's exactly what I'm afraid of.  I'd really like to keep the long
version AUTOVERSION strings the same, please.

						- Ted

Re: [patch] kbuild: Improve version string logic

[Johannes Berg]

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

On Tue, 2009-10-06 at 20:49 +0200, Ingo Molnar wrote:

> That said, i'm perfectly fine with just having the short version include 
> the '+' sign (i.e. Linus's original patch), that was my original 
> suggestion and that is already a big step forward.

However, that still breaks current make-kpkg iirc. I haven't used it in
quite some time, so I don't particularly care.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [patch] kbuild: Improve version string logic

[Ingo Molnar]

* Johannes Berg <johannes@sipsolutions.net> wrote:

> On Tue, 2009-10-06 at 19:35 +0200, Ingo Molnar wrote:
> 
> > It changes how CONFIG_LOCALVERSION_AUTO works, in the following trivial
> > way:
> > 
> >  - if it is set, things work the way they always have, and you get a
> >    extended kernel release like:
> > 
> > 	2.6.32-rc3+00052-g0eca52a-dirty
> > 
> >    ( with the difference that the extra version string is separated via
> >      '+' not via '-'. This improves visibility when we have additional
> >      changes over a vanilla tag. )
> 
> I really don't see how it improves visibility (unless you're oblivious 
> to the difference between long and short strings), but I do know that 
> I have a few parsers that this will break.

It improves visibility the same way the + at the end of the short 
version improves visibility.

That said, i'm perfectly fine with just having the short version include 
the '+' sign (i.e. Linus's original patch), that was my original 
suggestion and that is already a big step forward.

Thanks,

	Ingo

Re: [patch] kbuild: Improve version string logic

[Johannes Berg]

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

On Tue, 2009-10-06 at 19:35 +0200, Ingo Molnar wrote:

> It changes how CONFIG_LOCALVERSION_AUTO works, in the following trivial
> way:
> 
>  - if it is set, things work the way they always have, and you get a
>    extended kernel release like:
> 
> 	2.6.32-rc3+00052-g0eca52a-dirty
> 
>    ( with the difference that the extra version string is separated via
>      '+' not via '-'. This improves visibility when we have additional
>      changes over a vanilla tag. )

I really don't see how it improves visibility (unless you're oblivious
to the difference between long and short strings), but I do know that I
have a few parsers that this will break.

I think adding the + unconditionally will also break things like
debian's make-kpkg, for instance, since the + would get propagated into
the package version which reserves the + for something else.

Etc. Can we please make this stuff optional?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[patch] kbuild: Improve version string logic

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, 6 Oct 2009, Linus Torvalds wrote:
> > 
> > Unless:
> > 
> > > _That_ i think is a lot harder to confuse with the real .31 than a 
> > > v2.6.31-1234-g16123c4 version string.
> > 
> > .. are you saying that it would be just some automatically generated 
> > thing, just a crippled form of CONFIG_LOCALVERSION_AUTO? Kind of a 
> > CONFIG_LOCALVERSION_AUTO_SHORTFORM?
> 
> So how about this?

this patch is great IMHO. I've modified it to propagate the '+' into the 
long version string as well. I've tested it with auto-version not set 
and it now gives:

 Linux europe 2.6.32-rc3+ #2 SMP Tue Oct 6 19:26:58 CEST 2009 i686 i686 i386 GNU/Linux

the -rc3+ is a clearly visible distinction. This will improve things 
when bugs are reported with LOCALVERSION_AUTO not set - we'll always 
know when a tree is not vanilla.

With autoversion set 'uname -a' gives:

 Linux europe 2.6.32-rc3+00052-g0eca52a-dirty #3 SMP Tue Oct 6 19:29:54 CEST 2009 i686 i686 i386 GNU/Linux

IMO that's intuitive too. We get whatever is described in the 
localversion in addition to the tag. The '+' clearly signals that 'set 
union' operation we've done.

So this patch solves all the problems i had with our versioning. I've 
attached it below with a changelog.

Thanks,

	Ingo

--------------->
Subject: kbuild: Improve version string logic
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 6 Oct 2009 09:31:03 -0700 (PDT)

It changes how CONFIG_LOCALVERSION_AUTO works, in the following trivial
way:

 - if it is set, things work the way they always have, and you get a
   extended kernel release like:

	2.6.32-rc3+00052-g0eca52a-dirty

   ( with the difference that the extra version string is separated via
     '+' not via '-'. This improves visibility when we have additional
     changes over a vanilla tag. )

 - but if it is _not_ set, we'll still try to get a version from the
   underlying SCM (we actually support git, hg and SVN right now, even if
   some comments may say "git only"), and if the underlying SCM says it
   has a local version, we append just "+", so you get a version number
   like:

	2.6.32-rc3+

IOW, you'd never get 2.6.32-rc0, but you'd get either the complex git
version number (or SVN/hg/whatever), or at least "2.6.31+" with the "+"
showing that it is more than plain 2.6.31.

The "+" could be anything else, of course. The diff is pretty obvious, you
can argue about exactly _what_ you'd like to see as a suffix for "and then
some".

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 Makefile |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux/Makefile
===================================================================
--- linux.orig/Makefile
+++ linux/Makefile
@@ -963,16 +963,21 @@ localver = $(subst $(space),, $(string) 
 # .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
-ifdef CONFIG_LOCALVERSION_AUTO
-
 ifeq ($(wildcard .scmversion),)
         _localver-auto = $(shell $(CONFIG_SHELL) \
-                         $(srctree)/scripts/setlocalversion $(srctree))
+                         $(srctree)/scripts/setlocalversion $(srctree) | sed 's/^-/+/')
 else
         _localver-auto = $(shell cat .scmversion 2> /dev/null)
 endif
 
+ifdef CONFIG_LOCALVERSION_AUTO
 	localver-auto  = $(LOCALVERSION)$(_localver-auto)
+else
+	ifeq ($_localver-auto,)
+		localver-auto = $(LOCALVERSION)
+	else
+		localver-auto = $(LOCALVERSION)+
+	endif
 endif
 
 localver-full = $(localver)$(localver-auto)