Skip to contentSkip to conversation

Makefile: fix missing dash when using make LOCALVERSION=xxx

PATCH In review Amerigo Wang 4 participants 17 messages Jun 28, 2010 – Aug 18, 2010 Kbuild

lore
Share & tools
lore

Amerigo Wang

author
#0 lore raw

PATCH 1/2 Makefile: fix missing dash when using make LOCALVERSION=xxx

From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:42:28 +0800

When I use make LOCALVERSION=local and LOCALVERSION_AUTO is not set, I got "2.6.35-rc3local". It is supposed to be "2.6.35-rc3-local".

Cc Michal Marek <[email protected]>
Cc David Rientjes <[email protected]>
Signed-off-by Amerigo Wang <[email protected]>
---
 Makefile |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Makefile +5 -1
index 662e820..710f91c 100644
@@ -950,7 +950,11 @@ else
 	endif
 endif
 
-localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
+ifneq ($(LOCALVERSION),)
+	localver-full = $(localver)-$(LOCALVERSION)$(localver-extra)
+else
+	localver-full = $(localver)$(LOCALVERSION)$(localver-extra)
+endif
 
 # Store (new) KERNELRELASE string in include/config/kernel.release
 kernelrelease = $(KERNELVERSION)$(localver-full)
Signature · 1 line
1.6.5.2
author
#1 lore raw

PATCH 2/2 Makefile: "make kernelrelease" should show the correct full kernel version

From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:45:21 +0800

After commit 85a256d8e0116c8f5ad276730830f5d4d473344d, 'make kernelrelease' doesn't show the correct full kernel version. This patch fixes it, 'make kernelrelease' will show the same version name with the one you finally get.

Cc David Rientjes <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Amerigo Wang <[email protected]>
---
 Makefile |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Makefile +2 -3
index 710f91c..2b517a1 100644
@@ -414,7 +414,7 @@ endif
 no-dot-config-targets := clean mrproper distclean \
 			 cscope TAGS tags help %docs check% \
 			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelversion
 
 config-targets := 0
 mixed-targets  := 0

@@ -1468,8 +1468,7 @@ checkstack:
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
 kernelrelease:
-	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
-	$(error kernelrelease not valid - run 'make prepare' to update it))
+	@echo $(kernelrelease)
 kernelversion:
 	@echo $(KERNELVERSION)
Signature · 1 line
1.6.5.2
#2 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 28 lines · Amerigo Wang

From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:45:21 +0800

After commit 85a256d8e0116c8f5ad276730830f5d4d473344d, 'make kernelrelease' doesn't show the correct full kernel version. This patch fixes it, 'make kernelrelease' will show the same version name with the one you finally get.

Cc David Rientjes <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Amerigo Wang <[email protected]>
---
 Makefile |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Makefile +1 -1
index 710f91c..2b517a1 100644
@@ -414,7 +414,7 @@ endif
 no-dot-config-targets := clean mrproper distclean \
 			 cscope TAGS tags help %docs check% \
 			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelversion
 
 config-targets := 0
 mixed-targets  := 0

Makefile checkstack: · line 1473

Quoted 8 lines
Makefile +1 -2
@@ -1468,8 +1468,7 @@ checkstack:
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
 kernelrelease:
-	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
-	$(error kernelrelease not valid - run 'make prepare' to update it))
+	@echo $(kernelrelease)
 kernelversion:
 	@echo $(KERNELVERSION)

AFAICS it was broken even before 85a256d. Nevertheless, the patch is correct, but conflicts with 0915512 in the kbuild tree. What about this one?

From 01ab17887f4cdcb8bb5a5d1bc3b160d186e6e99b Mon Sep 17 00:00:00 2001
From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:45:21 +0800
Subject: [PATCH] Makefile: "make kernelrelease" should show the correct full kernel version

