Patchwork [PATCHv2] kernel/module-base: Append PR to MACHINE_KERNEL_PR

login
register
mail settings
Submitter Andreas Oberritter
Date March 24, 2011, 4:14 p.m.
Message ID <1300983264-16652-1-git-send-email-obi@opendreambox.org>
Download mbox | patch
Permalink /patch/1787/
State Under Review, archived
Headers show

Comments

Andreas Oberritter - March 24, 2011, 4:14 p.m.
Based on http://comments.gmane.org/gmane.comp.handhelds.openembedded/42905

Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
CC: Denis Dydychkin <nyrl@mail.ru>
CC: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
---
v2: Move code to machine-kernel-pr.bbclass, to avoid duplication and to
    allow other recipes to inherit it. This is useful for tasks which
    depend on different kernel modules for different kernel versions,
    e.g. madwifi-ng for old kernels, but ath5k for new kernels.

 classes/kernel.bbclass            |    7 ++-----
 classes/machine-kernel-pr.bbclass |   15 +++++++++++++++
 classes/module-base.bbclass       |   11 ++---------
 3 files changed, 19 insertions(+), 14 deletions(-)
 create mode 100644 classes/machine-kernel-pr.bbclass
Andreas Oberritter - April 4, 2011, 1 p.m.
Ping. Any votes for or against this patch with either appended or
prepended PR?

On 03/24/2011 05:14 PM, Andreas Oberritter wrote:
> Based on http://comments.gmane.org/gmane.comp.handhelds.openembedded/42905
> 
> Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
> CC: Denis Dydychkin <nyrl@mail.ru>
> CC: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> ---
> v2: Move code to machine-kernel-pr.bbclass, to avoid duplication and to
>     allow other recipes to inherit it. This is useful for tasks which
>     depend on different kernel modules for different kernel versions,
>     e.g. madwifi-ng for old kernels, but ath5k for new kernels.
> 
>  classes/kernel.bbclass            |    7 ++-----
>  classes/machine-kernel-pr.bbclass |   15 +++++++++++++++
>  classes/module-base.bbclass       |   11 ++---------
>  3 files changed, 19 insertions(+), 14 deletions(-)
>  create mode 100644 classes/machine-kernel-pr.bbclass
> 
> diff --git a/classes/kernel.bbclass b/classes/kernel.bbclass
> index 0109ce6..0187fb3 100644
> --- a/classes/kernel.bbclass
> +++ b/classes/kernel.bbclass
> @@ -20,13 +20,10 @@ python __anonymous () {
>      image = bb.data.getVar('INITRAMFS_IMAGE', d, True)
>      if image:
>          bb.data.setVar('INITRAMFS_TASK', '${INITRAMFS_IMAGE}:do_rootfs', d)
> -
> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
> -
> -    if machine_kernel_pr:
> -       bb.data.setVar('PR', machine_kernel_pr, d)
>  }
>  
> +inherit machine-kernel-pr
> +
>  INITRAMFS_IMAGE ?= ""
>  INITRAMFS_TASK ?= ""
>  
> diff --git a/classes/machine-kernel-pr.bbclass b/classes/machine-kernel-pr.bbclass
> new file mode 100644
> index 0000000..a72a0c0
> --- /dev/null
> +++ b/classes/machine-kernel-pr.bbclass
> @@ -0,0 +1,15 @@
> +# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
> +# rebuilds for kernel and external modules
> +python __anonymous () {
> +    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
> +    if machine_kernel_pr:
> +       # Append
> +       # a) .X, if the recipe's PR is rX, or
> +       # b) +${PR}, if the recipe's PR doesn't begin with r
> +       pr = bb.data.getVar('PR', d, True)
> +       if pr.startswith('r'):
> +               suffix = '.' + pr[1:]
> +       else:
> +               suffix = '+' + pr
> +       bb.data.setVar('PR', machine_kernel_pr + suffix, d)
> +}
> diff --git a/classes/module-base.bbclass b/classes/module-base.bbclass
> index 9aaaa4e..1a97955 100644
> --- a/classes/module-base.bbclass
> +++ b/classes/module-base.bbclass
> @@ -2,18 +2,11 @@ inherit module_strip
>  
>  inherit kernel-arch
>  
> +inherit machine-kernel-pr
> +
>  export OS = "${TARGET_OS}"
>  export CROSS_COMPILE = "${TARGET_PREFIX}"
>  
> -# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
> -# rebuilds for kernel and external modules
> -python __anonymous () {
> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
> -
> -    if machine_kernel_pr:
> -       bb.data.setVar('PR', machine_kernel_pr, d)
> -}
> -
>  export KERNEL_VERSION = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion')}"
>  export KERNEL_SOURCE = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-source')}"
>  KERNEL_OBJECT_SUFFIX = "${@[".o", ".ko"][base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion') > "2.6.0"]}"
Koen Kooi - April 4, 2011, 1:58 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04-04-11 15:00, Andreas Oberritter wrote:
> Ping. Any votes for or against this patch with either appended or
> prepended PR?

