[lxc-devel] [PATCH v2] add lua binding for the lxc API

[Natanael Copa]

Thanks you very much for working on those bindings!

I think the code is good enough for inclusion in git, even if I have
some comments.


On Thu, 24 Jan 2013 11:42:22 -0500
Dwight Engen <dwight.engen at oracle.com> wrote:

> +# Lua module and scripts
> +if test x"$with_distro" = "xdebian" -o x"$with_distro" = "xubuntu" ; then
> +    LUAPKGCONFIG=lua5.1
> +else
> +    LUAPKGCONFIG=lua
> +fi

I think that we should use 'pkg-config --exists' instead of testing for given distros.

Something like:
if pkg-config --exists lua5.1; then
    LUAPKGCONFIG=lua5.1
else
    LUAPKGCONFIG=lua
fi

In case there might be other variants or if the binding supports both
lua-5.1 and lua-5.2:

for LUAPKGCONFIG in lua5.2 lua5.1 lua; do
    if pkg-config --exists $LUAPKGCONFIG; then
        break
    fi
done

I think you can provide a list of modules to PKG_CHECK_MODULES:

PKG_CHECK_MODULES([LUA], [lua5.1 lua])


> +
> +AC_ARG_ENABLE([lua],
> +	[AC_HELP_STRING([--enable-lua], [enable lua binding])],
> +	[enable_lua=yes], [enable_lua=no])
> +
> +AM_CONDITIONAL([ENABLE_LUA], [test "x$enable_lua" = "xyes"])
> +
> +AM_COND_IF([ENABLE_LUA],
> +	[PKG_CHECK_MODULES([LUA], [$LUAPKGCONFIG >= 5.1],[],[AC_MSG_ERROR([You must install lua-devel for lua 5.1])])

I think this should work without using LUAPKGCONFIG
	[PKG_CHECK_MODULE[LUA], [lua5.1 lua >= 5.1],[],[AC_MSG_ERROR([You must install lua-devel for lua 5.1])])



> --- /dev/null
> +++ b/src/lua-lxc/Makefile.am
> @@ -0,0 +1,26 @@
> +if ENABLE_LUA
> +
> +luadir=$(datadir)/lua/5.1
> +sodir=$(libdir)/lua/5.1/lxc

maybe use something like:

luadir=$(pkg-config --variable INSTALL_LMOD $(LUAPKGCONFIG))
sodir=$(pkg-config --variable INSTALL_CMOD $(LUAPKGCONFIG))/lxc

It depends on the LUAPKGCONFIG variable above...


> diff --git a/src/lua-lxc/lxc.lua b/src/lua-lxc/lxc.lua
> new file mode 100755
> index 0000000..c71de48
> --- /dev/null
> +++ b/src/lua-lxc/lxc.lua
...

> @@ -0,0 +1,412 @@
> +--
> +-- lua lxc module
> +--
> +-- Copyright © 2012 Oracle.
> +--
> +-- Authors:
> +-- Dwight Engen <dwight.engen at oracle.com>
> +--
> +-- This library is free software; you can redistribute it and/or modify
> +-- it under the terms of the GNU General Public License version 2, as
> +-- published by the Free Software Foundation.
> +--
> +-- This program is distributed in the hope that it will be useful,
> +-- but WITHOUT ANY WARRANTY; without even the implied warranty of
> +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +-- GNU General Public License for more details.
> +--
> +-- You should have received a copy of the GNU General Public License along
> +-- with this program; if not, write to the Free Software Foundation, Inc.,
> +-- 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +--
> +
> +local core   = require("lxc.core")
> +local lfs    = require("lfs")
> +local table  = require("table")
> +local string = require("string")
> +local io     = require("io")
> +module("lxc", package.seeall)

I think module( ... , package.seeall) is not available in lua 5.2. I
know you focus on 5.1 for now but...

> +function string:split(delim, max_cols)
> +    local cols = {}
> +    local start = 1
> +    local nextc
> +    repeat
> +	nextc = string.find(self, delim, start)
> +	if (nextc and #cols ~= max_cols - 1) then
> +	    table.insert(cols, string.sub(self, start, nextc-1))
> +	    start = nextc + #delim
> +	else
> +	    table.insert(cols, string.sub(self, start, string.len(self)))
> +	    nextc = nil
> +	end
> +    until nextc == nil or start > #self
> +    return cols
> +end
> +
> +function dirname(path)
> +    local f,output
> +    f = io.popen("dirname " .. path)
> +    output = f:read('*all')
> +    f:close()
> +    return output:sub(1,-2)
> +end
> +
> +function basename(path, suffix)
> +    local f,output
> +    f = io.popen("basename " .. path .. " " .. (suffix or ""))
> +    output = f:read('*all')
> +    f:close()
> +    return output:sub(1,-2)
> +end

We must be able to do better than popen for dirname/basename.

What about using microlight? http://stevedonovan.github.com/microlight/

local ml = require('ml')

function dirname(path)
	local dir, file = ml.splitpath(path)
	return dir
end

function basename(path)
	local dir, file = ml.splitpath(path)
	return file
end

Microlight also ships a split() implementation.

-nc