After commit 85a256d8e0116c8f5ad276730830f5d4d473344d, 'make kernelrelease' doesn't show the correct full kernel version. This patch fixes it, 'make kernelrelease' will show the same version name with the one you finally get.

Cc David Rientjes <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Amerigo Wang <[email protected]>

[mmarek: merged with 0915512 and added dependency on include/config/kernel.release]

Signed-off-by Michal Marek <[email protected]>
Makefile +4 -4
index a86ac8c..dceb4f1 100644
@@ -414,7 +414,7 @@ endif
 no-dot-config-targets := clean mrproper distclean \
 			 cscope TAGS tags help %docs check% \
 			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelversion
 
 config-targets := 0
 mixed-targets  := 0

@@ -1395,9 +1395,9 @@ checkstack:
 	$(OBJDUMP) -d vmlinux $$(find . -name '*.ko') | \
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
-kernelrelease:
-	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
-	$(error kernelrelease not valid - run 'make prepare' to update it))
+kernelrelease: include/config/kernel.release
+	@echo $(KERNELRELEASE)
+
 kernelversion:
 	@echo $(KERNELVERSION)
author
#4 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

In reply to Michal Marek
Quoted 31 lines · Michal Marek
Quoted 30 lines · Amerigo Wang

From: Amerigo Wang<[email protected]>
Date: Mon, 28 Jun 2010 10:45:21 +0800

After commit 85a256d8e0116c8f5ad276730830f5d4d473344d, 'make kernelrelease' doesn't show the correct full kernel version. This patch fixes it, 'make kernelrelease' will show the same version name with the one you finally get.

Cc David Rientjes<[email protected]>
Cc Michal Marek<[email protected]>
Signed-off-by Amerigo Wang<[email protected]>
---
  Makefile |    5 ++---
  1 files changed, 2 insertions(+), 3 deletions(-)
Makefile +1 -1
index 710f91c..2b517a1 100644
@@ -414,7 +414,7 @@ endif
  no-dot-config-targets := clean mrproper distclean \
  			 cscope TAGS tags help %docs check% \
  			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelversion
 
  config-targets := 0
  mixed-targets  := 0

@@ -1468,8 +1468,7 @@ checkstack:
  	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
Quoted 11 lines

Makefile checkstack: · line 1402

Quoted 6 lines
Makefile +1 -2
  kernelrelease:
-	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
-	$(error kernelrelease not valid - run 'make prepare' to update it))
+	@echo $(kernelrelease)
  kernelversion:
  	@echo $(KERNELVERSION)

AFAICS it was broken even before 85a256d. Nevertheless, the patch is correct, but conflicts with 0915512 in the kbuild tree. What about this one?

Hmm, I am not sure. IIRC, my first try was also using KERNELRELEASE but it didn't work. Note, I may remember wrongly. :)

Have you tested it? Sorry that currently I don't have time to test it, I am quite busy with other stuffs...

Thanks!

#5 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

In reply to Amerigo Wang

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

