Patchwork [1/5] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

login
register
mail settings
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

Andreas Oberritter - April 17, 2012, 1:05 p.m.
* 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(-)
Darren Hart - April 23, 2012, 3:31 p.m.
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
Andreas Oberritter - April 23, 2012, 3:55 p.m.
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
>
Darren Hart - April 23, 2012, 5:35 p.m.
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?
Andreas Oberritter - April 23, 2012, 9:21 p.m.
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?
>
Darren Hart - April 23, 2012, 9:26 p.m.
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