I don't like this patch *at all*. Recipes can already do
MACHINE_KERNEL_PR_append = "something" if they need to.

Can you should me a specific example on how this would be an improvement?

regards,

Koen

> 
> On 03/24/2011 05:14 PM, Andreas Oberritter wrote:
>> Based on http://comments.gmane.org/gmane.comp.handhelds.openembedded/42905
>>
>> Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
>> CC: Denis Dydychkin <nyrl@mail.ru>
>> CC: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
>> ---
>> v2: Move code to machine-kernel-pr.bbclass, to avoid duplication and to
>>     allow other recipes to inherit it. This is useful for tasks which
>>     depend on different kernel modules for different kernel versions,
>>     e.g. madwifi-ng for old kernels, but ath5k for new kernels.
>>
>>  classes/kernel.bbclass            |    7 ++-----
>>  classes/machine-kernel-pr.bbclass |   15 +++++++++++++++
>>  classes/module-base.bbclass       |   11 ++---------
>>  3 files changed, 19 insertions(+), 14 deletions(-)
>>  create mode 100644 classes/machine-kernel-pr.bbclass
>>
>> diff --git a/classes/kernel.bbclass b/classes/kernel.bbclass
>> index 0109ce6..0187fb3 100644
>> --- a/classes/kernel.bbclass
>> +++ b/classes/kernel.bbclass
>> @@ -20,13 +20,10 @@ python __anonymous () {
>>      image = bb.data.getVar('INITRAMFS_IMAGE', d, True)
>>      if image:
>>          bb.data.setVar('INITRAMFS_TASK', '${INITRAMFS_IMAGE}:do_rootfs', d)
>> -
>> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>> -
>> -    if machine_kernel_pr:
>> -       bb.data.setVar('PR', machine_kernel_pr, d)
>>  }
>>  
>> +inherit machine-kernel-pr
>> +
>>  INITRAMFS_IMAGE ?= ""
>>  INITRAMFS_TASK ?= ""
>>  
>> diff --git a/classes/machine-kernel-pr.bbclass b/classes/machine-kernel-pr.bbclass
>> new file mode 100644
>> index 0000000..a72a0c0
>> --- /dev/null
>> +++ b/classes/machine-kernel-pr.bbclass
>> @@ -0,0 +1,15 @@
>> +# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
>> +# rebuilds for kernel and external modules
>> +python __anonymous () {
>> +    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>> +    if machine_kernel_pr:
>> +       # Append
>> +       # a) .X, if the recipe's PR is rX, or
>> +       # b) +${PR}, if the recipe's PR doesn't begin with r
>> +       pr = bb.data.getVar('PR', d, True)
>> +       if pr.startswith('r'):
>> +               suffix = '.' + pr[1:]
>> +       else:
>> +               suffix = '+' + pr
>> +       bb.data.setVar('PR', machine_kernel_pr + suffix, d)
>> +}
>> diff --git a/classes/module-base.bbclass b/classes/module-base.bbclass
>> index 9aaaa4e..1a97955 100644
>> --- a/classes/module-base.bbclass
>> +++ b/classes/module-base.bbclass
>> @@ -2,18 +2,11 @@ inherit module_strip
>>  
>>  inherit kernel-arch
>>  
>> +inherit machine-kernel-pr
>> +
>>  export OS = "${TARGET_OS}"
>>  export CROSS_COMPILE = "${TARGET_PREFIX}"
>>  
>> -# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
>> -# rebuilds for kernel and external modules
>> -python __anonymous () {
>> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>> -
>> -    if machine_kernel_pr:
>> -       bb.data.setVar('PR', machine_kernel_pr, d)
>> -}
>> -
>>  export KERNEL_VERSION = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion')}"
>>  export KERNEL_SOURCE = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-source')}"
>>  KERNEL_OBJECT_SUFFIX = "${@[".o", ".ko"][base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion') > "2.6.0"]}"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFNmc6QMkyGM64RGpERApSbAJkBcJm8dGGqOLOftp/ulcfEyQjR+ACgjPAu
LUeJwMv5bmRdL9kDne0GvIc=
=U/65
-----END PGP SIGNATURE-----
Andreas Oberritter - April 4, 2011, 3:14 p.m.
On 04/04/2011 03:58 PM, Koen Kooi wrote:
> On 04-04-11 15:00, Andreas Oberritter wrote:
>> Ping. Any votes for or against this patch with either appended or
>> prepended PR?
> 
> I don't like this patch *at all*. Recipes can already do
> MACHINE_KERNEL_PR_append = "something" if they need to.

