Faramarz,

See below for my comments, some of them may have duplicated from other 
review ....

Paul

Faramarz Jalalian wrote:
> 
> Would you please review the changes to include CMake into OpenSolaris? You 
> can find the webrev at:
> 
> http://cr.opensolaris.org/~jalalian/cmake/
> 
> There is "almost" no changes made to the cmake. To make installation easier, 
> the configure script is passed
> /usr as build prefix, but cmake install target is modified to install 
> everything under sfw_stage directory so that
> later install-sfw can pick them up and install them in the right place.

=== Start of Comments ====

1. usr/src/cmd/cmake/Makefile.sfw
    Why have you put this in ? ...
     "# Portions Copyright 2008 Faramarz Jalalian"

    Use predefined '--prefix=' value from Makefile.master. You
    could also use the following instead ..
       ./configure $(CONFIGURE_OPTIONS)
    also for Makefile.master.

    You might want to apply these comments from
    'Roland Mainz'  ...
     > Use "env - ..." instead of "env ...".

     > Use either $(SHELL) or /usr/bin/bash for "configure"
     > calls (so you know which one is used and "configure"
     > doesn't pick one itself).

2. usr/src/cmd/cmake/METADATA
    Add lines for ..
       URL:   <link to where source code came from>
       SUPPORT:        ??
       BUGTRAQ:        solaris/utility/???

3. usr/src/cmd/cmake/install-sfw
    You might want to apply these comments from
   'Roland Mainz'  ...
     > 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)

4. usr/src/Makefile
    Why is this in your webrev?

5. usr/src/pkgdefs/SUNWcmake/depend
    This looks like the default 'depend file! If you have
    no other dependencies then you should remove this file
    and add "DATAFILES= depend" to your SUNWcmake/Makefile.

    Have you checked you have no other pkg dependencies? If
    you have keep this file and add them here. Also ..

    move Copyright lines to after the "CDDL HEADER END"
    header.

6. usr/src/pkgdefs/SUNWcmake/pkginfo
    Delete this file as its generated from the pkginfo.tmpl
    file.

7. usr/src/pkgdefs/SUNWcmake/pkginfo.tmpl
    Copyright year is wrong.

    Expand the 'DESC=' line, maybe ...
DESC="CMake - A cross-platform open-source make system (2.6.1)"

8. usr/src/pkgdefs/SUNWcmake/prototype_com
     & usr/src/pkgdefs/SUNWcmake/prototype_i386
     & usr/src/pkgdefs/SUNWcmake/prototype_sparc
    Copyright year is wrong.

=== End of Comments ======

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products

Reply via email to