diff mbox

[v5,2/2] pkg-luarocks: fix top-level parallel makefile support

Message ID 1435738247-9015-3-git-send-email-fabio.porcedda@gmail.com
State Accepted
Headers show

Commit Message

Fabio Porcedda July 1, 2015, 8:10 a.m. UTC
In the *-install-target phase the manifest file is being updated, if multiply packages try to update it they fail.

To avoid multiple access to the manifest file use flock to sync
multiple luarocks packages.

e.g. installing three luarocks packages:
make lua-cjson-build lua-coat-build lua-coatpersistent-build
make lua-cjson lua-coat lua-coatpersistent -j

Fix error:
Updating manifest for /home/tetsuya/buildroot/br2/output/target/usr/lib/luarocks/rocks
No existing manifest. Attempting to rebuild...

Error: rock_manifest file not found for lua-coat 0.9.1-1 - not a LuaRocks 2 tree?

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/luarocks/luarocks.mk | 6 ++++--
 package/pkg-luarocks.mk      | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Thomas Petazzoni July 4, 2015, 1:26 p.m. UTC | #1
Dear Fabio Porcedda,

On Wed,  1 Jul 2015 10:10:47 +0200, Fabio Porcedda wrote:

> -LUAROCKS_RUN = LUA_PATH="$(HOST_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua" \
> -	$(LUA_RUN) $(HOST_DIR)/usr/bin/luarocks
> +LUAROCKS_RUN_ENV = LUA_PATH="$(HOST_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua"
> +LUAROCKS_RUN_CMD = $(LUA_RUN) $(HOST_DIR)/usr/bin/luarocks
> +
> +LUAROCKS_RUN = $(LUAROCKS_RUN_ENV) $(LUAROCKS_RUN_CMD)

I've removed completely LUAROCKS_RUN, and used LUAROCKS_RUN_ENV and
LUAROCKS_RUN_CMD in the last place where LUAROCKS_RUN was used.

Applied with these changes, thanks!

However, won't we see some similar issues with other packages? Should
we preventively do a flock on HOST_DIR/STAGING_DIR/TARGET_DIR when
installing to host/staging/target respectively, so that we are sure
that at any given time, we don't have two packages installing at the
same time in one of the directories? Theoretically, this shouldn't be
needed if all packages install their own files, or if they have the
proper dependencies when they are overwriting files from another
package.

Thomas
Arnout Vandecappelle July 6, 2015, 10:03 p.m. UTC | #2
On 07/04/15 15:26, Thomas Petazzoni wrote:
> However, won't we see some similar issues with other packages? Should
> we preventively do a flock on HOST_DIR/STAGING_DIR/TARGET_DIR when
> installing to host/staging/target respectively, so that we are sure
> that at any given time, we don't have two packages installing at the
> same time in one of the directories? Theoretically, this shouldn't be
> needed if all packages install their own files, or if they have the
> proper dependencies when they are overwriting files from another
> package.

 There aren't many packages that would have this problem, but indeed a few. E.g.
packages installing kernel modules will typically call depmod. So yes, it would
be a good idea to do that in general.

 However, how are you going to do that in practice? flock works like fakeroot,
you have to run the commands from within the process. But all our INSTALL_*_CMDS
are supposed to be called as separate shell scripts.

 So we'd have to use something like dotlockfile, but I don't think we can assume
that's installed everywhere, can we? Or else we have to devise something similar
around flock.

 Regards,
 Arnout
Fabio Porcedda July 10, 2015, 11:38 a.m. UTC | #3
On Tue, Jul 7, 2015 at 12:03 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 07/04/15 15:26, Thomas Petazzoni wrote:
>> However, won't we see some similar issues with other packages? Should
>> we preventively do a flock on HOST_DIR/STAGING_DIR/TARGET_DIR when
>> installing to host/staging/target respectively, so that we are sure
>> that at any given time, we don't have two packages installing at the
>> same time in one of the directories? Theoretically, this shouldn't be
>> needed if all packages install their own files, or if they have the
>> proper dependencies when they are overwriting files from another
>> package.
>
>  There aren't many packages that would have this problem, but indeed a few. E.g.
> packages installing kernel modules will typically call depmod. So yes, it would
> be a good idea to do that in general.

Doing it in general is the simplest and safest solution, but to be the
most efficient possible i was thinking to use the same solution of
$(MAKE) and  $(MAKE1) so we need to sync just a small part of
packages.

Maybe we can try to test how much performance is lost if a global lock
for all packages is used.

>  However, how are you going to do that in practice? flock works like fakeroot,
> you have to run the commands from within the process. But all our INSTALL_*_CMDS
> are supposed to be called as separate shell scripts.
>
>  So we'd have to use something like dotlockfile, but I don't think we can assume
> that's installed everywhere, can we? Or else we have to devise something similar
> around flock.

I didn't know dotlockfile, it seems installed by default on Fedora 22
and Ubuntu 14.04.
Maybe the only downside of dotlockfile is that the sleep interval of 5
seconds is not configurable.
Anyway it seems a good solution for locking host/staging/target directories.

After merging the per-package staging feature we need to lock only
host and target.
Finally after developing the per-package host we need to sync just the
target directory.

Best regards
diff mbox

Patch

diff --git a/package/luarocks/luarocks.mk b/package/luarocks/luarocks.mk
index 2b6c975..413e23d 100644
--- a/package/luarocks/luarocks.mk
+++ b/package/luarocks/luarocks.mk
@@ -53,8 +53,10 @@  endef
 
 $(eval $(host-generic-package))
 
-LUAROCKS_RUN = LUA_PATH="$(HOST_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua" \
-	$(LUA_RUN) $(HOST_DIR)/usr/bin/luarocks
+LUAROCKS_RUN_ENV = LUA_PATH="$(HOST_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/?.lua"
+LUAROCKS_RUN_CMD = $(LUA_RUN) $(HOST_DIR)/usr/bin/luarocks
+
+LUAROCKS_RUN = $(LUAROCKS_RUN_ENV) $(LUAROCKS_RUN_CMD)
 
 define LUAROCKS_FINALIZE_TARGET
 	rm -rf $(TARGET_DIR)/usr/lib/luarocks
diff --git a/package/pkg-luarocks.mk b/package/pkg-luarocks.mk
index 83f338e..0a7ba47 100644
--- a/package/pkg-luarocks.mk
+++ b/package/pkg-luarocks.mk
@@ -58,8 +58,8 @@  endif
 #
 ifndef $(2)_INSTALL_TARGET_CMDS
 define $(2)_INSTALL_TARGET_CMDS
-	cd $$($(2)_SRCDIR) && \
-		$$(LUAROCKS_RUN) make --keep $$($(2)_ROCKSPEC) $$($(2)_BUILD_OPTS)
+	cd $$($(2)_SRCDIR) && $$(LUAROCKS_RUN_ENV) flock $$(TARGET_DIR) \
+		$$(LUAROCKS_RUN_CMD) make --keep $$($(2)_ROCKSPEC) $$($(2)_BUILD_OPTS)
 endef
 endif