| Submitter | Andreas Oberritter |
|---|---|
| Date | April 17, 2012, 1:05 p.m. |
| Message ID | <1334667923-13908-2-git-send-email-obi@opendreambox.org> |
| Download | mbox | patch |
| Permalink | /patch/26049/ |
| State | New |
| Headers | show |
Comments
On 04/17/2012 06:05 AM, Andreas Oberritter wrote: > * depmod already gets executed by pkg_postinst_kernel-image. What if you install a module package after the initial system installation? Don't you want the module install to run depmod? -- Darren > > Signed-off-by: Andreas Oberritter <obi@opendreambox.org> > --- > meta/classes/kernel.bbclass | 4 +--- > meta/classes/module.bbclass | 7 +++---- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass > index 3519e7c..c21ab96 100644 > --- a/meta/classes/kernel.bbclass > +++ b/meta/classes/kernel.bbclass > @@ -276,9 +276,7 @@ fi > } > > pkg_postinst_modules () { > -if [ -n "$D" ]; then > - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} > -else > +if [ -z "$D" ]; then > depmod -a > update-modules || true > fi > diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass > index 53c16b7..91628e4 100644 > --- a/meta/classes/module.bbclass > +++ b/meta/classes/module.bbclass > @@ -37,15 +37,14 @@ module_do_install() { > } > > pkg_postinst_append () { > - if [ -n "$D" ]; then > - exit 1 > - fi > +if [ -z "$D" ]; then > depmod -a > update-modules || true > +fi > } > > pkg_postrm_append () { > - update-modules || true > +update-modules || true > } > > EXPORT_FUNCTIONS do_compile do_install
On 23.04.2012 17:31, Darren Hart wrote: > > On 04/17/2012 06:05 AM, Andreas Oberritter wrote: >> * depmod already gets executed by pkg_postinst_kernel-image. > > What if you install a module package after the initial system > installation? Don't you want the module install to run depmod? If you look at the patch, you see that only the calls to cross-depmod get removed (which get called during do_rootfs). The behaviour of modules installed after the initial installation hasn't changed. What has changed in addition is that modules.bbclass' pkg_postinst doesn't return an error during do_rootfs, but instead behaves like pkg_postinst_modules from kernel.bbclass. Regards, Andreas > > -- > Darren > >> >> Signed-off-by: Andreas Oberritter <obi@opendreambox.org> >> --- >> meta/classes/kernel.bbclass | 4 +--- >> meta/classes/module.bbclass | 7 +++---- >> 2 files changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass >> index 3519e7c..c21ab96 100644 >> --- a/meta/classes/kernel.bbclass >> +++ b/meta/classes/kernel.bbclass >> @@ -276,9 +276,7 @@ fi >> } >> >> pkg_postinst_modules () { >> -if [ -n "$D" ]; then >> - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} >> -else >> +if [ -z "$D" ]; then >> depmod -a >> update-modules || true >> fi >> diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass >> index 53c16b7..91628e4 100644 >> --- a/meta/classes/module.bbclass >> +++ b/meta/classes/module.bbclass >> @@ -37,15 +37,14 @@ module_do_install() { >> } >> >> pkg_postinst_append () { >> - if [ -n "$D" ]; then >> - exit 1 >> - fi >> +if [ -z "$D" ]; then >> depmod -a >> update-modules || true >> +fi >> } >> >> pkg_postrm_append () { >> - update-modules || true >> +update-modules || true >> } >> >> EXPORT_FUNCTIONS do_compile do_install >
On 04/17/2012 06:05 AM, Andreas Oberritter wrote: > * depmod already gets executed by pkg_postinst_kernel-image. > Please include a description of the errors seen that this fixes. This makes it easier for people experiencing this to find the fix. > Signed-off-by: Andreas Oberritter <obi@opendreambox.org> > --- > meta/classes/kernel.bbclass | 4 +--- > meta/classes/module.bbclass | 7 +++---- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass > index 3519e7c..c21ab96 100644 > --- a/meta/classes/kernel.bbclass > +++ b/meta/classes/kernel.bbclass > @@ -276,9 +276,7 @@ fi > } > > pkg_postinst_modules () { > -if [ -n "$D" ]; then > - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} > -else > +if [ -z "$D" ]; then I understand it parallels the previous test - but it seems fragile to rely on the mere existence of $D. Consider the following? if [ ! -d "$D" ]; then This will do the right thing if D is empty as well as if $D is defined by bitbake. > depmod -a > update-modules || true > fi > diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass > index 53c16b7..91628e4 100644 > --- a/meta/classes/module.bbclass > +++ b/meta/classes/module.bbclass > @@ -37,15 +37,14 @@ module_do_install() { > } > > pkg_postinst_append () { > - if [ -n "$D" ]; then > - exit 1 > - fi > +if [ -z "$D" ]; then > depmod -a > update-modules || true > +fi > } > > pkg_postrm_append () { > - update-modules || true > +update-modules || true Unintentional whitespace change?
On 23.04.2012 19:35, Darren Hart wrote: > > > On 04/17/2012 06:05 AM, Andreas Oberritter wrote: >> * depmod already gets executed by pkg_postinst_kernel-image. >> > > Please include a description of the errors seen that this fixes. This > makes it easier for people experiencing this to find the fix. Well, there is no error. If you build a module using module.bbclass, pkg_postinst returns 1 in do_rootfs, causing pkg_postinst to run again on first boot. To improve this situation, I copied pkg_postinst from kernel.bbclass to module.bbclass. This was rejected by Koen, because he doesn't like the code from kernel.bblcass, which uses ${STAGING_DIR_KERNEL}. Richard then suggested that calling depmod during do_rootfs wasn't necessary at all, because it already gets done by kernel-image. I already explained this in the cover letter and in the previous email thread. > >> Signed-off-by: Andreas Oberritter <obi@opendreambox.org> >> --- >> meta/classes/kernel.bbclass | 4 +--- >> meta/classes/module.bbclass | 7 +++---- >> 2 files changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass >> index 3519e7c..c21ab96 100644 >> --- a/meta/classes/kernel.bbclass >> +++ b/meta/classes/kernel.bbclass >> @@ -276,9 +276,7 @@ fi >> } >> >> pkg_postinst_modules () { >> -if [ -n "$D" ]; then >> - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} >> -else >> +if [ -z "$D" ]; then > > I understand it parallels the previous test - but it seems fragile to > rely on the mere existence of $D. Consider the following? > > if [ ! -d "$D" ]; then > > This will do the right thing if D is empty as well as if $D is defined > by bitbake. > >> depmod -a >> update-modules || true >> fi >> diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass >> index 53c16b7..91628e4 100644 >> --- a/meta/classes/module.bbclass >> +++ b/meta/classes/module.bbclass >> @@ -37,15 +37,14 @@ module_do_install() { >> } >> >> pkg_postinst_append () { >> - if [ -n "$D" ]; then >> - exit 1 >> - fi >> +if [ -z "$D" ]; then >> depmod -a >> update-modules || true >> +fi >> } >> >> pkg_postrm_append () { >> - update-modules || true >> +update-modules || true > > Unintentional whitespace change? >
On 04/23/2012 02:21 PM, Andreas Oberritter wrote: > On 23.04.2012 19:35, Darren Hart wrote: >> >> >> On 04/17/2012 06:05 AM, Andreas Oberritter wrote: >>> * depmod already gets executed by pkg_postinst_kernel-image. >>> >> >> Please include a description of the errors seen that this fixes. This >> makes it easier for people experiencing this to find the fix. > > Well, there is no error. If you build a module using module.bbclass, > pkg_postinst returns 1 in do_rootfs, causing pkg_postinst to run again > on first boot. To improve this situation, I copied pkg_postinst from > kernel.bbclass to module.bbclass. This was rejected by Koen, because he > doesn't like the code from kernel.bblcass, which uses > ${STAGING_DIR_KERNEL}. Richard then suggested that calling depmod during > do_rootfs wasn't necessary at all, because it already gets done by > kernel-image. I already explained this in the cover letter and in the > previous email thread. Apologies if I missed some context. However, the detail needs to be in the commit log, not the cover letter, as the commit log is where developers will look for the rationale and justification of a change. -- Darren > >> >>> Signed-off-by: Andreas Oberritter <obi@opendreambox.org> >>> --- >>> meta/classes/kernel.bbclass | 4 +--- >>> meta/classes/module.bbclass | 7 +++---- >>> 2 files changed, 4 insertions(+), 7 deletions(-) >>> >>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass >>> index 3519e7c..c21ab96 100644 >>> --- a/meta/classes/kernel.bbclass >>> +++ b/meta/classes/kernel.bbclass >>> @@ -276,9 +276,7 @@ fi >>> } >>> >>> pkg_postinst_modules () { >>> -if [ -n "$D" ]; then >>> - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} >>> -else >>> +if [ -z "$D" ]; then >> >> I understand it parallels the previous test - but it seems fragile to >> rely on the mere existence of $D. Consider the following? >> >> if [ ! -d "$D" ]; then >> >> This will do the right thing if D is empty as well as if $D is defined >> by bitbake. >> >>> depmod -a >>> update-modules || true >>> fi >>> diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass >>> index 53c16b7..91628e4 100644 >>> --- a/meta/classes/module.bbclass >>> +++ b/meta/classes/module.bbclass >>> @@ -37,15 +37,14 @@ module_do_install() { >>> } >>> >>> pkg_postinst_append () { >>> - if [ -n "$D" ]; then >>> - exit 1 >>> - fi >>> +if [ -z "$D" ]; then >>> depmod -a >>> update-modules || true >>> +fi >>> } >>> >>> pkg_postrm_append () { >>> - update-modules || true >>> +update-modules || true >> >> Unintentional whitespace change? >> >
Patch
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass index 3519e7c..c21ab96 100644 --- a/meta/classes/kernel.bbclass +++ b/meta/classes/kernel.bbclass @@ -276,9 +276,7 @@ fi } pkg_postinst_modules () { -if [ -n "$D" ]; then - ${HOST_PREFIX}depmod -A -b $D -F ${STAGING_KERNEL_DIR}/System.map-${KERNEL_VERSION} ${KERNEL_VERSION} -else +if [ -z "$D" ]; then depmod -a update-modules || true fi diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass index 53c16b7..91628e4 100644 --- a/meta/classes/module.bbclass +++ b/meta/classes/module.bbclass @@ -37,15 +37,14 @@ module_do_install() { } pkg_postinst_append () { - if [ -n "$D" ]; then - exit 1 - fi +if [ -z "$D" ]; then depmod -a update-modules || true +fi } pkg_postrm_append () { - update-modules || true +update-modules || true } EXPORT_FUNCTIONS do_compile do_install
* depmod already gets executed by pkg_postinst_kernel-image. Signed-off-by: Andreas Oberritter <obi@opendreambox.org> --- meta/classes/kernel.bbclass | 4 +--- meta/classes/module.bbclass | 7 +++---- 2 files changed, 4 insertions(+), 7 deletions(-)