Paul Cunningham wrote:
> Eddie,
>
> Here are a few comments ...
>
> Paul
>
> Eddie Luo wrote:
> .... cut ...
>>
>> New webrev can be found at the same place:
>> http://cr.opensolaris.org/~eddie/lua/.
>
> === Start of Comments ===
>
> 1. CDDL headers (most new files)
> Cosmetic, remove the double space as the start of the
> lines in these headers. eg.
> change ...
> # .....
> to
> # .....
Yes.
>
> 2. usr/src/cmd/lua/install-sfw
> Shouldn't the install of the man pages be done
> with (M)..
> _install M doc/lua.1
> so they are modified with the sunman-stability stuff.
>
Yes.
> Do you also need the modify the file permissions installed
> in the proto area (by 'make install') in here so they
> correspond to the permissions in SUNWlua/prototype_com?
> If they are different doesn't it get reported at the end
> of the full ws build.
Yes, it reported some, but I'll fix them in tarball's Makefile.
>
> 3. usr/src/cmd/lua/Makefile.sfw
> How does ...
> 41 $(MAKE) install)
> know to install it in the ws proto area ?
It's controlled by tarball's Makefile & src/Makefile after patching, see
below tarball's Makefile after unpacked and patching:
[zl162101 at mogo lua]$cat lua-5.1.4/Makefile
# makefile for installing Lua
# see INSTALL for installation instructions
# see src/Makefile and src/luaconf.h for further customization
# == CHANGE THE SETTINGS BELOW TO SUIT YOUR ENVIRONMENT
=======================
# Your platform. See PLATS for possible values.
PLAT= none
# Where to install. The installation starts in the src and doc directories,
# so take care if INSTALL_TOP is not an absolute path.
INSTALL_TOP= $(ROOT)/usr
INSTALL_BIN= $(INSTALL_TOP)/bin
INSTALL_INC= $(INSTALL_TOP)/include
INSTALL_LIB= $(INSTALL_TOP)/lib
#INSTALL_MAN= $(INSTALL_TOP)/man/man1
INSTALL_DOC= $(INSTALL_TOP)/share/doc/lua
INSTALL_TEST= $(INSTALL_TOP)/lib/lua/test
#
# You probably want to make INSTALL_LMOD and INSTALL_CMOD consistent with
# LUA_ROOT, LUA_LDIR, and LUA_CDIR in luaconf.h (and also with etc/lua.pc).
INSTALL_LMOD= $(INSTALL_TOP)/share/lua/$V
INSTALL_CMOD= $(INSTALL_TOP)/lib/lua/$V
# How to install. If your install program does not support "-p", then you
# may have to run ranlib on the installed liblua.a (do "make ranlib").
INSTALL= ginstall -p
INSTALL_EXEC= $(INSTALL) -m 0555
INSTALL_DATA= $(INSTALL) -m 0644
#
# If you don't have install you can use cp instead.
# INSTALL= cp -p
# INSTALL_EXEC= $(INSTALL)
# INSTALL_DATA= $(INSTALL)
# Utilities.
MKDIR= mkdir -p
RANLIB= ranlib
# == END OF USER SETTINGS. NO NEED TO CHANGE ANYTHING BELOW THIS LINE
=========
# Convenience platforms targets.
PLATS= aix ansi bsd freebsd generic linux macosx mingw posix solaris
# What to install.
TO_BIN= lua luac
TO_INC= lua.h luaconf.h lualib.h lauxlib.h ../etc/lua.hpp
TO_LIB= liblua.a liblua.so
TO_MAN= lua.1 luac.1
TO_DOC= amazon.gif contents.html cover.png logo.gif lua.css lua.html
luac.html manual.css manual.html readme.html
TO_TEST= bisect.lua cf.lua echo.lua env.lua factorial.lua fib.lua
fibfor.lua globals.lua hello.lua life.lua luac.lua printf.lua
readonly.lua sieve.lua sort.lua table.lua trace-calls.lua
trace-globals.lua xd.lua
# Lua version and release.
V= 5.1
R= 5.1.4
all: $(PLAT)
$(PLATS) clean:
cd src && $(MAKE) $@
test: dummy
src/lua test/hello.lua
install: dummy
cd src && $(MKDIR) $(INSTALL_BIN) $(INSTALL_INC) $(INSTALL_LIB)
$(INSTALL_DOC) $(INSTALL_TEST) $(INSTALL_LMOD) $(INSTALL_CMOD)
cd src && $(INSTALL_EXEC) $(TO_BIN) $(INSTALL_BIN)
cd src && $(INSTALL_DATA) $(TO_INC) $(INSTALL_INC)
cd src && $(INSTALL_EXEC) $(TO_LIB) $(INSTALL_LIB)
cd doc && $(INSTALL_DATA) $(TO_DOC) $(INSTALL_DOC)
cd test && $(INSTALL_EXEC) $(TO_TEST) $(INSTALL_TEST)
...
>
> I assume the tarball unpack and patching is done
> implicitly rather than explicitly in here. Personally
> I prefer to see things done explicitly so I know what
> is going on (ignore this comment if you are happy).
>
> === End of Comments =====
>
Yes, done implicitly by including usr/src/cmd/Makefile.targ.
Paul & Jim: I've updated my webrev and rebuilt the package per your
comments, could you help review again?
Thanks,
Eddie
