[Buildroot] [PATCH 1/1] package/olsr: use make foreach loops

[Arnout Vandecappelle]

On 13/01/2020 21:32, Fabrice Fontaine wrote:
> Dear Thomas,
> 
> Le lun. 13 janv. 2020 à 21:17, Thomas Petazzoni
> <thomas.petazzoni at bootlin.com> a écrit :
>>
>> On Mon, 13 Jan 2020 19:08:56 +0100
>> Fabrice Fontaine <fontaine.fabrice at gmail.com> wrote:
>>
>>> Replace shell for loops by make foreach loops
>>>
>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice at gmail.com>
>>> ---
>>>  package/olsr/olsr.mk | 14 +++++++-------
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/package/olsr/olsr.mk b/package/olsr/olsr.mk
>>> index e643a0cfb3..3533e2a1fd 100644
>>> --- a/package/olsr/olsr.mk
>>> +++ b/package/olsr/olsr.mk
>>> @@ -18,19 +18,19 @@ OLSR_DEPENDENCIES = host-flex host-bison
>>>
>>>  define OLSR_BUILD_CMDS
>>>       $(TARGET_CONFIGURE_OPTS) $(MAKE) ARCH=$(KERNEL_ARCH) -C $(@D) olsrd
>>> -     for p in $(OLSR_PLUGINS) ; do \
>>> -             $(TARGET_CONFIGURE_OPTS) $(MAKE) ARCH=$(KERNEL_ARCH) -C $(@D)/lib/$$p ; \
>>> -     done
>>> +     $(foreach p,$(OLSR_PLUGINS),
>>
>> Did you test this? To me, it seems like a backslash is missing at the end of this line.
> Yes, it builds fine. I decided to skip the backslash as some packages
> (ltp-testsuite, selinux-python, policycoreutils, python-pyqt5) don't
> add it while others (systemd, sunxi-tools) do.
> But I can add a backslash if you want, I don't know what is the "best practice".

 It works because it's within a define:

-------------
# This works, because 'define' creates a multiline "statement"
define test1
$(foreach i,1 2 3,
  $(info $(i)))
endef

$(test1)

# The following fail without backslash, because it's interpreted line-by-line
all:
	$(foreach i,4 5,
	  $(info $(i)))

$(foreach i,7 8,
  $(info $(i)))
-------------


 For consistency, I think it makes sense to always use a backslash.

 Regards,
 Arnout




>>
>>> +             $(TARGET_CONFIGURE_OPTS) $(MAKE) ARCH=$(KERNEL_ARCH) -C $(@D)/lib/$(p)
>>> +     )
>>>  endef
>>>
>>>  define OLSR_INSTALL_TARGET_CMDS
>>>       $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \
>>>               prefix="/usr" install_bin
>>> -     for p in $(OLSR_PLUGINS) ; do \
>>> -             $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/lib/$$p \
>>> +     $(foreach p,$(OLSR_PLUGINS),
>>
>> Same here.
>>
>>> +             $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/lib/$(p) \
>>>                       LDCONFIG=/bin/true DESTDIR=$(TARGET_DIR) \
>>> -                     prefix="/usr" install ; \
>>> -     done
>>> +                     prefix="/usr" install
>>> +     )
>>
>> Best regards,
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
> Best Regards,
> 
> Fabrice
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>