Patchwork kernel.bbclass: use the same versioning schema for modules.tgz and provide link to latest

login
register
mail settings
Submitter Martin Jansa
Date Nov. 26, 2012, 12:37 p.m.
Message ID <1353933452-13251-1-git-send-email-Martin.Jansa@gmail.com>
Download mbox | patch
Permalink /patch/39609/
State Accepted, archived
Commit 79f31adabf1b90d27832b65a56734464370dba9c
Headers show

Comments

Martin Jansa - Nov. 26, 2012, 12:37 p.m.
* so it will work look KERNEL_IMAGE
* also we were recreating modules.tgz with every kernel build, but
  overwritting the same output file

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/classes/kernel.bbclass | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Bruce Ashfield - Nov. 26, 2012, 1:49 p.m.
On Mon, Nov 26, 2012 at 7:37 AM, Martin Jansa <martin.jansa@gmail.com>wrote:

> * so it will work look KERNEL_IMAGE
> * also we were recreating modules.tgz with every kernel build, but
>   overwritting the same output file
>
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/classes/kernel.bbclass | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index ee59aaf..9c70e70 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -497,6 +497,10 @@ KERNEL_IMAGE_BASE_NAME ?=
> "${KERNEL_IMAGETYPE}-${PE}-${PV}-${PR}-${MACHINE}-${DA
>  # Don't include the DATETIME variable in the sstate package signatures
>  KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"
>  KERNEL_IMAGE_SYMLINK_NAME ?= "${KERNEL_IMAGETYPE}-${MACHINE}"
> +MODULE_TARBALL_BASE_NAME ?=
> "modules-${PE}-${PV}-${PR}-${MACHINE}-${DATETIME}.tgz"
> +# Don't include the DATETIME variable in the sstate package signatures
> +MODULE_TARBALL_BASE_NAME[vardepsexclude] = "DATETIME"
> +MODULE_TARBALL_SYMLINK_NAME ?= "modules-${MACHINE}.tgz"
>
>  do_uboot_mkimage() {
>         if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then
> @@ -526,7 +530,8 @@ addtask uboot_mkimage before do_install after
> do_compile
>  kernel_do_deploy() {
>         install -m 0644 ${KERNEL_OUTPUT}
> ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
>         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> -               tar -cvzf
> ${DEPLOYDIR}/modules-${KERNEL_VERSION}-${PR}-${MACHINE}.tgz -C ${D} lib
> +               tar -cvzf ${DEPLOYDIR}/${MODULE_TARBALL_BASE_NAME} -C ${D}
> lib
> +               ln -sf ${MODULE_TARBALL_BASE_NAME}.bin
> ${MODULE_TARBALL_SYMLINK_NAME}
>

Just a quick question, are there any concerns about existing users of the
old tgz name ? Obviously, if they were in oe-core, we'd know about them
and update them, but I have no idea myself how much (if anything) was based
on the old name. I'm not sure of the best practices oe-core/oe uses for
cases
like this in the past, so I thought I'd ask.

The old shorter name was nice, but I agree that having it different than
the image
name makes it non obvious how to associate the image and modules, and that's
a nice thing to fix.

Since we are already making a symlink, I guess we could consider
a transnational
symlink with the old name ? (but then again, if the link is there, people
will continue
to have dependencies on it and not update their own code).

Cheers,

Bruce


>         fi
>
>         cd ${DEPLOYDIR}
> --
> 1.8.0
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Martin Jansa - Nov. 26, 2012, 1:57 p.m.
On Mon, Nov 26, 2012 at 08:49:32AM -0500, Bruce Ashfield wrote:
> On Mon, Nov 26, 2012 at 7:37 AM, Martin Jansa <martin.jansa@gmail.com>wrote:
> 
> > * so it will work look KERNEL_IMAGE
> > * also we were recreating modules.tgz with every kernel build, but
> >   overwritting the same output file
> >
> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> >  meta/classes/kernel.bbclass | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> > index ee59aaf..9c70e70 100644
> > --- a/meta/classes/kernel.bbclass
> > +++ b/meta/classes/kernel.bbclass
> > @@ -497,6 +497,10 @@ KERNEL_IMAGE_BASE_NAME ?=
> > "${KERNEL_IMAGETYPE}-${PE}-${PV}-${PR}-${MACHINE}-${DA
> >  # Don't include the DATETIME variable in the sstate package signatures
> >  KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"
> >  KERNEL_IMAGE_SYMLINK_NAME ?= "${KERNEL_IMAGETYPE}-${MACHINE}"
> > +MODULE_TARBALL_BASE_NAME ?=
> > "modules-${PE}-${PV}-${PR}-${MACHINE}-${DATETIME}.tgz"
> > +# Don't include the DATETIME variable in the sstate package signatures
> > +MODULE_TARBALL_BASE_NAME[vardepsexclude] = "DATETIME"
> > +MODULE_TARBALL_SYMLINK_NAME ?= "modules-${MACHINE}.tgz"
> >
> >  do_uboot_mkimage() {
> >         if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then
> > @@ -526,7 +530,8 @@ addtask uboot_mkimage before do_install after
> > do_compile
> >  kernel_do_deploy() {
> >         install -m 0644 ${KERNEL_OUTPUT}
> > ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
> >         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> > -               tar -cvzf
> > ${DEPLOYDIR}/modules-${KERNEL_VERSION}-${PR}-${MACHINE}.tgz -C ${D} lib
> > +               tar -cvzf ${DEPLOYDIR}/${MODULE_TARBALL_BASE_NAME} -C ${D}
> > lib
> > +               ln -sf ${MODULE_TARBALL_BASE_NAME}.bin
> > ${MODULE_TARBALL_SYMLINK_NAME}
> >
> 
> Just a quick question, are there any concerns about existing users of the
> old tgz name ? Obviously, if they were in oe-core, we'd know about them
> and update them, but I have no idea myself how much (if anything) was based
> on the old name. I'm not sure of the best practices oe-core/oe uses for
> cases
> like this in the past, so I thought I'd ask.

I remember seeing some thread about making whole modules tarball
creation optional and disabled by default, so I guess there is only a
few users of this.

> The old shorter name was nice, but I agree that having it different than
> the image
> name makes it non obvious how to associate the image and modules, and that's
> a nice thing to fix.

True, but since oldname was the same for different kernel images I
always found misleading to see 5 different uImages and only one modules
tarball (which we know was created during build of latest uImage - but
maybe not newest uImage version wise - so quite confusing for someone
just trying to download matching uImage and modules).

> Since we are already making a symlink, I guess we could consider
> a transnational
> symlink with the old name ? (but then again, if the link is there, people
> will continue
> to have dependencies on it and not update their own code).

I don't mind having 2 symlinks if there is the need for old name.

Cheers,
Phil Blundell - Nov. 26, 2012, 3:01 p.m.
On Mon, 2012-11-26 at 14:57 +0100, Martin Jansa wrote:
> I remember seeing some thread about making whole modules tarball
> creation optional and disabled by default, so I guess there is only a
> few users of this.

That particular patch was apparently rejected, but I think you're right
that only one or two people said at the time that they were using the
tarball.  It does seem to be a slightly fringe interest.

p.
Martin Jansa - Nov. 26, 2012, 6:18 p.m.
On Mon, Nov 26, 2012 at 03:01:15PM +0000, Phil Blundell wrote:
> On Mon, 2012-11-26 at 14:57 +0100, Martin Jansa wrote:
> > I remember seeing some thread about making whole modules tarball
> > creation optional and disabled by default, so I guess there is only a
> > few users of this.
> 
> That particular patch was apparently rejected, but I think you're right
> that only one or two people said at the time that they were using the
> tarball.  It does seem to be a slightly fringe interest.

http://patches.openembedded.org/patch/37087/

Was it really rejected? I take RP's comment as rejecting setting
KERNEL_DEPLOY_MODULE_TARBALL to 0 as default, but not as rejecting whole
patch..

Cheers,
Bruce Ashfield - Nov. 26, 2012, 6:49 p.m.
On Mon, Nov 26, 2012 at 8:57 AM, Martin Jansa <martin.jansa@gmail.com>wrote:

> On Mon, Nov 26, 2012 at 08:49:32AM -0500, Bruce Ashfield wrote:
> > On Mon, Nov 26, 2012 at 7:37 AM, Martin Jansa <martin.jansa@gmail.com
> >wrote:
> >
> > > * so it will work look KERNEL_IMAGE
> > > * also we were recreating modules.tgz with every kernel build, but
> > >   overwritting the same output file
> > >
> > > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > > ---
> > >  meta/classes/kernel.bbclass | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> > > index ee59aaf..9c70e70 100644
> > > --- a/meta/classes/kernel.bbclass
> > > +++ b/meta/classes/kernel.bbclass
> > > @@ -497,6 +497,10 @@ KERNEL_IMAGE_BASE_NAME ?=
> > > "${KERNEL_IMAGETYPE}-${PE}-${PV}-${PR}-${MACHINE}-${DA
> > >  # Don't include the DATETIME variable in the sstate package signatures
> > >  KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"
> > >  KERNEL_IMAGE_SYMLINK_NAME ?= "${KERNEL_IMAGETYPE}-${MACHINE}"
> > > +MODULE_TARBALL_BASE_NAME ?=
> > > "modules-${PE}-${PV}-${PR}-${MACHINE}-${DATETIME}.tgz"
> > > +# Don't include the DATETIME variable in the sstate package signatures
> > > +MODULE_TARBALL_BASE_NAME[vardepsexclude] = "DATETIME"
> > > +MODULE_TARBALL_SYMLINK_NAME ?= "modules-${MACHINE}.tgz"
> > >
> > >  do_uboot_mkimage() {
> > >         if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then
> > > @@ -526,7 +530,8 @@ addtask uboot_mkimage before do_install after
> > > do_compile
> > >  kernel_do_deploy() {
> > >         install -m 0644 ${KERNEL_OUTPUT}
> > > ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
> > >         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> > > -               tar -cvzf
> > > ${DEPLOYDIR}/modules-${KERNEL_VERSION}-${PR}-${MACHINE}.tgz -C ${D} lib
> > > +               tar -cvzf ${DEPLOYDIR}/${MODULE_TARBALL_BASE_NAME} -C
> ${D}
> > > lib
> > > +               ln -sf ${MODULE_TARBALL_BASE_NAME}.bin
> > > ${MODULE_TARBALL_SYMLINK_NAME}
> > >
> >
> > Just a quick question, are there any concerns about existing users of the
> > old tgz name ? Obviously, if they were in oe-core, we'd know about them
> > and update them, but I have no idea myself how much (if anything) was
> based
> > on the old name. I'm not sure of the best practices oe-core/oe uses for
> > cases
> > like this in the past, so I thought I'd ask.
>
> I remember seeing some thread about making whole modules tarball
> creation optional and disabled by default, so I guess there is only a
> few users of this.
>

Agreed. It isn't common, the modules package yes .. the tarball, not that
I've
heard much about.


>
> > The old shorter name was nice, but I agree that having it different than
> > the image
> > name makes it non obvious how to associate the image and modules, and
> that's
> > a nice thing to fix.
>
> True, but since oldname was the same for different kernel images I
> always found misleading to see 5 different uImages and only one modules
> tarball (which we know was created during build of latest uImage - but
> maybe not newest uImage version wise - so quite confusing for someone
> just trying to download matching uImage and modules).
>

I definitely agree.


>
> > Since we are already making a symlink, I guess we could consider
> > a transnational
> > symlink with the old name ? (but then again, if the link is there, people
> > will continue
> > to have dependencies on it and not update their own code).
>
> I don't mind having 2 symlinks if there is the need for old name.
>

I'd wait to see if anyone pops up with a strong opinion. I'm all for making
the
file names consistent, but having a temporary symlink to catch any outliers
might be a good thing too.

So I'm a solid "maybe" :) I'm hoping RP has a stronger opinion on the
migration
path from the existing name.

Cheers,

Bruce


>
> Cheers,
>
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>

Patch

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index ee59aaf..9c70e70 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -497,6 +497,10 @@  KERNEL_IMAGE_BASE_NAME ?= "${KERNEL_IMAGETYPE}-${PE}-${PV}-${PR}-${MACHINE}-${DA
 # Don't include the DATETIME variable in the sstate package signatures
 KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"
 KERNEL_IMAGE_SYMLINK_NAME ?= "${KERNEL_IMAGETYPE}-${MACHINE}"
+MODULE_TARBALL_BASE_NAME ?= "modules-${PE}-${PV}-${PR}-${MACHINE}-${DATETIME}.tgz"
+# Don't include the DATETIME variable in the sstate package signatures
+MODULE_TARBALL_BASE_NAME[vardepsexclude] = "DATETIME"
+MODULE_TARBALL_SYMLINK_NAME ?= "modules-${MACHINE}.tgz"
 
 do_uboot_mkimage() {
 	if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then 
@@ -526,7 +530,8 @@  addtask uboot_mkimage before do_install after do_compile
 kernel_do_deploy() {
 	install -m 0644 ${KERNEL_OUTPUT} ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
 	if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
-		tar -cvzf ${DEPLOYDIR}/modules-${KERNEL_VERSION}-${PR}-${MACHINE}.tgz -C ${D} lib
+		tar -cvzf ${DEPLOYDIR}/${MODULE_TARBALL_BASE_NAME} -C ${D} lib
+		ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME}
 	fi
 
 	cd ${DEPLOYDIR}