$ make kernelrelease -d
[...]
Considering target file `kernelrelease'.
 File `kernelrelease' does not exist.
  Considering target file `include/config/kernel.release'.
    Pruning file `include/config/auto.conf'.
    Considering target file `FORCE'.
     File `FORCE' does not exist.
     Finished prerequisites of target file `FORCE'.
    Must remake target `FORCE'.
    Successfully remade target file `FORCE'.
   Finished prerequisites of target file `include/config/kernel.release'.
   Prerequisite `include/config/auto.conf' is older than target `include/config/kernel.release'.
   Prerequisite `FORCE' of target `include/config/kernel.release' does not exist.
  Must remake target `include/config/kernel.release'.

Thanks,
Brice

Quoted 38 lines · Amerigo Wang

From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:45:21 +0800

After commit 85a256d8e0116c8f5ad276730830f5d4d473344d, 'make kernelrelease' doesn't show the correct full kernel version. This patch fixes it, 'make kernelrelease' will show the same version name with the one you finally get.

Cc David Rientjes <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Amerigo Wang <[email protected]>
---
 Makefile |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Makefile +2 -3
index 710f91c..2b517a1 100644
@@ -414,7 +414,7 @@ endif
 no-dot-config-targets := clean mrproper distclean \
 			 cscope TAGS tags help %docs check% \
 			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelversion
 
 config-targets := 0
 mixed-targets  := 0

@@ -1468,8 +1468,7 @@ checkstack:
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
 kernelrelease:
-	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
-	$(error kernelrelease not valid - run 'make prepare' to update it))
+	@echo $(kernelrelease)
 kernelversion:
 	@echo $(KERNELVERSION)
author
#6 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 6 lines · Brice Goglin

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

I think so, because "LOCALVERSION=" can be given from command line, so we need to regenerate it.

Or am I missing your point here?

Signature · 2 lines
The opposite of love is not hate, it's indifference.
 - Elie Wiesel
#7 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 8 lines · Cong Wang
Quoted 6 lines · Brice Goglin

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

Quoted 5 lines

I think so, because "LOCALVERSION=" can be given from command line, so we need to regenerate it.

Or am I missing your point here?

Interesting. I assumed "make kernelrelease" was mainly here to display the release string (which means you would not need write access to the kernel build dir). And indeed make help says:
kernelrelease - Output the release version string
Right now, it looks like "update the version string and by the way display it too" (and you need write access).

Brice

author
#8 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 15 lines · Brice Goglin
Quoted 8 lines · Cong Wang
Quoted 6 lines · Brice Goglin

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

Quoted 5 lines

I think so, because "LOCALVERSION=" can be given from command line, so we need to regenerate it.

Or am I missing your point here?

Quoted 7 lines

Interesting. I assumed "make kernelrelease" was mainly here to display the release string (which means you would not need write access to the kernel build dir). And indeed make help says:
kernelrelease - Output the release version string
Right now, it looks like "update the version string and by the way display it too" (and you need write access).

I believe you will also need write access even without this patch, if you compile a fresh kernel. So your assumption is not correct.

Signature · 2 lines
The opposite of love is not hate, it's indifference.
 - Elie Wiesel
#9 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 25 lines · Cong Wang
Quoted 16 lines · Brice Goglin
Quoted 9 lines · Cong Wang
Quoted 7 lines · Brice Goglin

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a
problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

Quoted 5 lines

I think so, because "LOCALVERSION=" can be given from command line, so we need to regenerate it.

Or am I missing your point here?

Quoted 7 lines

Interesting. I assumed "make kernelrelease" was mainly here to display the release string (which means you would not need write access to the kernel build dir). And indeed make help says:
kernelrelease - Output the release version string
Right now, it looks like "update the version string and by the way display it too" (and you need write access).

Quoted 3 lines

I believe you will also need write access even without this patch, if you compile a fresh kernel. So your assumption is not correct.

If I revert 01ab17887f4, I don't need write access. Things always worked fine before 2.6.36 as far I remember.

Brice

author
#10 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 26 lines · Brice Goglin
Quoted 25 lines · Cong Wang
Quoted 23 lines · Brice Goglin
Quoted 9 lines · Cong Wang
Quoted 7 lines · Brice Goglin

This patch (actually 01ab17887 in 2.6.36-rc1) reveals what looks like a
problem to me: make kernelrelease always regenerates include/config/kernel.release even if it's already more recent than include/config/auto.conf. Is this the expected behavior? Do we really need include/config/kernel.release to depend on FORCE?

Quoted 5 lines

I think so, because "LOCALVERSION=" can be given from command line, so we need to regenerate it.

Or am I missing your point here?

Interesting. I assumed "make kernelrelease" was mainly here to display the release string (which means you would not need write access to the kernel build dir). And indeed make help says:
kernelrelease - Output the release version string
Right now, it looks like "update the version string and by the way display it too" (and you need write access).

Quoted 7 lines
Quoted 3 lines

I believe you will also need write access even without this patch, if you compile a fresh kernel. So your assumption is not correct.

If I revert 01ab17887f4, I don't need write access. Things always worked fine before 2.6.36 as far I remember.

Ah, I forgot Michal checked in a slightly different patch with mine. :)

Does the attached patch work for you?

---

Signed-off-by WANG Cong <[email protected]>
Makefile +2 -2
index f3bdff8..25da252 100644
@@ -1408,8 +1408,8 @@ checkstack:
 	$(OBJDUMP) -d vmlinux $$(find . -name '*.ko') | \
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
-kernelrelease: include/config/kernel.release
-	@echo $(KERNELRELEASE)
+kernelrelease:
+	@echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
 
 kernelversion:
 	@echo $(KERNELVERSION)
Tested-by
#11 lore raw

Re: [PATCH 2/2] Makefile: "make kernelrelease" should show the correct full kernel version

Quoted 6 lines · Cong Wang
Makefile +0 -0
index f3bdff8..25da252 100644

Makefile checkstack: · line 1415

Quoted 10 lines
Makefile +2 -2
@@ -1408,8 +1408,8 @@ checkstack:
 	$(OBJDUMP) -d vmlinux $$(find . -name '*.ko') | \
 	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
 
-kernelrelease: include/config/kernel.release
-	@echo $(KERNELRELEASE)
+kernelrelease:
+	@echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
 
 kernelversion:
 	@echo $(KERNELVERSION)

It works, thanks!

Tested-by Brice Goglin <[email protected]>

Brice

#12 lore raw
In reply to Amerigo Wang
Quoted 6 lines · Amerigo Wang

From: Amerigo Wang <[email protected]>
Date: Mon, 28 Jun 2010 10:42:28 +0800

When I use make LOCALVERSION=local and LOCALVERSION_AUTO is not set, I got "2.6.35-rc3local". It is supposed to be "2.6.35-rc3-local".

No, a `-' is not implied when using LOCALVERSION=.

