John,

See below for a couple of comments, other than these it looks okay to me 
  ...

Paul

John Cui wrote:
> 
> Could you kindly review 6840288? http://cr.opensolaris.org/~johncui/6840288/
> 
> The fix is pretty simple and straightforward. It uses 
> /usr/perl5/bin/perl instead of /usr/bin/perl, and uses cp instead of mv.


1. mkdirs
    The lines 42-44, 49-51 and 63-65, for example ..
      42 AWSTATSDIR=${PREFIX}/share/doc/awstats
      43 rm -rf ${AWSTATSDIR}
      44 mkdir ${AWSTATSDIR}
    these directories should really be defined in 'Targetdirs', so
    you don't really need the 'rm -rf' and 'mkdir' for them here.

2. chmod +w
    The lines 53 and 74 add the write permissions to the files
    which are then copied (cp -r) into the ws proto area: Do
    you install stuff into /usr with the 'write permission' bits
    set ? If you do you should not be doing that (I can't check as
    I do not have access to the other ws files for the pkg).

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to