[PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Andreas Oberritter]

* depmod already gets executed by pkg_postinst_kernel-image.

* 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.

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
-- 
1.7.5.4

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Darren Hart]

On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
> * depmod already gets executed by pkg_postinst_kernel-image.
> 
> * 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.
> 

Thanks for adding that in. I'm fine not addressing the reliance on the
existence of $D for now (no worse than it was). Some whitespace issues
persist in this version though.

> 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

This appears to be purely a whitespace change - and for the worse.
Please drop this from the patch.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Andreas Oberritter]

On 25.04.2012 02:29, Darren Hart wrote:
> 
> 
> On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
>> * depmod already gets executed by pkg_postinst_kernel-image.
>>
>> * 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.
>>
> 
> Thanks for adding that in. I'm fine not addressing the reliance on the
> existence of $D for now (no worse than it was).

Can you explain what could be improved?

> Some whitespace issues
> persist in this version though.

No. See below.

>> 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
> 
> This appears to be purely a whitespace change - and for the worse.
> Please drop this from the patch.

This just makes it equal to pkg_postrm from kernel.bbclass.

Code in pkg_postrm etc. gets copied to the postinst scripts verbatim.
Therefore any indentation results in strangely indented scripts inside
the package.

Regards,
Andreas

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Darren Hart]

On 04/24/2012 05:42 PM, Andreas Oberritter wrote:
> On 25.04.2012 02:29, Darren Hart wrote:
>>
>>
>> On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
>>> * depmod already gets executed by pkg_postinst_kernel-image.
>>>
>>> * 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.
>>>
>>
>> Thanks for adding that in. I'm fine not addressing the reliance on the
>> existence of $D for now (no worse than it was).
> 
> Can you explain what could be improved?'

I did in the previous thread:

http://lists.linuxtogo.org/pipermail/openembedded-core/2012-April/021419.html

But, I do not think this should hold up your patch.

> 
>> Some whitespace issues
>> persist in this version though.
> 
> No. See below.
> 
>>> 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
>>
>> This appears to be purely a whitespace change - and for the worse.
>> Please drop this from the patch.
> 
> This just makes it equal to pkg_postrm from kernel.bbclass.
> 
> Code in pkg_postrm etc. gets copied to the postinst scripts verbatim.
> Therefore any indentation results in strangely indented scripts inside
> the package.

I see, I suppose that makes sense. Not sure if there is a precedent
here. Personally I'd prefer the code we write and maintain look right
rather than the generated bits. But that's not my call. In any case, if
it needs explanation here, it should also be in the changelog. I
wouldn't hold up this patch for it at this point.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Andreas Oberritter]

On 25.04.2012 03:14, Darren Hart wrote:
> 
> 
> On 04/24/2012 05:42 PM, Andreas Oberritter wrote:
>> On 25.04.2012 02:29, Darren Hart wrote:
>>>
>>>
>>> On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
>>>> * depmod already gets executed by pkg_postinst_kernel-image.
>>>>
>>>> * 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.
>>>>
>>>
>>> Thanks for adding that in. I'm fine not addressing the reliance on the
>>> existence of $D for now (no worse than it was).
>>
>> Can you explain what could be improved?'
> 
> I did in the previous thread:
> 
> http://lists.linuxtogo.org/pipermail/openembedded-core/2012-April/021419.html

Sorry, I must have missed the inline comments when I replied to that mail.

From the manpage of test(1):

       -d FILE
              FILE exists and is a directory

       -z STRING
              the length of STRING is zero

I'm not sure what using -d should accomplish. (Virtually?) all postinst
scripts in OE test for $D's emptiness to decide whether they are called
offline (during do_rootfs) or online (on the target), so if a package
manager sets $D, it should rather stop doing that, instead of relying on
$D not being a directory by chance.

Regards,
Andreas

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Darren Hart]

On 04/24/2012 06:46 PM, Andreas Oberritter wrote:
> On 25.04.2012 03:14, Darren Hart wrote:
>>
>>
>> On 04/24/2012 05:42 PM, Andreas Oberritter wrote:
>>> On 25.04.2012 02:29, Darren Hart wrote:
>>>>
>>>>
>>>> On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
>>>>> * depmod already gets executed by pkg_postinst_kernel-image.
>>>>>
>>>>> * 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.
>>>>>
>>>>
>>>> Thanks for adding that in. I'm fine not addressing the reliance on the
>>>> existence of $D for now (no worse than it was).
>>>
>>> Can you explain what could be improved?'
>>
>> I did in the previous thread:
>>
>> http://lists.linuxtogo.org/pipermail/openembedded-core/2012-April/021419.html
> 
> Sorry, I must have missed the inline comments when I replied to that mail.
> 
> From the manpage of test(1):
> 
>        -d FILE
>               FILE exists and is a directory
> 
>        -z STRING
>               the length of STRING is zero
> 
> I'm not sure what using -d should accomplish. (Virtually?) all postinst
> scripts in OE test for $D's emptiness to decide whether they are called
> offline (during do_rootfs) or online (on the target), so if a package
> manager sets $D, it should rather stop doing that, instead of relying on
> $D not being a directory by chance.

If it's standard throughout OE then it's really not worth discussing for
this patch. It seems like a fragile method to me, but in the event of a
problem we'd have a much larger issue on our hands.

So, looks good to me. Thanks for patiently addressing the feedback.

Acked-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCHv2] {kernel, module}.bbclass: don't run depmod for module packages during do_rootfs

[Saul Wold]

On 04/23/2012 02:48 PM, Andreas Oberritter wrote:
> * depmod already gets executed by pkg_postinst_kernel-image.
>
> * 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.
>
> 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

Merged into OE-Core

Thanks
	Sau!