Cunningham, Paul - UK wrote:
>
> Did you send this out twice? - I've ignore the first one
>
> 1. Have you updated your webrev - it doesn't look like it
>
Updated now
>
> 2. "CONFIGURE_OPTIONS is set to --prefix=/usr in Makefile.master but I
> need it to be ${ROOT}/usr so that statement 52 works correctly and
> installs to the proto area and not /usr"
>
> You need to tell 'make install' where to install it using DESTDIR, or
> what ever you Makefile uses, into the proto area. If you use
> 'configure --prefix=${ROOT}/usr ' that tell it your stuff will be
> installed on the target runtime system in ${ROOT}/usr rather than
> /usr. *Some one correct me if I've got this wrong*. It only needs to
> know about ${ROOT}/usr while its creating the pkg proto stuff
>
thanks for the DESTDIR. I was able to make the changes with this.
>
> 5. I can't see your example 'depend' file (not on the SWAN), but the
> recommended order is 'CDDL Header' then 'Copyright' (not that it
> really matters)
>
>
Changed this, but I think the default one that is copied in if you don't
create your own has copyright first. I don't really care either.
>
> 6. I'll assume you have delete the 'DATAFILES= depend', that says use
> the default 'depend' file.
>
I did not because I found some dependency when I ran this testing tool
that is mentioned in the SFW cheat sheet.
> Paul (Outlook arrrrrrrrrrrrrrrrrrrrrrrrrr!)
>
>
> ------------------------------------------------------------------------
> *From:* sfwnv-discuss-bounces at opensolaris.org
> [mailto:sfwnv-discuss-bounces at opensolaris.org] *On Behalf Of
> *Bruce Rothermal
> *Sent:* 25 June 2008 20:33
> *To:* Cunningham, Paul - UK
> *Cc:* sfwnv-discuss at opensolaris.org
> *Subject:* Re: [sfwnv-discuss] Request for code review - swig utility
>
> comments below
>
> And thank you for looking it over.
>
> Bruce
>
> Cunningham, Paul - UK wrote:
>> See below ...
>>
>> Paul
>>
>>
>>> -----Original Message-----
>>> From: sfwnv-discuss-bounces at opensolaris.org
>>> [mailto:sfwnv-discuss-bounces at opensolaris.org] On Behalf Of
>>> Bruce Rothermal
>>> Sent: 24 June 2008 22:08
>>> To: sfwnv-discuss at opensolaris.org
>>> Subject: [sfwnv-discuss] Request for code review - swig utility
>>>
>>> Made changes as provided on first review.
>>>
>>> Please see http://cr.opensolaris.org/~bruce_r/swig_try2/ and
>>> provide any comments or changes I need to make.
>>>
>>>
>>
>> === Start of additional comments ===
>>
>> 1. usr/src/Targetdirs
>> The indenting of your added bits still doesn't look
>> right in the webrev (tab?)
>>
> I was using vi and tabstop=4. On console every looked aligned.
> Change to 8 and reset alignment.
>> 2. usr/src/cmd/swig/Makefile.sfw
>> 32 # (use default --prefix in Makefile.master)
>> 33 CONFIGURE_OPTIONS += --prefix=${ROOT}/usr
>> Why have you added the '--prefix=' when its already
>> include in the Makefile.master (I think)?
>>
> CONFIGURE_OPTIONS is set to --prefix=/usr in Makefile.master but I
> need it to be ${ROOT}/usr so that statement 52 works correctly and
> installs to the proto area and not /usr
>> 52 $(GMAKE) -e install
>> Is this actually installing it into the proto area? DESTDIR?
>>
>> 42 # LIBS=-lcurses \
>> 39/61 # "CFLAGS=$(CFLAGS)" \
>> Remove these commented out lines
>>
> Comments removed
>> 3. usr/src/cmd/swig/install-swig
>> 23 # Copyright 2007
>> Year wrong - and still wrong in *various* other files :-(
>>
>> 4. usr/src/pkgdefs/SUNWswig/pkginfo
>> Its still in your webrev :-( - it shouldn't be
>>
> I deleted it but somehow it got stuck. I'll have to fiddle with
> sccs and make it be removed.
>> 5. usr/src/pkgdefs/SUNWswig/depend
>> Move Copyright lines to after the "CDDL HEADER END"
>> and correct year
>>
> Using the example template given in the instructions
>
> http://tas.sfbay/net/sfwnv.sfbay/gates/sfwnv/gate/usr/src/pkgdefs/SUNWexpect/depend
>
>> 6. usr/src/pkgdefs/SUNWswig/Makefile
>> 30 DATAFILES= depend
>> Don't think this is needed as you have your own 'depend'
>> file.
>> Copyright year is wrong
>>
> ran the suggested check tool /home/jyri/sfw_tools/sfw_check_pkgs
> and it came up with some additional dependencies I did not know
> about.
>> === End of additional comments =====
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/sfwnv-discuss/attachments/20080626/798c997d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bruce_rothermal.vcf
Type: text/x-vcard
Size: 305 bytes
Desc: not available
URL:
<http://mail.opensolaris.org/pipermail/sfwnv-discuss/attachments/20080626/798c997d/attachment.vcf>