The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
append something to it in a recipe.

> Can you should me a specific example on how this would be an improvement?

With the version below. a distro can start to use MACHINE_KERNEL_PR any
time without breaking updates.

The vast majority of modules do not use MACHINE_KERNEL_PR, as the
following numbers can tell (from 2011.03-maintenance):

$ git grep 'inherit.*module' | grep -v ^linux/ | wc -l
111
$ git grep 'MACHINE_KERNEL_PR' | grep -v ^linux/ | wc -l
33
$ git grep 'MACHINE_KERNEL_PR_' | grep -v ^linux/ | wc -l
10

Of the 33 recipes, 32 are directly related to TI, AFAICT. So, besides
TI, nobody is using MACHINE_KERNEL_PR in recipes at all, with
open-iscsi-kernel being the only exception.

Only 10 of the 33 recipes use MACHINE_KERNEL_PR_append. The other 23
users use the variable to set PR inside the recipe, which indicates that
they don't inherit module-base.bbclass.

In other words, 101 of 111 or about 91% of all recipes inheriting
module-base.bbclass currently won't rebuild automatically when their PR
gets incremented and the distro uses MACHINE_KERNEL_PR.

Regards,
Andreas

>> On 03/24/2011 05:14 PM, Andreas Oberritter wrote:
>>> Based on http://comments.gmane.org/gmane.comp.handhelds.openembedded/42905
>>>
>>> Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
>>> CC: Denis Dydychkin <nyrl@mail.ru>
>>> CC: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
>>> ---
>>> v2: Move code to machine-kernel-pr.bbclass, to avoid duplication and to
>>>     allow other recipes to inherit it. This is useful for tasks which
>>>     depend on different kernel modules for different kernel versions,
>>>     e.g. madwifi-ng for old kernels, but ath5k for new kernels.
>>>
>>>  classes/kernel.bbclass            |    7 ++-----
>>>  classes/machine-kernel-pr.bbclass |   15 +++++++++++++++
>>>  classes/module-base.bbclass       |   11 ++---------
>>>  3 files changed, 19 insertions(+), 14 deletions(-)
>>>  create mode 100644 classes/machine-kernel-pr.bbclass
>>>
>>> diff --git a/classes/kernel.bbclass b/classes/kernel.bbclass
>>> index 0109ce6..0187fb3 100644
>>> --- a/classes/kernel.bbclass
>>> +++ b/classes/kernel.bbclass
>>> @@ -20,13 +20,10 @@ python __anonymous () {
>>>      image = bb.data.getVar('INITRAMFS_IMAGE', d, True)
>>>      if image:
>>>          bb.data.setVar('INITRAMFS_TASK', '${INITRAMFS_IMAGE}:do_rootfs', d)
>>> -
>>> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>>> -
>>> -    if machine_kernel_pr:
>>> -       bb.data.setVar('PR', machine_kernel_pr, d)
>>>  }
>>>  
>>> +inherit machine-kernel-pr
>>> +
>>>  INITRAMFS_IMAGE ?= ""
>>>  INITRAMFS_TASK ?= ""
>>>  
>>> diff --git a/classes/machine-kernel-pr.bbclass b/classes/machine-kernel-pr.bbclass
>>> new file mode 100644
>>> index 0000000..a72a0c0
>>> --- /dev/null
>>> +++ b/classes/machine-kernel-pr.bbclass
>>> @@ -0,0 +1,15 @@
>>> +# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
>>> +# rebuilds for kernel and external modules
>>> +python __anonymous () {
>>> +    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>>> +    if machine_kernel_pr:
>>> +       # Append
>>> +       # a) .X, if the recipe's PR is rX, or
>>> +       # b) +${PR}, if the recipe's PR doesn't begin with r
>>> +       pr = bb.data.getVar('PR', d, True)
>>> +       if pr.startswith('r'):
>>> +               suffix = '.' + pr[1:]
>>> +       else:
>>> +               suffix = '+' + pr
>>> +       bb.data.setVar('PR', machine_kernel_pr + suffix, d)
>>> +}
>>> diff --git a/classes/module-base.bbclass b/classes/module-base.bbclass
>>> index 9aaaa4e..1a97955 100644
>>> --- a/classes/module-base.bbclass
>>> +++ b/classes/module-base.bbclass
>>> @@ -2,18 +2,11 @@ inherit module_strip
>>>  
>>>  inherit kernel-arch
>>>  
>>> +inherit machine-kernel-pr
>>> +
>>>  export OS = "${TARGET_OS}"
>>>  export CROSS_COMPILE = "${TARGET_PREFIX}"
>>>  
>>> -# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
>>> -# rebuilds for kernel and external modules
>>> -python __anonymous () {
>>> -    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
>>> -
>>> -    if machine_kernel_pr:
>>> -       bb.data.setVar('PR', machine_kernel_pr, d)
>>> -}
>>> -
>>>  export KERNEL_VERSION = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion')}"
>>>  export KERNEL_SOURCE = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-source')}"
>>>  KERNEL_OBJECT_SUFFIX = "${@[".o", ".ko"][base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion') > "2.6.0"]}"
>
Koen Kooi - April 4, 2011, 5:54 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04-04-11 17:14, Andreas Oberritter wrote:
> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>> Ping. Any votes for or against this patch with either appended or
>>> prepended PR?
>>
>> I don't like this patch *at all*. Recipes can already do
>> MACHINE_KERNEL_PR_append = "something" if they need to.
> 
> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
> append something to it in a recipe.
> 
>> Can you should me a specific example on how this would be an improvement?
> 
> With the version below. a distro can start to use MACHINE_KERNEL_PR any
> time without breaking updates.
> 
> The vast majority of modules do not use MACHINE_KERNEL_PR

