[Buildroot] [v3, 1/4] barebox: prepare for secondary config build

[Arnout Vandecappelle <arnout@mind.be>]

 [Adding other core devs in CC, see the beginning and the end]

On 03/02/16 08:50, Pieter Smith wrote:
> On Wed, Mar 02, 2016 at 12:08:34AM +0100, Arnout Vandecappelle wrote:
>> On 02/29/16 08:47, Pieter Smith wrote:
> [snip]
>>>  
>>> Thanks for the suggestion. It is shaping up nicely. There is one aspect that I
>>> would appreciate input on: To all but completely eliminate duplication in the
>>> makefiles for the two packages, I am wrapping all the current functionality in
>>> boot/barebox/barebox.mk in a parameterized barebox-package function. E.g.:
>>>
>>>   define barebox-package
>>>   $(1)_VERSION = $$(call qstrip,$$(BR2_TARGET_BAREBOX_VERSION))
>>>   ...
>>>   $$(eval $$(kconfig-package))
>>>   endef
>>>   $(eval $(call barebox-package,BAREBOX))
>>>
>>> And the whole of boot/barebox/barebox-2/barebox-2.mk becomes:
>>>
>>>   $(eval $(call barebox-package,BAREBOX_2))
>>
>>  No, I don't think we want this, because it hides a lot of what barebox-2 is
>> doing internally. It's OK and actually better (in my opinion) if barebox-2.mk is
>> just a long list like:
> 
> Pity. This quite elegantly solved my concerns. barebox and barebox-2 differ
> only in menuconfig and barebox environment configuration space. The rest should
> always be identical.
> 
>> BAREBOX_2_VERSION = $(BAREBOX_VERSION)
>> BAREBOX_2_SITE = $(BAREBOX_SITE)
> 
> This shouldn't change much in future, so I can still do the variable copying if
> you prefer.
> 
>> BAREBOX_2_BUILD_CMDS = $(BAREBOX_BUILD_CMDS)
> 
> This will not work. Both the BAREBOX_BUILD_CMDS and BAREBOX_INSTALL_IMAGES_CMDS
> need parameterization. I can solve this with with:
> 1. A scaled down version of the above barebox-package function to generate the
>    boilerplate, or
> 2. Parameterizable functions and duplicated boilerplate.
> 
> IMHO reducing duplication always improves clarity and maintenance, so I would
> go for option 1.
> 
> What has your preference?

 The BUILD_CMDS can be parameterized without using functions, by using
$($(PKG)_...) variables instead of $(BAREBOX_2_...). That's what I tried to show
in my first reply:

define BAREBOX_BUILD_CMDS
	$($(PKG)_BUILD_BAREBOXENV_CMDS)
	$(TARGET_MAKE_ENV) $(MAKE) $(BAREBOX_MAKE_FLAGS) -C $(@D)
	$($(PKG)_BUILD_CUSTOM_ENV)
endef
BAREBOX_2_BUILD_CMDS = $(BAREBOX_BUILD_CMDS)

BAREBOX_2_BUILD_BAREBOXENV_CMDS and BAREBOX_2_BUILD_CUSTOM_ENV will not be set,
so those parts are removed. The rest should be identical for barebox-2, but if
you do need something else you can add something like $($(PKG)_EXTRA_FLAGS).

> 
>> ...
>> $(eval $(kconfig-package))
>>
>>
>>  Regards,
>>  Arnout
> 
> [snip]
> 
> I would like to:
> 1. Ensure that all improvements to barebox propagate to barebox-2 in future.
> 2. Avoid having something error-prone to resolve the above. IMHO using a review
>    process to keep two pieces of code in sync is quite error-prone and an
>    unnecessary cognitive burden on developers.

 There isn't really much to keep in sync between the two, only when you suddenly
add _INSTALL_STAGING_CMDS for instance you'd have to update barebox-2.mk to copy
that as well.


 There are two reasons why I prefer the copying of variables:

1. I think that functions are a bit more difficult to understand.
2. I'm considering to add infrastructure that facilitates the pattern of copying
variables, something like

$(eval $(call inherit-package,barebox))

which would expand to

BAREBOX_2_VERSION ?= BAREBOX_VERSION
BAREBOX_2_SITE ?= BAREBOX_SITE
...
BAREBOX_2_BUILD_CMDS ?= BAREBOX_BUILD_CMDS

so that in the end the barebox-2.mk does indeed become nothing more than a
single eval line, and that the same approach can easily be used for other packages.


 However, this is just my opinion, other developers may see it differently,
hence the CC.


 Regards,
 Arnout

> 
> - Pieter
> 

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF