On Tue, Jan 14, 2014 at 07:06:34PM +0100, Jason A. Donenfeld wrote:
> >
> 
> On Tue, Jan 14, 2014 at 10:08 AM, John Keeping <j...@keeping.me.uk> wrote:
> > It was more of a "there doesn't seem much overhead to supporting both,
> > since the API is the same".  I think the Makefile should take an
> > approach more like this though:
> >
> >         ifdef NO_LUA
> >                 CGIT_CFLAGS += -DNO_LUA
> >         else if defined(USE_LUAJIT)
> >                 # LuaJIT code goes here
> >         else
> >                 # Lua code goes here
> >         endif
> 
> Okay we've got this fancy autodetection logic now. From the README:
> 
> > If you'd like to compile without Lua support, you may use:
> >    $ make NO_LUA=1
> > And if you'd like to specify a Lua implementation, you may use:
> >    $ make LUA_IMPLEMENTATION=JIT
> > for using the LuaJIT project. Or:
> >
> >    $ make LUA_IMPLEMENTATION=VANILLA
> > for the mainline Lua project. If you specify neither implementation, it will
> > be auto-detected, preferring LuaJIT if both are present.
> 
> From cgit.mk:
> 
> > ifdef NO_LUA
> >     LUA_MESSAGE := linking without specified Lua support
> >     CGIT_CFLAGS += -DNO_LUA
> > else
> > LUAJIT_CFLAGS := $(shell pkg-config --cflags luajit 2>/dev/null)
> > LUAJIT_LIBS := $(shell pkg-config --libs luajit 2>/dev/null)
> > LUA_LIBS := $(shell pkg-config --libs lua 2>/dev/null)
> > LUA_CFLAGS := $(shell pkg-config --cflags lua 2>/dev/null)
> > ifeq (JIT,$(LUA_IMPLEMENTATION))
> >     ifeq ($(strip $(LUAJIT_LIBS)),)
> >          $(error LuaJIT specified via LUA_IMPLEMENTATION=JIT, but library 
> > could not be found.)
> >     endif
> >     LUA_MESSAGE := linking with selected LuaJIT
> >     CGIT_LIBS += $(LUAJIT_LIBS)
> >     CGIT_CFLAGS += $(LUAJIT_CFLAGS)
> > else ifeq (VANILLA,$(LUA_IMPLEMENTATION))
> >     ifeq ($(strip $(LUA_LIBS)),)
> >          $(error Lua specified via LUA_IMPLEMENTATION=VANILLA, but library 
> > could not be found.)
> >     endif
> >     LUA_MESSAGE := linking with selected Lua
> >     CGIT_LIBS += $(LUA_LIBS)
> >     CGIT_LIBS += $(LUA_CFLAGS)
> > else ifneq ($(strip $(LUAJIT_LIBS)),)
> >     LUA_MESSAGE := linking with autodetected LuaJIT
> >     CGIT_LIBS += $(LUAJIT_LIBS)
> >     CGIT_CFLAGS += $(LUAJIT_CFLAGS)
> > else ifneq ($(strip $(LUA_LIBS)),)
> >     LUA_MESSAGE := linking with autodetected Lua
> >     CGIT_LIBS += $(LUA_LIBS)
> >     CGIT_CFLAGS += $(LUA_CFLAGS)
> > else
> >     LUA_MESSAGE := linking without autodetected Lua support
> >     NO_LUA := YesPlease
> >     CGIT_CFLAGS += -DNO_LUA
> > endif
> >
> > endif
> >
> > # Add -ldl to linker flags on non-BSD systems.
> > ifeq ($(findstring BSD,$(uname_S)),)
> >     CGIT_LIBS += -ldl
> > endif
> 
> How's this look to you? The correct way to be doing things?

I think it does the right thing for all the explicitly specified
combinations.

Personally I would let the compiler error out if Lua isn't installed,
and add some documentation in Makefile to point users at NO_LUA, but I
don't feel particularly strongly about that, and since you've done the
hard work to make it more intelligent...
_______________________________________________
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit

Reply via email to