2010-07-13 09:35:51

by Martin Jansa

[permalink] [raw]
Subject: [PATCH] kbuild: setlocalversion should respect srctree param when looking for auto.conf

* it works properly only when CWD == scrtree
* with this patch:
jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
-JaMa-00001-g18672de
* without:
jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
Error: kernelrelease not valid - run 'make prepare' to update it

Signed-off-by: Martin Jansa <[email protected]>
---
scripts/setlocalversion | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index d6a866e..41864d1 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -135,7 +135,7 @@ if $scm_only; then
exit
fi

-if test -e include/config/auto.conf; then
+if test -e "${srctree}/include/config/auto.conf"; then
source "$_"
else
echo "Error: kernelrelease not valid - run 'make prepare' to update it"
--
1.7.1.1


2010-07-13 12:36:54

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] kbuild: setlocalversion should respect srctree param when looking for auto.conf

On 07/13/2010 11:34 AM, Martin Jansa wrote:
> * it works properly only when CWD == scrtree
> * with this patch:
> jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
> -JaMa-00001-g18672de
> * without:
> jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
> Error: kernelrelease not valid - run 'make prepare' to update it

This doesn't look correct, include/config/auto.conf is generated in the
build directory (*), where this script is called from by the Makefile.
If you call the script manually, you have to cd to the build directory
first.

(*) which is different from the source directory if you build with O=.

Michal
>
> Signed-off-by: Martin Jansa <[email protected]>
> ---
> scripts/setlocalversion | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> index d6a866e..41864d1 100755
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -135,7 +135,7 @@ if $scm_only; then
> exit
> fi
>
> -if test -e include/config/auto.conf; then
> +if test -e "${srctree}/include/config/auto.conf"; then
> source "$_"
> else
> echo "Error: kernelrelease not valid - run 'make prepare' to update it"

2010-07-13 12:57:48

by Martin Jansa

[permalink] [raw]
Subject: Re: [PATCH] kbuild: setlocalversion should respect srctree param when looking for auto.conf

On Tue, Jul 13, 2010 at 02:36:51PM +0200, Michal Marek wrote:
> On 07/13/2010 11:34 AM, Martin Jansa wrote:
> > * it works properly only when CWD == scrtree
> > * with this patch:
> > jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
> > -JaMa-00001-g18672de
> > * without:
> > jama src # /usr/src/linux/scripts/setlocalversion /usr/src/linux
> > Error: kernelrelease not valid - run 'make prepare' to update it
>
> This doesn't look correct, include/config/auto.conf is generated in the
> build directory (*), where this script is called from by the Makefile.
> If you call the script manually, you have to cd to the build directory
> first.
>
> (*) which is different from the source directory if you build with O=.

Ah OK, I didn't notice that auto.conf should be generated in build
directory (in my case build dir == source dir).

I just noticed broken kernel module builds in gentoo, because local
version is also checked without changing CWD
KV_LOCAL="${KV_LOCAL}$(sh ${KV_DIR}/scripts/setlocalversion ${KV_DIR})"
in get_version()
http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info.eclass?view=markup

So if scripts/* behavior is not considered as "API" then it should be
fixed in gentoo eclass. It also appends LOCALVERSION twice now, because
first appends LOCALVERSION from config and then calls setlocalversion
also returning it at the beginning.

Thanks

Regards,

--
Martin 'JaMa' Jansa jabber: [email protected]

2010-07-13 16:09:05

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] kbuild: setlocalversion should respect srctree param when looking for auto.conf

On 07/13/2010 02:57 PM, Martin Jansa wrote:
> I just noticed broken kernel module builds in gentoo, because local
> version is also checked without changing CWD
> KV_LOCAL="${KV_LOCAL}$(sh ${KV_DIR}/scripts/setlocalversion ${KV_DIR})"
> in get_version()
> http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info.eclass?view=markup
>
> So if scripts/* behavior is not considered as "API" then it should be
> fixed in gentoo eclass. It also appends LOCALVERSION twice now, because
> first appends LOCALVERSION from config and then calls setlocalversion
> also returning it at the beginning.

I didn't expect that anyone would really use the setlocalversion script
manually, it's hard to draw the line between internal kbuild helpers and
scripts for users. But I'm wondering, why don't you simply do 'make -s
-C $KV_DIR kernelrelease' instead of reimplementing the whole logic
again? You also seem to miss CONFIG_LOCALVERSION.

Michal