[12/23] arch/xtensa: add macros to generate hooks

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

These macro generators, one for the target variant, one for the host
variant, work like the package macro generators, and generate a
post-extract hook.

Packages that need the Xtensa overlay can then:
  - define <PKG>_ARC_XTENSA_OVERLAY_COMPONENT
  - $(eval ...) the appropriate macro (target or host).

This will allow to keep consistency across all packages that need that
overlay, to avoid them diverging again in the future should we need to
change the way we handle the Xtensa overlay (like, allowing it to be
downloaded).

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/arch.mk.xtensa | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Hello,

On Tue, 14 Mar 2017 11:30:41 -0700, Max Filippov wrote:

> +################################################################################
> +# arch-xtensa-overlay-inner -- generates the make targets needed to extract
> +# the xtensa overlay
> +#
> +# argument 1 is the uppercase package name, including a HOST_ prefix
> +#            for host packages
> +#
> +# Packages that call that macro shall define FOO_ARCH_XTENSA_OVERLAY_COMPONENT
> +# and set it to one of the component to extract, one of: binutils, gcc, gdb.
> +#
> +################################################################################
> +define arch-xtensa-overlay-inner
> +
> +ifneq ($$(ARCH_XTENSA_CORE_NAME),)
> +
> +define $(1)_XTENSA_OVERLAY_EXTRACT
> +	$$(call arch-xtensa-overlay-extract,$$(@D),$$($(1)_ARCH_XTENSA_OVERLAY_COMPONENT))
> +endef
> +$(1)_POST_EXTRACT_HOOKS += $(1)_XTENSA_OVERLAY_EXTRACT
> +
> +endif # ARCH_XTENSA_CORE_NAME != ""
> +
> +endef # arch-xtensa-overlay-inner
> +
> +################################################################################
> +# arch-xtensa-overlay -- the target generator macro for the Xtensa overlay
> +################################################################################
> +arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,$(call UPPERCASE,$(pkgname)))
> +host-arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,HOST_$(call UPPERCASE,$(pkgname)))

I must say I am not entirely convinced by this macro. It really makes
thing less obvious to read, and doesn't really remove a lot of code
duplication.

So I'm still hesitating on this one. It's not a big no, but an
hesitation. However, I'm clearly interested by the following stuff that
allows to download the overlay tarball instead of having it in
Buildroot itself.

Thanks,

Thomas

Hi Max,

On 26-03-17 21:44, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 14 Mar 2017 11:30:41 -0700, Max Filippov wrote:
> 
>> +################################################################################
>> +# arch-xtensa-overlay-inner -- generates the make targets needed to extract
>> +# the xtensa overlay
>> +#
>> +# argument 1 is the uppercase package name, including a HOST_ prefix
>> +#            for host packages
>> +#
>> +# Packages that call that macro shall define FOO_ARCH_XTENSA_OVERLAY_COMPONENT
>> +# and set it to one of the component to extract, one of: binutils, gcc, gdb.
>> +#
>> +################################################################################
>> +define arch-xtensa-overlay-inner
>> +
>> +ifneq ($$(ARCH_XTENSA_CORE_NAME),)
>> +
>> +define $(1)_XTENSA_OVERLAY_EXTRACT
>> +	$$(call arch-xtensa-overlay-extract,$$(@D),$$($(1)_ARCH_XTENSA_OVERLAY_COMPONENT))
>> +endef
>> +$(1)_POST_EXTRACT_HOOKS += $(1)_XTENSA_OVERLAY_EXTRACT
>> +
>> +endif # ARCH_XTENSA_CORE_NAME != ""
>> +
>> +endef # arch-xtensa-overlay-inner
>> +
>> +################################################################################
>> +# arch-xtensa-overlay -- the target generator macro for the Xtensa overlay
>> +################################################################################
>> +arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,$(call UPPERCASE,$(pkgname)))
>> +host-arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,HOST_$(call UPPERCASE,$(pkgname)))
> 
> I must say I am not entirely convinced by this macro. It really makes
> thing less obvious to read, and doesn't really remove a lot of code
> duplication.
> 
> So I'm still hesitating on this one. It's not a big no, but an
> hesitation. However, I'm clearly interested by the following stuff that
> allows to download the overlay tarball instead of having it in
> Buildroot itself.

 We discussed this again at the BR Summer Camp, and decided that the 2-3 lines
that you save with this macro in only 3 packages are really not worth defining
this macro. So, could you respin this series without the addition of these macros?

 I've marked the patches as Changes Requested in patchwork.

 Regards,
 Arnout

Hello,

On Wed, Jul 5, 2017 at 1:03 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 26-03-17 21:44, Thomas Petazzoni wrote:
>> On Tue, 14 Mar 2017 11:30:41 -0700, Max Filippov wrote:
>>
>>> +################################################################################
>>> +# arch-xtensa-overlay-inner -- generates the make targets needed to extract
>>> +# the xtensa overlay
>>> +#
>>> +# argument 1 is the uppercase package name, including a HOST_ prefix
>>> +#            for host packages
>>> +#
>>> +# Packages that call that macro shall define FOO_ARCH_XTENSA_OVERLAY_COMPONENT
>>> +# and set it to one of the component to extract, one of: binutils, gcc, gdb.
>>> +#
>>> +################################################################################
>>> +define arch-xtensa-overlay-inner
>>> +
>>> +ifneq ($$(ARCH_XTENSA_CORE_NAME),)
>>> +
>>> +define $(1)_XTENSA_OVERLAY_EXTRACT
>>> +    $$(call arch-xtensa-overlay-extract,$$(@D),$$($(1)_ARCH_XTENSA_OVERLAY_COMPONENT))
>>> +endef
>>> +$(1)_POST_EXTRACT_HOOKS += $(1)_XTENSA_OVERLAY_EXTRACT
>>> +
>>> +endif # ARCH_XTENSA_CORE_NAME != ""
>>> +
>>> +endef # arch-xtensa-overlay-inner
>>> +
>>> +################################################################################
>>> +# arch-xtensa-overlay -- the target generator macro for the Xtensa overlay
>>> +################################################################################
>>> +arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,$(call UPPERCASE,$(pkgname)))
>>> +host-arch-xtensa-overlay = $(call arch-xtensa-overlay-inner,HOST_$(call UPPERCASE,$(pkgname)))
>>
>> I must say I am not entirely convinced by this macro. It really makes
>> thing less obvious to read, and doesn't really remove a lot of code
>> duplication.
>>
>> So I'm still hesitating on this one. It's not a big no, but an
>> hesitation. However, I'm clearly interested by the following stuff that
>> allows to download the overlay tarball instead of having it in
>> Buildroot itself.
>
>  We discussed this again at the BR Summer Camp, and decided that the 2-3 lines
> that you save with this macro in only 3 packages are really not worth defining
> this macro. So, could you respin this series without the addition of these macros?

Sure, will do.