It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
sets it. The distro has *no* say in it. Therefore the module recipes
don't need to set it, since it's automatic.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFNmgXgMkyGM64RGpERAr9CAJ9urNa16wfGmCwh4/NvUZd4GOUm2wCfcCrV
Q4CvioBpYYMC4bjEHL4cUD0=
=t85H
-----END PGP SIGNATURE-----
Tom Rini - April 4, 2011, 6:12 p.m.
On 04/04/2011 10:54 AM, Koen Kooi wrote:
> On 04-04-11 17:14, Andreas Oberritter wrote:
>> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>>> Ping. Any votes for or against this patch with either appended or
>>>> prepended PR?
>>>
>>> I don't like this patch *at all*. Recipes can already do
>>> MACHINE_KERNEL_PR_append = "something" if they need to.
> 
>> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
>> append something to it in a recipe.
> 
>>> Can you should me a specific example on how this would be an improvement?
> 
>> With the version below. a distro can start to use MACHINE_KERNEL_PR any
>> time without breaking updates.
> 
>> The vast majority of modules do not use MACHINE_KERNEL_PR
> 
> It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
> sets it. The distro has *no* say in it. Therefore the module recipes
> don't need to set it, since it's automatic.

If I read the original thread right, Andreas' problem is that changing
the module recipe (ie fix a bug in the version, not a new version) means
you need to change all the machine PRs to get it bumped.  This seems
wrong to me.
Andreas Oberritter - April 4, 2011, 6:12 p.m.
On 04/04/2011 07:54 PM, Koen Kooi wrote:
> On 04-04-11 17:14, Andreas Oberritter wrote:
>> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>>> Ping. Any votes for or against this patch with either appended or
>>>> prepended PR?
>>>
>>> I don't like this patch *at all*. Recipes can already do
>>> MACHINE_KERNEL_PR_append = "something" if they need to.
> 
>> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
>> append something to it in a recipe.
> 
>>> Can you should me a specific example on how this would be an improvement?
> 
>> With the version below. a distro can start to use MACHINE_KERNEL_PR any
>> time without breaking updates.
> 
>> The vast majority of modules do not use MACHINE_KERNEL_PR
> 
> It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
> sets it. The distro has *no* say in it. Therefore the module recipes
> don't need to set it, since it's automatic.

That's nitpicking. Let me rephrase:

With the previously cited version. a *machine* can start to use
MACHINE_KERNEL_PR any time without breaking updates.

The vast majority of module recipes do not use MACHINE_KERNEL_PR. That
means that they use their own PR instead of appending something to
MACHINE_KERNEL_PR, as suggested by you. Setting MACHINE_KERNEL_PR in the
machine's configuration disables automatic rebuilds of 91% of module
recipes.

Regards,
Andreas
Koen Kooi - April 5, 2011, 6:22 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04-04-11 20:12, Andreas Oberritter wrote:
> On 04/04/2011 07:54 PM, Koen Kooi wrote:
>> On 04-04-11 17:14, Andreas Oberritter wrote:
>>> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>>>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>>>> Ping. Any votes for or against this patch with either appended or
>>>>> prepended PR?
>>>>
>>>> I don't like this patch *at all*. Recipes can already do
>>>> MACHINE_KERNEL_PR_append = "something" if they need to.
>>
>>> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
>>> append something to it in a recipe.
>>
>>>> Can you should me a specific example on how this would be an improvement?
>>
>>> With the version below. a distro can start to use MACHINE_KERNEL_PR any
>>> time without breaking updates.
>>
>>> The vast majority of modules do not use MACHINE_KERNEL_PR
>>
>> It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
>> sets it. The distro has *no* say in it. Therefore the module recipes
>> don't need to set it, since it's automatic.
> 
> That's nitpicking. Let me rephrase:
> 
> With the previously cited version. a *machine* can start to use
> MACHINE_KERNEL_PR any time without breaking updates.
> 
> The vast majority of module recipes do not use MACHINE_KERNEL_PR. That
> means that they use their own PR instead of appending something to
> MACHINE_KERNEL_PR, as suggested by you. Setting MACHINE_KERNEL_PR in the
> machine's configuration disables automatic rebuilds of 91% of module
> recipes.

So make all the module recipes use MACHINE_KERNEL_PR and ditch PR. Doing
all the appending in different classes is just fixing this at the wrong end.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFNmrUMMkyGM64RGpERAuLPAJ9YYQEQj0H8R3uncTqpCqx0AtGkpQCeIqlA
RmVtl113MqrJ6axwCMYyhkU=
=rvEk
-----END PGP SIGNATURE-----
Andreas Oberritter - April 5, 2011, 9:08 a.m.
On 04/05/2011 08:22 AM, Koen Kooi wrote:
> On 04-04-11 20:12, Andreas Oberritter wrote:
>> On 04/04/2011 07:54 PM, Koen Kooi wrote:
>>> On 04-04-11 17:14, Andreas Oberritter wrote:
>>>> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>>>>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>>>>> Ping. Any votes for or against this patch with either appended or
>>>>>> prepended PR?
>>>>>
>>>>> I don't like this patch *at all*. Recipes can already do
>>>>> MACHINE_KERNEL_PR_append = "something" if they need to.
>>>
>>>> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
>>>> append something to it in a recipe.
>>>
>>>>> Can you should me a specific example on how this would be an improvement?
>>>
>>>> With the version below. a distro can start to use MACHINE_KERNEL_PR any
>>>> time without breaking updates.
>>>
>>>> The vast majority of modules do not use MACHINE_KERNEL_PR
>>>
>>> It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
>>> sets it. The distro has *no* say in it. Therefore the module recipes
>>> don't need to set it, since it's automatic.
> 
>> That's nitpicking. Let me rephrase:
> 
>> With the previously cited version. a *machine* can start to use
>> MACHINE_KERNEL_PR any time without breaking updates.
> 
>> The vast majority of module recipes do not use MACHINE_KERNEL_PR. That
>> means that they use their own PR instead of appending something to
>> MACHINE_KERNEL_PR, as suggested by you. Setting MACHINE_KERNEL_PR in the
>> machine's configuration disables automatic rebuilds of 91% of module
>> recipes.
> 
> So make all the module recipes use MACHINE_KERNEL_PR and ditch PR.

