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

[Henning Schild <henning.schild@siemens.com>]

On Wed, 11 Dec 2019 19:36:05 +0100
Jan Kiszka <jan.kiszka@siemens.com> wrote:

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

Yes, the check is just early catching a weird error that would have
popped up later. That must have been either the build or the step
copying the kernel binary to DEPLOY.

If a new way of building can deal with it, the check can be dropped.

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

I am not sure i fully get the suggestion. I think you suggest to have a
bitbake variable control parts of the config ... that one localversion
key in it.

The user expectation would probably be that the PV will become _the_
version. So i would go for a sanity check for that, and a warning if
not. After that we can discuss a magic that will turn something behind
the first or last "-" in PV into CONFIG_LOCALVERSION and patch that
into the config.

So instead of a new variable, come up with a new recipe naming
convention. And for people that really want to call the recipe
"kernel.bb" they would get the default

PV = "1.0"
PR = ""
PLOCALV = ""

Would have to check if "PR" is the thing after the first "-" ... But
maybe PR is what we are looking for ...

Henning
 
> Jan
>