Re: [PATCH v5 4/5] linux-custom: rewrite to no longer depend on the kernel's builddeb

[Jan Kiszka <jan.kiszka@siemens.com>]

On 11.12.19 16:43, [ext] Jan Kiszka wrote:
> On 11.12.19 16:20, Gylstorff Quirin wrote:
>>> +do_build() {
>>> +
>>> +    # Print a few things that are of particular interest
>>> +    print_settings
>>> +
>>> +    # Process existing kernel configuration to make sure it is complete
>>> +    # (use defaults for options that were not specified)
>>> +    ${MAKE} O=${KERNEL_BUILD_DIR} olddefconfig prepare || exit ${?}
>>> +
>>> +    # Check if the recipe's PV makes sense
>>> +    KR=$(${MAKE} O=${KERNEL_BUILD_DIR} -s --no-print-directory
>>> kernelrelease)
>>> +    eval $(grep ^CONFIG_LOCALVERSION= ${KERNEL_BUILD_DIR}/${KCONF} ||
>>> true)
>>> +    if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then
>>> +        echo "ERROR: Recipe version (${PV}-${KERNEL_LOCALVERSION})
>>> does not seem to match the kernelrelease (${KR})!" 1>&2
>>> +        echo "ERROR: Make sure the kernel version in your NAME/PV/PR
>>> settings and/or CONFIG_LOCALVERSION are aligned" 1>&2
>>> +        exit 1 > +    fi
>>
>> we have some CI use case where we build the latest git release could we
>> add something like this
>>
>> -    if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then
>> -        echo "ERROR: Recipe version (${PV}-${KERNEL_LOCALVERSION}) does
>> not seem to match the kernelrelease (${KR})!" 1>&2
>> -        echo "ERROR: Make sure the kernel version in your NAME/PV/PR
>> settings and/or CONFIG_LOCALVERSION are aligned" 1>&2
>> -        exit 1
>> +    if [ "${PV}" =~ "latest" ]; then
> 
> I suspect you wanted to suggest != "latest".
> 
>> +        if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then
>> +            echo "ERROR: Recipe version (${PV}-${KERNEL_LOCALVERSION})
>> does not seem to match the kernelrelease (${KR})!" 1>&2
>> +            echo "ERROR: Make sure the kernel version in your
>> NAME/PV/PR settings and/or CONFIG_LOCALVERSION are aligned" 1>&2
>> +            exit 1
>> +        fi
> 
> We need some relaxation path for the check, yes. Given the other
> versioning issue, I'm still trying to build a complete picture.

Looking the Henning's commit that introduced the check, it reads to me
like just addressing constraints of the old build approach. The new one
has a way to set LOCALVERSION from the recipe.

So, what we would rather need than this hard check is the following:

 - optional KERNEL_LOCALVERSION
 - pick-up of LOCALVERSION from the kernel config for the case it was
   defined via the config
 - KERNEL_LOCALVERSION ?= "" to avoid breaking existing users
   needlessly

That approach would both enable CONFIG_LOCALVERSION usage via own
configs as well as convenient management in recipes via
KERNEL_LOCALVERSION. But it has a catch: We need the LOCALVERSION
information already for the templating step while dpkg_configure_kernel
is part of the build.

So we may be left with these options:

 - check if CONFIG_LOCALVERSION == KERNEL_LOCALVERSION, which is true
   when KERNEL_LOCALVERSION is used but could be violated when the
   custom config provides a LOCALVERSION while KERNEL_LOCALVERSION is
   empty
 - always override CONFIG_LOCALVERSION with KERNEL_LOCALVERSION, as in
   this version of the patch - may cause surprises, though
 - try to pick up CONFIG_LOCALVERSION early, but only from a user-
   provided defconfig, not from fragments or templates - maybe too
   unintuitive

Not so easy. Thoughts?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux