Skip to content

Commit

Permalink
Changes to Lua module loading, and file generation.
Browse files Browse the repository at this point in the history
This change has several parts:

1. Resurrected tools/upbc.  The code was all there but the build was
   broken for open-source.  Now you can type "make tools/upbc" and
   it will build all necessary Lua modules and create a robust shell
   script for running upbc.

2. Changed Lua module loading to no longer rely on OS-level .so
   dependencies.  The net effect of this is that you now only need
   to set LUA_PATH and LUA_CPATH; setting LD_LIBRARY_PATH or rpaths
   is no longer required.  Downside: this drops compatibility with
   Lua 5.1, since it depends on a feature that only exists in Lua >=5.2
   (and LuaJIT).

3. Since upbc works again, I fixed the re-generation of the descriptor
   files (descriptor.upb.h, descriptor.upb.c).  "make genfiles" will
   re-generate these as well as the JIT code generator.

4. Added a Travis test target that ensures that the checked-in generated
   files are not out of date.  I would do this for the Ragel generated
   file also, but we can't count on all versions of Ragel to necessarily
   generate identical output.

5. Changed Makefile to no longer automatically run Ragel to regenerate
   the JSON parser.  This is unfortuante, because it's convenient when
   you're developing the JSON parser.  However, "git clone" sometimes
   skews the timestamps a little bit so that "make" thinks it needs to
   regenerate these files for a fresh "git clone."  This would normally
   be harmless, but if the user doesn't have Ragel installed, it makes
   the build fail completely.  So now you have to explicitly regenerate
   the Ragel output.  If you want to you can uncomment the auto-generation
   during development.
  • Loading branch information
haberman committed May 13, 2015
1 parent 35923b4 commit 51cf616
Show file tree
Hide file tree
Showing 12 changed files with 234 additions and 176 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ env:
- UPB_TRAVIS_BUILD=lua
- UPB_TRAVIS_BUILD=coverage
- UPB_TRAVIS_BUILD=warnings
- UPB_TRAVIS_BUILD=genfiles
87 changes: 52 additions & 35 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,19 @@ clean_leave_profile:
@rm -rf obj lib
@rm -f tests/google_message?.h
@rm -f $(TESTS) tests/testmain.o tests/t.*
@rm -f upb/descriptor.pb
@rm -f upb/descriptor/descriptor.pb
@rm -rf tools/upbc deps
@rm -rf upb/bindings/python/build
@rm -f upb/bindings/ruby/Makefile
@rm -f upb/bindings/ruby/upb.o
@rm -f upb/bindings/ruby/upb.so
@rm -f upb/bindings/ruby/mkmf.log
@rm -f tests/google_messages.pb.*
@rm -f tests/google_messages.proto.pb
@rm -f upb.c upb.h
@find . | grep dSYM | xargs rm -rf

clean: clean_leave_profile
clean: clean_leave_profile clean_lua
@rm -rf $(call rwildcard,,*.gcno) $(call rwildcard,,*.gcda)

# A little bit of Make voodoo: you can call this from the deps of a patterned
Expand Down Expand Up @@ -166,8 +169,20 @@ upb_json_SRCS = \
upb/json/parser.c \
upb/json/printer.c \

upb/json/parser.c: upb/json/parser.rl
$(E) RAGEL $<
# Ideally we could keep this uncommented, but Git apparently sometimes skews
# timestamps slightly at "clone" time, which makes "Make" think that it needs
# to rebuild upb/json/parser.c when it actually doesn't. This would be harmless
# except that the user might not have Ragel installed.
#
# So instead we require an excplicit "make ragel" to rebuild this (for now).
# More pain for people developing upb/json/parser.rl, but less pain for everyone
# else.
#
# upb/json/parser.c: upb/json/parser.rl
# $(E) RAGEL $<
# $(Q) ragel -C -o upb/json/parser.c upb/json/parser.rl
ragel:
$(E) RAGEL upb/json/parser.rl
$(Q) ragel -C -o upb/json/parser.c upb/json/parser.rl

# If the user doesn't specify an -O setting, we use -O3 for critical-path
Expand Down Expand Up @@ -219,13 +234,24 @@ upb/descriptor/descriptor.pb: upb/descriptor/descriptor.proto
@# TODO: replace with upbc
protoc upb/descriptor/descriptor.proto -oupb/descriptor/descriptor.pb

descriptorgen: upb/descriptor/descriptor.pb tools/upbc
@# Regenerate descriptor_const.h
./tools/upbc -o upb/descriptor/descriptor upb/descriptor/descriptor.pb

tools/upbc: tools/upbc.c $(LIBUPB)
$(E) CC $<
$(Q) $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $< $(LIBUPB)
genfiles: upb/descriptor/descriptor.pb tools/upbc
./tools/upbc upb/descriptor/descriptor.pb upb/descriptor/descriptor google_protobuf_descriptor
lua dynasm/dynasm.lua upb/pb/compile_decoder_x64.dasc > upb/pb/compile_decoder_x64.h || (rm upb/pb/compile_decoder_x64.h ; false)

# upbc depends on these Lua extensions.
UPBC_LUA_EXTS = \
upb/bindings/lua/upb_c.so \
upb/bindings/lua/upb/pb_c.so \
upb/bindings/lua/upb/table_c.so \

tools/upbc: $(UPBC_LUA_EXTS) Makefile
$(E) ECHO tools/upbc
$(Q) echo "#!/bin/sh" > tools/upbc
$(Q) echo 'BASE=`dirname "$$0"`' >> tools/upbc
$(Q) echo 'export LUA_CPATH="$$BASE/../upb/bindings/lua/?.so"' >> tools/upbc
$(Q) echo 'export LUA_PATH="$$BASE/?.lua;$$BASE/../upb/bindings/lua/?.lua"' >> tools/upbc
$(Q) echo 'lua $$BASE/upbc.lua "$$@"' >> tools/upbc
$(Q) chmod a+x tools/upbc

examples/msg: examples/msg.c $(LIBUPB)
$(E) CC $<
Expand Down Expand Up @@ -379,14 +405,14 @@ tests/bindings/googlepb/test_vs_proto2.googlemessage2: $(GOOGLEPB_TEST_DEPS) \
# Lua extension ##################################################################

ifeq ($(shell uname), Darwin)
LUA_LDFLAGS = -undefined dynamic_lookup
LUA_LDFLAGS = -undefined dynamic_lookup -flat_namespace
else
LUA_LDFLAGS =
endif

LUAEXTS = \
upb/bindings/lua/upb.so \
upb/bindings/lua/upb/pb.so \
upb/bindings/lua/upb_c.so \
upb/bindings/lua/upb/pb_c.so \

LUATESTS = \
tests/bindings/lua/test_upb.lua \
Expand All @@ -398,46 +424,37 @@ testlua: lua
@set -e # Abort on error.
@for test in $(LUATESTS) ; do \
echo LUA $$test; \
LUA_PATH="tests/bindings/lua/lunit/?.lua" \
LUA_PATH="tests/bindings/lua/lunit/?.lua;upb/bindings/lua/?.lua" \
LUA_CPATH=upb/bindings/lua/?.so \
lua $$test; \
done

clean: clean_lua
clean_lua:
@rm -f upb/bindings/lua/upb.lua.h
@rm -f upb/bindings/lua/upb.so
@rm -f upb/bindings/lua/upb/pb.so
@rm -f upb/bindings/lua/upb_c.so
@rm -f upb/bindings/lua/upb/pb_c.so
@rm -f upb/bindings/lua/upb/table_c.so

lua: $(LUAEXTS)

upb/bindings/lua/upb.lua.h:
$(E) XXD upb/bindings/lua/upb.lua
$(Q) xxd -i < upb/bindings/lua/upb.lua > upb/bindings/lua/upb.lua.h

# Right now the core upb module depends on all of these.
# It's a TODO to factor this more cleanly in the code.
LUA_LIB_DEPS = \
lib/libupb.pb_pic.a \
lib/libupb.descriptor_pic.a \
lib/libupb_pic.a \

upb/bindings/lua/upb.so: upb/bindings/lua/upb.c upb/bindings/lua/upb.lua.h $(LUA_LIB_DEPS)
upb/bindings/lua/upb_c.so: upb/bindings/lua/upb.c $(LUA_LIB_DEPS)
$(E) CC upb/bindings/lua/upb.c
$(Q) $(CC) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CFLAGS) -fpic -shared -o $@ $< $(LUA_LDFLAGS) $(LUA_LIB_DEPS)