Thereby making MACHINE_KERNEL_PR mandatory for all machines?
Koen Kooi - April 5, 2011, 9:23 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05-04-11 11:08, Andreas Oberritter wrote:
> On 04/05/2011 08:22 AM, Koen Kooi wrote:
>> On 04-04-11 20:12, Andreas Oberritter wrote:
>>> On 04/04/2011 07:54 PM, Koen Kooi wrote:
>>>> On 04-04-11 17:14, Andreas Oberritter wrote:
>>>>> On 04/04/2011 03:58 PM, Koen Kooi wrote:
>>>>>> On 04-04-11 15:00, Andreas Oberritter wrote:
>>>>>>> Ping. Any votes for or against this patch with either appended or
>>>>>>> prepended PR?
>>>>>>
>>>>>> I don't like this patch *at all*. Recipes can already do
>>>>>> MACHINE_KERNEL_PR_append = "something" if they need to.
>>>>
>>>>> The use of MACHINE_KERNEL_PR is optional, so it feels really wrong to
>>>>> append something to it in a recipe.
>>>>
>>>>>> Can you should me a specific example on how this would be an improvement?
>>>>
>>>>> With the version below. a distro can start to use MACHINE_KERNEL_PR any
>>>>> time without breaking updates.
>>>>
>>>>> The vast majority of modules do not use MACHINE_KERNEL_PR
>>>>
>>>> It seems you don't understand how MACHINE_KERNEL_PR works. The *machine*
>>>> sets it. The distro has *no* say in it. Therefore the module recipes
>>>> don't need to set it, since it's automatic.
>>
>>> That's nitpicking. Let me rephrase:
>>
>>> With the previously cited version. a *machine* can start to use
>>> MACHINE_KERNEL_PR any time without breaking updates.
>>
>>> The vast majority of module recipes do not use MACHINE_KERNEL_PR. That
>>> means that they use their own PR instead of appending something to
>>> MACHINE_KERNEL_PR, as suggested by you. Setting MACHINE_KERNEL_PR in the
>>> machine's configuration disables automatic rebuilds of 91% of module
>>> recipes.
>>
>> So make all the module recipes use MACHINE_KERNEL_PR and ditch PR.
> 
> Thereby making MACHINE_KERNEL_PR mandatory for all machines?

I don't see why not.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFNmt99MkyGM64RGpERAsEeAKCFdLTfAEdS0jQsE3roLugG2yvTtQCbBXx7
z2FSA/yy6GVHr8+pASP14e4=
=mJV7
-----END PGP SIGNATURE-----
Phil Blundell - April 5, 2011, 9:32 a.m.
On Tue, 2011-04-05 at 11:23 +0200, Koen Kooi wrote:
> On 05-04-11 11:08, Andreas Oberritter wrote:
> > Thereby making MACHINE_KERNEL_PR mandatory for all machines?
> 
> I don't see why not.

The majority of machines aren't using it at present so this would be an
unexpected (and presumably unwelcome) change for them.  That seems like
a bad idea unless there is a compelling reason why it needs to be done.

p.
Koen Kooi - April 5, 2011, 10:39 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05-04-11 11:32, Phil Blundell wrote:
> On Tue, 2011-04-05 at 11:23 +0200, Koen Kooi wrote:
>> On 05-04-11 11:08, Andreas Oberritter wrote:
>>> Thereby making MACHINE_KERNEL_PR mandatory for all machines?
>>
>> I don't see why not.
> 
> The majority of machines aren't using it at present so this would be an
> unexpected (and presumably unwelcome) change for them.  That seems like
> a bad idea unless there is a compelling reason why it needs to be done.

First: You don't get to complain about broken kernel module PRs when you
are against rebuilding them when the kernel changes.

Second: the majority of machines doesn't even build, so those kind of
arguments are bogus

So apart from handwaving ("presumably unwelcome") do you have any actual
arguments against moving all machines to MACHINE_KERNEL_PR?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFNmvFnMkyGM64RGpERAqqkAJ4vgIlyQ3cgVAnZS2rcjdGJHjn3uQCfTOjt
UtUZ9dRa9XfWvnganZPrAbg=
=KtE5
-----END PGP SIGNATURE-----
Phil Blundell - April 5, 2011, 10:58 a.m.
On Tue, 2011-04-05 at 12:39 +0200, Koen Kooi wrote:
> So apart from handwaving ("presumably unwelcome") do you have any actual
> arguments against moving all machines to MACHINE_KERNEL_PR?

Well, I can't obviously speak for all the machine config maintainers.
It does seem a bit odd to me that you assume they would wish to use
MACHINE_KERNEL_PR when, despite having had it at their disposal for some
time, they have not availed themselves of that opportunity.

As we've discovered in the past when M_K_PR was originally landed in the
tree (and then again in the contemporary stable branch), having it
suddenly switched on tends to cause PRs to go backwards unless it is
done very carefully.  I would certainly consider that to be "unwelcome"
though perhaps you don't share this viewpoint.

Also, as we have previously discussed, the design of M_K_PR does have
some shortcomings, particularly if you have a MACHINE with multiple
kernel packages.  I personally consider it to be a poorly engineered
solution and, although I have no problem with other people and/or
DISTROs using it if it solves their particular problems, I have no
desire to do so myself and I would be opposed to anything which obliges
me to do so.

p.
Frans Meulenbroeks - April 5, 2011, 11:31 a.m.
2011/4/5 Koen Kooi <koen@dominion.thruhere.net>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05-04-11 11:32, Phil Blundell wrote:
>> On Tue, 2011-04-05 at 11:23 +0200, Koen Kooi wrote:
>>> On 05-04-11 11:08, Andreas Oberritter wrote:
>>>> Thereby making MACHINE_KERNEL_PR mandatory for all machines?
>>>
>>> I don't see why not.
>>
>> The majority of machines aren't using it at present so this would be an
>> unexpected (and presumably unwelcome) change for them.  That seems like
>> a bad idea unless there is a compelling reason why it needs to be done.
>
> First: You don't get to complain about broken kernel module PRs when you
> are against rebuilding them when the kernel changes.
>
> Second: the majority of machines doesn't even build, so those kind of
> arguments are bogus

In the tree I have handy there are 313 machines.
I'd be rather hesitant to make statements that the majority does not
build without having that tested (and if you have: how many do not
build).

These are the files that have M_K_PR in them:
ea3250.conf:MACHINE_KERNEL_PR		= "r0"
nokia900.conf:MACHINE_KERNEL_PR = "r58"
omap3-pandora.conf:MACHINE_KERNEL_PR = "r2"
smartq5.conf:MACHINE_KERNEL_PR = "r1"
smartqv7.conf:MACHINE_KERNEL_PR = "r0"
include/davinci.inc:MACHINE_KERNEL_PR = "r50"
include/kirkwood.inc:MACHINE_KERNEL_PR = "r19"
include/omap3.inc:MACHINE_KERNEL_PR = "r101"
include/omap4.inc:MACHINE_KERNEL_PR = "r101"
include/palmpre.inc:MACHINE_KERNEL_PR = "r93"
include/ti814x.inc:MACHINE_KERNEL_PR = "r1"
include/ti816x.inc:MACHINE_KERNEL_PR = "r1"

