Charles,

See below for my comments from my quick skip through ...

Paul

zheng he - Sun Microsystems - Beijing China wrote:
> 
> I am porting clisp which is an ANSI Common Lisp Implementation. Please
> help review on this, any feedback would be very appreciated.
> 
> 
> Webrev: http://cr.opensolaris.org/~zhenghe/clisp/

=== Start of Comments ===

1. usr/src/cmd/clisp/Makefile.sfw
    I don't think that ...
    32 CLISP_INSTALLDIR=$(ROOT)/usr/clisp/$(COMPONENT_VERSION:sh)
    should include the $(ROOT) otherwise it is telling it it
    is always installed in the ws proto area.

    In the 'all:' rule, move the 'make install' bit down
    to the 'install:' rule.

    Do you need the following lines (or are they historic
    from what ever file you used as a template), if not
    remove them ...
    76  find $(CLISP) -type d -exec /usr/bin/chmod 755 "{}" \;
    77  find $(CLISP) -type f -exec /usr/bin/chmod ugo+r "{}" \;

    Rename the file install-clisp (invoked here) to its more
    common name of install-sfw.

    Change lines ...
     67   /usr/bin/bash ./configure --prefix=$(CLISP_INSTALLDIR) \
     68    --ignore-absence-of-libsigsegv \
     69    --with-readline \
     70    --with-libreadline-prefix="$(ROOT)/$(READLINE_DIR)")
    to ...
       CONFIGURE_OPTIONS = --prefix=$(CLISP_INSTALLDIR)
       CONFIGURE_OPTIONS += --ignore-absence-of-libsigsegv
       CONFIGURE_OPTIONS += --with-readline
       CONFIGURE_OPTIONS += --with-libreadline-prefix=...
       .....
       .....
          /usr/bin/bash ./configure $(CONFIGURE_OPTIONS)

2. usr/src/cmd/clisp/install-clisp
    Rename this to 'install-sfw'

    Roland Mainz wrote:
    > use /usr/bin/ksh93 or /usr/bin/bash for install-sfw*
    > and add a $ set -o errexit # at the beginning and
    > replace ". ${SRC}/tools/install.subr" with
    > "source ${SRC}/tools/install.subr" (the idea is to
    > catch failures in the script and abort it at that
    > point, right now the script will just continue)

    It looks as though the man page comes from the src
    package so you need to modify it with the
    sunman-stability stuff

3. dependencies
    You are using the default package 'depend' file,
    have you checked with the dependency checker script
    that your packages has no other dependencies?

4. usr/src/pkgdefs/SUNWclisp/prototype_com
    Do not install files into /usr with the 'write
    permission' bit set.

    Shouldn't the man page ...
      usr/share/man/man1/clisp.1
    be installed directly into usr/share/man/man1
    rather than as a symbolic link to
      ./../../clisp/2.46/share/man/man1/clisp.1



=== End of Comments =====
-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to