author
#13 lore raw
Quoted 10 lines · David Rientjes
Quoted 6 lines · Amerigo Wang

From: Amerigo Wang<[email protected]>
Date: Mon, 28 Jun 2010 10:42:28 +0800

When I use make LOCALVERSION=local and LOCALVERSION_AUTO is not set, I got "2.6.35-rc3local". It is supposed to be "2.6.35-rc3-local".

No, a `-' is not implied when using LOCALVERSION=.

But in your patch description, you said:

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.

So, shouldn't that be "make LOCALVERSION=-kbuild"?

Thanks.

#14 lore raw
Quoted 9 lines · Cong Wang

But in your patch description, you said:

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.

So, shouldn't that be "make LOCALVERSION=-kbuild"?

Yup, that's what it should have said. That doesn't mean that we need to change the long-standing behavior of LOCALVERSION= to add an implied `-', though.

author
#15 lore raw
Quoted 9 lines · David Rientjes
Quoted 7 lines · Cong Wang

But in your patch description, you said:

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.

Quoted 6 lines
Quoted 2 lines

So, shouldn't that be "make LOCALVERSION=-kbuild"?

Yup, that's what it should have said. That doesn't mean that we need to change the long-standing behavior of LOCALVERSION= to add an implied `-', though.

Ok, but your patch description is the only doc I found, so... :)

Thanks.

#16 lore raw
In reply to David Rientjes
Quoted 9 lines · David Rientjes
Quoted 7 lines · Cong Wang

But in your patch description, you said:

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.

Quoted 6 lines
Quoted 2 lines

So, shouldn't that be "make LOCALVERSION=-kbuild"?

Yup, that's what it should have said. That doesn't mean that we need to change the long-standing behavior of LOCALVERSION= to add an implied `-', though.

Agreed, we should not enforce any new policy here. Someone might wish to use underscores as separators for instance.

Michal