Actually it seems quite some machines used to test the 2011.3 release
do not seem to use M_K_PR
(http://wiki.openembedded.org/index.php/Release-2011.03).

Frans.

PS: the machines I feel responsible for, and use regularly all build.
Not all of them do use M_K_PR.

>
> So apart from handwaving ("presumably unwelcome") do you have any actual
> arguments against moving all machines to MACHINE_KERNEL_PR?
>
> -----BEGIN PGP SI-
> Version: GnuPG v1.4.5 (Darwin)
>
> iD8DBQFNmvFnMkyGM64RGpERAqqkAJ4vgIlyQ3cgVAnZS2rcjdGJHjn3uQCfTOjt
> UtUZ9dRa9XfWvnganZPrAbg=
> =KtE5
> -----END PGP SIGNATURE-----
>
>
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>
Mike Westerhof - April 5, 2011, 10:33 p.m.
On 4/5/2011 4:32 AM, Phil Blundell wrote:
> On Tue, 2011-04-05 at 11:23 +0200, Koen Kooi wrote:
>> On 05-04-11 11:08, Andreas Oberritter wrote:
>>> Thereby making MACHINE_KERNEL_PR mandatory for all machines?
>>
>> I don't see why not.
> 
> The majority of machines aren't using it at present so this would be an
> unexpected (and presumably unwelcome) change for them.  That seems like
> a bad idea unless there is a compelling reason why it needs to be done.

Quite frankly, I recall when this was originally discussed, and I really
didn't understand the use case for it -- if it makes sense to use it for
SlugOS, I'd be happy to do so.

On a somewhat tangential note, I am responsible for a *distro*, but I'm
not sure that I'm responsible for any *machines* -- so perhaps this is
not my decision to make WRT to SlugOS?  Am I missing something?

-Mike (mwester)

Patch

diff --git a/classes/kernel.bbclass b/classes/kernel.bbclass
index 0109ce6..0187fb3 100644
--- a/classes/kernel.bbclass
+++ b/classes/kernel.bbclass
@@ -20,13 +20,10 @@  python __anonymous () {
     image = bb.data.getVar('INITRAMFS_IMAGE', d, True)
     if image:
         bb.data.setVar('INITRAMFS_TASK', '${INITRAMFS_IMAGE}:do_rootfs', d)
-
-    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
-
-    if machine_kernel_pr:
-       bb.data.setVar('PR', machine_kernel_pr, d)
 }
 
+inherit machine-kernel-pr
+
 INITRAMFS_IMAGE ?= ""
 INITRAMFS_TASK ?= ""
 
diff --git a/classes/machine-kernel-pr.bbclass b/classes/machine-kernel-pr.bbclass
new file mode 100644
index 0000000..a72a0c0
--- /dev/null
+++ b/classes/machine-kernel-pr.bbclass
@@ -0,0 +1,15 @@ 
+# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
+# rebuilds for kernel and external modules
+python __anonymous () {
+    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
+    if machine_kernel_pr:
+       # Append
+       # a) .X, if the recipe's PR is rX, or
+       # b) +${PR}, if the recipe's PR doesn't begin with r
+       pr = bb.data.getVar('PR', d, True)
+       if pr.startswith('r'):
+               suffix = '.' + pr[1:]
+       else:
+               suffix = '+' + pr
+       bb.data.setVar('PR', machine_kernel_pr + suffix, d)
+}
diff --git a/classes/module-base.bbclass b/classes/module-base.bbclass
index 9aaaa4e..1a97955 100644
--- a/classes/module-base.bbclass
+++ b/classes/module-base.bbclass
@@ -2,18 +2,11 @@  inherit module_strip
 
 inherit kernel-arch
 
+inherit machine-kernel-pr
+
 export OS = "${TARGET_OS}"
 export CROSS_COMPILE = "${TARGET_PREFIX}"
 
-# A machine.conf or local.conf can increase MACHINE_KERNEL_PR to force
-# rebuilds for kernel and external modules
-python __anonymous () {
-    machine_kernel_pr = bb.data.getVar('MACHINE_KERNEL_PR', d, True)
-
-    if machine_kernel_pr:
-       bb.data.setVar('PR', machine_kernel_pr, d)
-}
-
 export KERNEL_VERSION = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion')}"
 export KERNEL_SOURCE = "${@base_read_file('${STAGING_KERNEL_DIR}/kernel-source')}"
 KERNEL_OBJECT_SUFFIX = "${@[".o", ".ko"][base_read_file('${STAGING_KERNEL_DIR}/kernel-abiversion') > "2.6.0"]}"