[Buildroot] [PATCH 1/1] package/lua-augeas: new package
Arnout Vandecappelle
arnout at mind.be
Sun Oct 3 10:30:08 UTC 2021
On 01/10/2021 18:35, Herve Codina wrote:
> Hello,
>
> On Fri, 1 Oct 2021 15:26:10 +0200
> Thomas Petazzoni <thomas.petazzoni at bootlin.com> wrote:
>
> [...]
>
>>> diff --git a/package/lua-augeas/Config.in b/package/lua-augeas/Config.in
>>> new file mode 100644
>>> index 0000000000..ed07da5426
>>> --- /dev/null
>>> +++ b/package/lua-augeas/Config.in
>>> @@ -0,0 +1,10 @@
>>> +config BR2_PACKAGE_LUA_AUGEAS
>>> + bool "lua-augeas"
>>> + depends on BR2_PACKAGE_AUGEAS
>>
>> I think a "select" here would be better. I looked at several other
>> package/lua-*/Config.in, and most of the time we select the libraries
>> that they need.
>>
>> Of course, it means you would have to replicate the "depends on" of
>> package/augeas/Config.in, but that's not a big deal.
>
> I used the same dependency construction as the one present in
> python-augeas (python binding for augeas).
> Same kind of binding, same kind of dependency.
>
> I can change to "select" here if you prefer.
Yes, select is preferred. I don't know why it was not done like that for
python-augeas.
[snip]
> If this makes sense, I can add the following comment in v2:
> LDFLAGS=$(LDFLAGS) is present to pass LDFLAGS from environment
> to command line.
> With LDFLAGS in command line, related ordinary asignment present
> in the makefile are ignored and so lua-augeas makefile cannot not
> add '-L/lib' to this value.
Yes please add that comment. It can be in the commit message or in the .mk
file, up to you.
Regards,
Arnout
>
> Or I can simply add a patch to remove the incorrect line in lua-augeas
> makefile and remove 'LDFLAGS=$(LDFLAGS)' from make command line.
>
> Which solution do you prefer ?
>
>
>>
>>> + LUA_VERSION="$(LUAINTERPRETER_ABIVER)" \
>>> + INSTALL_CMOD="/usr/lib/lua/$(LUAINTERPRETER_ABIVER)" \
>>> + LUA_CFLAGS=""
>>
>> LUA_CFLAGS is needed ?
>
> No, LUA_CFLAGS is not needed.
> Will be removed in v2.
>
>>
>>> +define LUA_AUGEAS_BUILD_CMDS
>>> + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE1) -C $(@D) \
>>
>> is $(MAKE1) needed? This project has a single C source file, it would
>> be challenging to have parallel build issues, no?
>
> Changed to $(MAKE)
>
>>
>>> + $(LUA_AUGEAS_CONF_OPTS) all
>>> +endef
>>> +
>>> +define LUA_AUGEAS_INSTALL_TARGET_CMDS
>>> + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE1) -C $(@D) \
>>
>> Same question here.
>
> Same change :)
>
>>
>>> + $(LUA_AUGEAS_CONF_OPTS) DESTDIR="$(TARGET_DIR)" install
>>> +endef
>>> +
>>> +$(eval $(generic-package))
>>
>> Thanks!
>>
>> Thomas
>
> Thanks for the review
>
> Hervé
>
More information about the buildroot
mailing list