Re: [PATCH 2/3] add basic lua infrastructure
Jeff King writes: > On Tue, Sep 25, 2012 at 03:21:10AM +, Robin H. Johnson wrote: > >> On Mon, Sep 24, 2012 at 08:25:12PM -0400, Jeff King wrote: >> > +ifdef USE_LUA >> > + BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2` >> > + EXTLIBS += `pkg-config --libs lua5.2` >> > +endif >> Can you please hoist the packagename out to a variable? It's just plain >> "lua" on Gentoo. > > Yeah. I mentioned these patches were very rough, but I didn't go into > detail on all the bad points. That is definitely one of them. I have no > idea what the "normal" name is; my debian system sticks the version > number in to allow multiple concurrent versions. Yeah, there is no point nitpicking yet. Even the choice of lua is not all that interesting; embedding _any_ reasonable interpreter, and figuring out which operations and codepaths in us benefit most from such embedding, are of bigger interest at this early stage. How about doing this on top at the minimum? You can let pkg-config to tell you where -I and what -l is, or you can set it yourself. $ make USE_LUA=YesPlease \ LUA_INCLUDE_ARG=-I/usr/include/lua5.2 \ LUA_LINK_ARG=-llua5.2 or $ make USE_LUA=lua5.2 Makefile | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git i/Makefile w/Makefile index 620df89..90335ba 100644 --- i/Makefile +++ w/Makefile @@ -1898,8 +1898,14 @@ ifdef USE_NED_ALLOCATOR endif ifdef USE_LUA - BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2` - EXTLIBS += `pkg-config --libs lua5.2` + # You can say + # $ make USE_LUA=YesPlease LUA_INCLUDE_ARG=-I/usr/include + # or + # $ make USE_LUA=lua5.2 + LUA_INCLUDE_ARG ?= $(shell pkg-config --cflags $(USE_LUA)) + LUA_LINK_ARG ?= $(shell pkg-config --libs $(USE_LUA)) + BASIC_CFLAGS += -DUSE_LUA $(LUA_INCLUDE_ARG) + EXTLIBS += $(LUA_LINK_ARG) endif ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] add basic lua infrastructure
On Tue, Sep 25, 2012 at 08:55:23AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Tue, Sep 25, 2012 at 7:25 AM, Jeff King wrote:
> > +ifdef USE_LUA
> > + BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
> > + EXTLIBS += `pkg-config --libs lua5.2`
> > +endif
> > +
>
> I remember we paid noticeable penalty when linking with libcurl to
> main git binary and Linus removed libcurl from main git, moving it to
> git-http-*. Do we pay similar penalty linking to liblua?
I don't think so. The real problem with libcurl is that it brings in a
ton of other libraries:
$ ldd /usr/lib/x86_64-linux-gnu/libcurl.so | awk '{print $1}'
linux-vdso.so.1
libidn.so.11
libssh2.so.1
liblber-2.4.so.2
libldap_r-2.4.so.2
librt.so.1
libgssapi_krb5.so.2
libssl.so.1.0.0
libcrypto.so.1.0.0
librtmp.so.0
libz.so.1
libc.so.6
libgcrypt.so.11
libresolv.so.2
libsasl2.so.2
libgnutls.so.26
libpthread.so.0
/lib64/ld-linux-x86-64.so.2
libkrb5.so.3
libk5crypto.so.3
libcom_err.so.2
libkrb5support.so.0
libdl.so.2
libkeyutils.so.1
libgpg-error.so.0
libtasn1.so.3
libp11-kit.so.0
Compare with lua:
$ ldd /usr/lib/x86_64-linux-gnu/liblua5.2.so | awk '{print $1}'
linux-vdso.so.1
libm.so.6
libdl.so.2
libc.so.6
/lib64/ld-linux-x86-64.so.2
The original timings from Linus are here:
http://article.gmane.org/gmane.comp.version-control.git/123946
The main issue is really hitting all those libraries on a cold cache.
Here are before-and-after timings of:
echo 3 >/proc/sys/vm/drop_caches && git
which should basically just measure startup time. All times are
best-of-five.
[before]
real0m0.065s
user0m0.000s
sys 0m0.004s
[after]
real0m0.063s
user0m0.000s
sys 0m0.004s
So we actually did better, though the difference is well within the
run-to-run noise. I don't think it's a big deal.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] add basic lua infrastructure
On Tue, Sep 25, 2012 at 03:21:10AM +, Robin H. Johnson wrote: > On Mon, Sep 24, 2012 at 08:25:12PM -0400, Jeff King wrote: > > +ifdef USE_LUA > > + BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2` > > + EXTLIBS += `pkg-config --libs lua5.2` > > +endif > Can you please hoist the packagename out to a variable? It's just plain > "lua" on Gentoo. Yeah. I mentioned these patches were very rough, but I didn't go into detail on all the bad points. That is definitely one of them. I have no idea what the "normal" name is; my debian system sticks the version number in to allow multiple concurrent versions. I was hoping somebody with more Lua experience could tell me what's usual. It would be nice if it just worked out of the box as soon as you said USE_LUA, but that may not be realistic. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] add basic lua infrastructure
On Mon, Sep 24, 2012 at 08:25:12PM -0400, Jeff King wrote: > +ifdef USE_LUA > + BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2` > + EXTLIBS += `pkg-config --libs lua5.2` > +endif Can you please hoist the packagename out to a variable? It's just plain "lua" on Gentoo. -- Robin Hugh Johnson Gentoo Linux: Developer, Trustee & Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] add basic lua infrastructure
On Tue, Sep 25, 2012 at 7:25 AM, Jeff King wrote: > +ifdef USE_LUA > + BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2` > + EXTLIBS += `pkg-config --libs lua5.2` > +endif > + I remember we paid noticeable penalty when linking with libcurl to main git binary and Linus removed libcurl from main git, moving it to git-http-*. Do we pay similar penalty linking to liblua? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] add basic lua infrastructure
This adds a small module for examining parts of a commit
from inside a lua interpreter. Eventually you'll be able to
do grep-like filtering and --pretty formatting.
The most naive presentation would be to parse the whole
commit and put it in a lua table. However, instead we build
upon the incremental parsing used by the --format parser,
and lazily parse bits of the commit as the lua code requests
them.
Signed-off-by: Jeff King
---
Set "USE_LUA" in your Makefile to turn it on.
Makefile | 7 +++
lua-commit.c | 166 +++
lua-commit.h | 9
3 files changed, 182 insertions(+)
create mode 100644 lua-commit.c
create mode 100644 lua-commit.h
diff --git a/Makefile b/Makefile
index a49d1db..54473e2 100644
--- a/Makefile
+++ b/Makefile
@@ -636,6 +636,7 @@ LIB_H += log-tree.h
LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
+LIB_H += lua-commit.h
LIB_H += mailmap.h
LIB_H += merge-file.h
LIB_H += merge-recursive.h
@@ -749,6 +750,7 @@ LIB_OBJS += log-tree.o
LIB_OBJS += ll-merge.o
LIB_OBJS += lockfile.o
LIB_OBJS += log-tree.o
+LIB_OBJS += lua-commit.o
LIB_OBJS += mailmap.o
LIB_OBJS += match-trees.o
LIB_OBJS += merge-file.o
@@ -1818,6 +1820,11 @@ endif
COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
endif
+ifdef USE_LUA
+ BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
+ EXTLIBS += `pkg-config --libs lua5.2`
+endif
+
ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
export GIT_TEST_CMP_USE_COPIED_CONTEXT
endif
diff --git a/lua-commit.c b/lua-commit.c
new file mode 100644
index 000..ce1eeeb
--- /dev/null
+++ b/lua-commit.c
@@ -0,0 +1,166 @@
+#include "cache.h"
+#include "lua-commit.h"
+#include "commit.h"
+
+#ifndef USE_LUA
+
+static const char msg[] = "git was built without lua support";
+
+void lua_commit_init(const char *)
+{
+ die(msg);
+}
+
+void lua_commit_format(struct strbuf *,
+ struct format_commit_context *)
+{
+ die(msg);
+}
+
+#else
+
+#include
+#include
+#include
+
+static lua_State *lua;
+
+/* XXX
+ * We need to access this from functions called from inside lua. Probably it
+ * would be cleaner use a lua "register" to let each function access it, but I
+ * haven't looked into it.
+ */
+static struct format_commit_context *c;
+
+static int lua_fun_hash(lua_State *lua)
+{
+ lua_pushstring(lua, sha1_to_hex(c->commit->object.sha1));
+ return 1;
+}
+
+static int lua_fun_abbrev(lua_State *lua)
+{
+ const char *hex;
+ unsigned char sha1[20];
+
+ hex = lua_tostring(lua, -1);
+ if (!hex || get_sha1_hex(hex, sha1)) {
+ lua_pushstring(lua, "abbrev requires a sha1");
+ lua_error(lua);
+ }
+
+ lua_pushstring(lua, find_unique_abbrev(sha1, c->pretty_ctx->abbrev));
+ return 1;
+}
+
+static int get_ident(lua_State *lua, const char *line, int len)
+{
+ struct ident_split s;
+
+ if (split_ident_line(&s, line, len) < 0) {
+ lua_pushstring(lua, "unable to parse ident line");
+ lua_error(lua);
+ }
+
+ lua_createtable(lua, 0, 2);
+ lua_pushstring(lua, "name");
+ lua_pushlstring(lua, s.name_begin, s.name_end - s.name_begin);
+ lua_settable(lua, -3);
+ lua_pushstring(lua, "email");
+ lua_pushlstring(lua, s.mail_begin, s.mail_end - s.mail_begin);
+ lua_settable(lua, -3);
+
+ /* XXX should also put date in the table */
+
+ return 1;
+}
+
+static int lua_fun_author(lua_State *lua)
+{
+ if (!c->commit_header_parsed)
+ parse_commit_header(c);
+ return get_ident(lua, c->message + c->author.off, c->author.len);
+}
+
+static int lua_fun_committer(lua_State *lua)
+{
+ if (!c->commit_header_parsed)
+ parse_commit_header(c);
+ return get_ident(lua, c->message + c->committer.off, c->committer.len);
+}
+
+static int lua_fun_message(lua_State *lua)
+{
+ lua_pushstring(lua, c->message + c->message_off + 1);
+ return 1;
+}
+
+static int lua_fun_subject(lua_State *lua)
+{
+ struct strbuf tmp = STRBUF_INIT;
+
+ if (!c->commit_header_parsed)
+ parse_commit_header(c);
+ if (!c->commit_message_parsed)
+ parse_commit_message(c);
+
+ format_subject(&tmp, c->message + c->subject_off, " ");
+ lua_pushlstring(lua, tmp.buf, tmp.len);
+ return 1;
+}
+
+static int lua_fun_body(lua_State *lua)
+{
+ if (!c->commit_header_parsed)
+ parse_commit_header(c);
+ if (!c->commit_message_parsed)
+ parse_commit_message(c);
+
+ lua_pushstring(lua, c->message + c->body_off);
+ return 1;
+}
+
+void lua_commit_init(const char *snippet)
+{
+ if (!lua) {
+ lua = luaL_newstate();
+ if (!lua)
+ die("unable to open lua interpreter");
+ luaL_openlibs(lua);
+
+#define REG(name) do { \
+ lua_
