[Buildroot] [PATCH next v2] package/luarocks: rework configuration file for per-package folders

[Buildroot] [PATCH next v2] package/luarocks: rework configuration file for per-package folders

[Thomas Petazzoni]

Currently, luarocks.mk generates a configuration file with hardcoded
STAGING_DIR, TARGET_DIR, TARGET_CC, LUAROCKS_CFLAGS and TARGET_LDFLAGS
values. This is not compatible with per-package folders, where the
value of STAGING_DIR, TARGET_DIR, TARGET_CC and possibly
TARGET_CFLAGS/TARGET_LDFLAGS may be different from one package to the
other.

Based on input from Fran?ois Perrad, this commit:

 - Changes the Luarocks configuration file to use os_getenv() for the
   appropriate variables. Since the contents of this file is not
   fixed, it is no longer generated by luarocks.mk using a series of
   'echo' but simply concatenated with the rest of the Luarocks
   configuration file.

 - Adjusts LUAROCKS_RUNV_ENV so that the necessary environment
   variables are now passed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes since v1:
 - Drop the luarocks patch exporting getenv() in the sandbox, as
   Fran?ois explained this is not needed.
 - Define a getenv() function in luarocks-br-config that returns the
   empty string when os_getenv() returns nothing so that luarocks can
   still be called without all the environment variable defined, as
   suggested by Fran?ois.
---
 package/luarocks/luarocks-br-config.lua | 13 +++++++++++++
 package/luarocks/luarocks.mk            | 21 ++++++++-------------
 2 files changed, 21 insertions(+), 13 deletions(-)
 create mode 100644 package/luarocks/luarocks-br-config.lua

diff --git a/package/luarocks/luarocks-br-config.lua b/package/luarocks/luarocks-br-config.lua
new file mode 100644
index 0000000000..32d71e1e21
--- /dev/null
+++ b/package/luarocks/luarocks-br-config.lua
@@ -0,0 +1,13 @@
+-- BR cross-compilation
+local function getenv (name) return os_getenv(name) or '' end
+variables.LUA_INCDIR = getenv('STAGING_DIR') .. [[/usr/include]]
+variables.LUA_LIBDIR = getenv('STAGING_DIR') .. [[/usr/lib]]
+variables.CC = getenv('TARGET_CC')
+variables.LD = getenv('TARGET_CC')
+variables.CFLAGS = getenv('TARGET_CFLAGS')
+variables.LIBFLAG = [[-shared ]] .. getenv('TARGET_LDFLAGS')
+external_deps_dirs = { getenv('STAGING_DIR') .. [[/usr]] }
+gcc_rpath = false
+rocks_trees = { getenv('TARGET_DIR') .. [[/usr]] }
+wrap_bin_scripts = false
+deps_mode = [[none]]
diff --git a/package/luarocks/luarocks.mk b/package/luarocks/luarocks.mk
index cfef8f19b3..1577189007 100644
--- a/package/luarocks/luarocks.mk
+++ b/package/luarocks/luarocks.mk
@@ -30,23 +30,18 @@ endef
 define HOST_LUAROCKS_INSTALL_CMDS
 	rm -f $(LUAROCKS_CONFIG_FILE)
 	$(MAKE1) -C $(@D) install
-	echo "-- BR cross-compilation"                                  >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.LUA_INCDIR = [[$(STAGING_DIR)/usr/include]]"    >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.LUA_LIBDIR = [[$(STAGING_DIR)/usr/lib]]"        >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.CC = [[$(TARGET_CC)]]"                          >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.LD = [[$(TARGET_CC)]]"                          >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.CFLAGS = [[$(LUAROCKS_CFLAGS)]]"                >> $(LUAROCKS_CONFIG_FILE)
-	echo "variables.LIBFLAG = [[-shared $(TARGET_LDFLAGS)]]"        >> $(LUAROCKS_CONFIG_FILE)
-	echo "external_deps_dirs = { [[$(STAGING_DIR)/usr]] }"          >> $(LUAROCKS_CONFIG_FILE)
-	echo "gcc_rpath = false"                                        >> $(LUAROCKS_CONFIG_FILE)
-	echo "rocks_trees = { [[$(TARGET_DIR)/usr]] }"                  >> $(LUAROCKS_CONFIG_FILE)
-	echo "wrap_bin_scripts = false"                                 >> $(LUAROCKS_CONFIG_FILE)
-	echo "deps_mode = [[none]]"                                     >> $(LUAROCKS_CONFIG_FILE)
+	cat $(HOST_LUAROCKS_PKGDIR)/luarocks-br-config.lua >> $(LUAROCKS_CONFIG_FILE)
 endef
 
 $(eval $(host-generic-package))
 
-LUAROCKS_RUN_ENV = LUA_PATH="$(HOST_DIR)/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua"
+LUAROCKS_RUN_ENV = \
+	LUA_PATH="$(HOST_DIR)/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua" \
+	STAGING_DIR="$(STAGING_DIR)" \
+	TARGET_DIR="$(TARGET_DIR)" \
+	TARGET_CC="$(TARGET_CC)" \
+	TARGET_CFLAGS="$(LUAROCKS_CFLAGS)" \
+	TARGET_LDFLAGS="$(TARGET_LDFLAGS)"
 LUAROCKS_RUN_CMD = $(LUA_RUN) $(HOST_DIR)/bin/luarocks
 
 define LUAROCKS_FINALIZE_TARGET
-- 
2.19.1

[Buildroot] [PATCH next v2] package/luarocks: rework configuration file for per-package folders

[Arnout Vandecappelle]

On 20/11/2018 14:01, Thomas Petazzoni wrote:
> +LUAROCKS_RUN_ENV = \
> +	LUA_PATH="$(HOST_DIR)/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua" \
> +	STAGING_DIR="$(STAGING_DIR)" \
> +	TARGET_DIR="$(TARGET_DIR)" \

 These two are already exported in top-level Makefile, so it shouldn't be needed
(note that when a variable is exported, it will get its value from the context
where the shell is started, not from where the 'export' statement is put, so it
does exactly what we need it to).

> +	TARGET_CC="$(TARGET_CC)" \
> +	TARGET_CFLAGS="$(LUAROCKS_CFLAGS)" \
> +	TARGET_LDFLAGS="$(TARGET_LDFLAGS)"

 For these three, I expect other package infras will need it as well, so it
would make sense to export them too.

 Regards,
 Arnout

[Buildroot] [PATCH next v2] package/luarocks: rework configuration file for per-package folders

[Thomas Petazzoni]

Hello,

On Tue, 20 Nov 2018 23:11:28 +0100, Arnout Vandecappelle wrote:
> On 20/11/2018 14:01, Thomas Petazzoni wrote:
> > +LUAROCKS_RUN_ENV = \
> > +	LUA_PATH="$(HOST_DIR)/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua" \
> > +	STAGING_DIR="$(STAGING_DIR)" \
> > +	TARGET_DIR="$(TARGET_DIR)" \  
> 
>  These two are already exported in top-level Makefile, so it shouldn't be needed
> (note that when a variable is exported, it will get its value from the context
> where the shell is started, not from where the 'export' statement is put, so it
> does exactly what we need it to).

ACK.

> > +	TARGET_CC="$(TARGET_CC)" \
> > +	TARGET_CFLAGS="$(LUAROCKS_CFLAGS)" \
> > +	TARGET_LDFLAGS="$(TARGET_LDFLAGS)"  
> 
>  For these three, I expect other package infras will need it as well, so it
> would make sense to export them too.

Generally speaking, I am not a big fan of things exported in the
environment. One thing I very often do when debugging build issues is
copy/paste the command line that Buildroot ran, to run it again on my
own, sometimes doing additional tweaks. When there's more and more
stuff "hidden" (i.e exported in the environment), it makes that more
and more difficult.

In addition, as you can see here, the TARGET_CFLAGS we use are *NOT*
the global TARGET_CFLAGS that Buildroot defines. It's LUAROCKS_CFLAGS,
which is TARGET_CFLAGS + a few additional luarocks specific CFLAGS.

So no, exporting TARGET_CFLAGS globally doesn't solve that particularly
case.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com