John Cui wrote:
> Paul and Amanda,
> Thanks for your comments. I reply your emails in one. See below.
>> 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.
> Yes, it is better to use Targetdirs.
>> 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).
> Makefile.sfw:
> $(SRC)/tools/protofix --pkg SUNWawstats --perm
> will ensure the correct permission. So we do not need worry about it.
>> I think that depending on $PERL from $SRC/tools/install.subr might
>> lead to problems. I think it's set there to be used by itself, not to
>> be exported. So if they change the script then they might break your
>> build. There is a $PERL available for the Makefile (defined in
>> ${SRC}/Makefile.master), you could pass it in from there when you
>> call the script, something like:
>>
>> PERL=$(PERL) $(SHELL) ./install-sfw
> Using the PERL from ${SRC}/Makefile.master. However there are some
> install-sfw depend on $PERL from $SRC/tools/install.subr since
> "_install P"
It's fine if the _install functions use $PERL from install.subr, but
those functions are liable to change and $PERL doesn't seem to me to be
part of what install.subr exports, it's an internal variable. Why not
use $PERLmaster everywhere you use $PERL?
If you leave it as it is I'll be happy, at least the shebang for the
perl scripts you are integrating will be ok. Hopefully one day they'll
have a single $PERL variable for the entire workspace.
Everything else looks ok.
Amanda
>
> Please reload the link.
>
> Thanks,
>
> John Cui wrote on 06/02/09 16:51:
>> Hi all,
>> 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.
>>
>> Thanks,
>> --
>> John Cui
>> x82195
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> sfwnv-discuss mailing list
>> sfwnv-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>>
>
> --
> John Cui
> x82195
>