# TODO: the dependency between upb/pb.so and upb.so is expressed at the
# .so level, which means that the OS will try to load upb.so when upb/pb.so
# is loaded. This is what we want, but getting the paths right is tricky.
# Basically the dynamic linker needs to be able to find upb.so at:
# $(LD_LIBRARY_PATH)/upb/bindings/lua/upb.so
# So the user has to set both LD_LIBRARY_PATH and LUA_CPATH correctly.
# Another option would be to require the Lua program to always require
# "upb" before requiring eg. "upb.pb", and then the dependency would not
# be expressed at the .so level.
upb/bindings/lua/upb/pb.so: upb/bindings/lua/upb/pb.c upb/bindings/lua/upb.so
$(E) CC upb/bindings/lua/upb.pb.c
$(Q) $(CC) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CFLAGS) -fpic -shared -o $@ $< upb/bindings/lua/upb.so $(LUA_LDFLAGS)
upb/bindings/lua/upb/table_c.so: upb/bindings/lua/upb/table.c lib/libupb_pic.a
$(E) CC upb/bindings/lua/upb/table.c
$(Q) $(CC) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CFLAGS) -fpic -shared -o $@ $< $(LUA_LDFLAGS)

upb/bindings/lua/upb/pb_c.so: upb/bindings/lua/upb/pb.c $(LUA_LIB_DEPS)
$(E) CC upb/bindings/lua/upb/pb.c
$(Q) $(CC) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CFLAGS) -fpic -shared -o $@ $< $(LUA_LDFLAGS)


# Python extension #############################################################
Expand Down
4 changes: 3 additions & 1 deletion tests/bindings/lua/test_upb.pb.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@

local upb = require "upb"
-- Require "pb" first to ensure that the transitive require of "upb" is
-- handled properly by the "pb" module.
local pb = require "upb.pb"
local upb = require "upb"
local lunit = require "lunit"

if _VERSION >= 'Lua 5.2' then
Expand Down
2 changes: 1 addition & 1 deletion tools/dump_cinit.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
--]]

local upbtable = require "upbtable"
local upbtable = require "upb.table"
local upb = require "upb"
local export = {}

Expand Down
6 changes: 6 additions & 0 deletions tools/upbc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ local upb = require "upb"
local src = arg[1]
local outbase = arg[2]
local basename = arg[3]

if not (src and outbase and basename) then
print("Usage: upbc <binary descriptor> <output filename base> <symbol prefix>")
return 1
end

local hfilename = outbase .. ".upb.h"
local cfilename = outbase .. ".upb.c"

Expand Down
15 changes: 15 additions & 0 deletions travis.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ lua_script() {
make -j12 testlua USER_CPPFLAGS=`pkg-config lua5.2 --cflags`
}

# Test that generated files don't need to be regenerated.
#
# We would include the Ragel output here too, but we can't really guarantee
# that its output will be stable for multiple versions of the tool, and we
# don't want the test to be brittle.
genfiles_install() {
sudo apt-get update -qq
sudo apt-get install lua5.2 liblua5.2-dev protobuf-compiler
}
genfiles_script() {
make -j12 genfiles USER_CPPFLAGS=`pkg-config lua5.2 --cflags`
# Will fail if any differences were observed.
git diff --exit-code
}

# A run that executes with coverage support and uploads to coveralls.io
coverage_install() {
sudo apt-get update -qq
Expand Down
17 changes: 2 additions & 15 deletions upb/bindings/lua/upb.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@
#include "upb/pb/glue.h"
#include "upb/shim/shim.h"

static const char upb_lua[] = {
#include "upb/bindings/lua/upb.lua.h"
};

// Lua metatable types.
#define LUPB_MSG "lupb.msg"
#define LUPB_ARRAY "lupb.array"
Expand Down Expand Up @@ -1771,9 +1767,9 @@ static void lupb_setfieldi(lua_State *L, const char *field, int i) {
lua_setfield(L, -2, field);
}

int luaopen_upb(lua_State *L) {
int luaopen_upb_c(lua_State *L) {
static char module_key;
if (lupb_openlib(L, &module_key, "upb", lupb_toplevel_m)) {
if (lupb_openlib(L, &module_key, "upb_c", lupb_toplevel_m)) {
return 1;
}

Expand Down Expand Up @@ -1858,14 +1854,5 @@ int luaopen_upb(lua_State *L) {
lupb_setfieldi(L, "HANDLER_STARTSEQ", UPB_HANDLER_STARTSEQ);
lupb_setfieldi(L, "HANDLER_ENDSEQ", UPB_HANDLER_ENDSEQ);

// Call the chunk that will define the extra functions on upb, passing our
// package dictionary as the argument.
if (luaL_loadbuffer(L, upb_lua, sizeof(upb_lua), "upb.lua") ||
lua_pcall(L, 0, LUA_MULTRET, 0)) {
lua_error(L);
}
lua_pushvalue(L, -2);
lua_call(L, 1, 0);

return 1; // Return package table.
}
Loading

0 comments on commit 51cf616

Please sign in to comment.