[OE-core] can "IMAGE_INSTALL ?= ..." not be written in a more obvious way?

[Khem Raj]

On Tue, Dec 13, 2016 at 12:52 AM, Peter Kjellerstedt
<peter.kjellerstedt at axis.com> wrote:
>> -----Original Message-----
>> From: openembedded-core-bounces at lists.openembedded.org
>> [mailto:openembedded-core-bounces at lists.openembedded.org] On Behalf Of
>> Khem Raj
>> Sent: den 10 december 2016 22:16
>> To: Robert P. J. Day
>> Cc: OE Core mailing list
>> Subject: Re: [OE-core] can "IMAGE_INSTALL ?= ..." not be written in a
>> more obvious way?
>>
>> On Sat, Dec 10, 2016 at 3:40 AM, Robert P. J. Day
>> <rpjday at crashcourse.ca> wrote:
>> >
>> >   i've nattered about this before but not sure i ever got an answer --
>> > here's the last bit of core-image.bbclass:
>> >
>> >   CORE_IMAGE_BASE_INSTALL = '\
>> >     packagegroup-core-boot \
>> >     packagegroup-base-extended \
>> >     \
>> >     ${CORE_IMAGE_EXTRA_INSTALL} \
>> >     '
>> >
>> >   CORE_IMAGE_EXTRA_INSTALL ?= ""
>> >
>> >   IMAGE_INSTALL ?= "${CORE_IMAGE_BASE_INSTALL}"
>> >
>> > the first time i saw that (long ago), it took me a few looks to figure
>> > out what was happening. can this not be written in a more obvious way:
>> >
>> >   CORE_IMAGE_BASE_INSTALL = '\
>> >     packagegroup-core-boot \
>> >     packagegroup-base-extended \
>> >     '
>> >
>> >   CORE_IMAGE_EXTRA_INSTALL ?= ""
>> >
>> >   IMAGE_INSTALL ?= " \
>> >     ${CORE_IMAGE_BASE_INSTALL} \
>> >     ${CORE_IMAGE_EXTRA_INSTALL} \
>> >     "
>> >
>> > is that not equivalent, or am i missing something? it's certainly
>> > clearer as to what's happening if people are perusing the code.
>>
>> They are same AFAICT, dont feel strongly  about readability but feel
>> free to send a patch
>
> Careful now. Changing these can affect image recipes outside of OE-core.
> If one has an image recipe that inherits core-image and then defines
>
> IMAGE_INSTALL = "${CORE_IMAGE_BASE_INSTALL} my-own-packages ..."
>
> then changing CORE_IMAGE_BASE_INSTALL as per above would suddenly
> cause that image to miss including packages that it previously did.

yes I think thats the interface it will end up in. It would be more
explicit for someone to set IMAGE_INSTALL explicitly and not expect
this sort of bundling. its also more readable when you construct
IMAGE_INSTALL

>
> I recommend leaving it as is.

agreed.Probably not worth the churn

>
> //Peter
>