Bruce,
> http://cr.opensolaris.org/~bruce_r/swig
<http://cr.opensolaris.org/~bruce_r/swig>
More comments ...
1. usr/src/pkgdefs/SUNWswig/depend
Copyright year is wrong
2. usr/src/pkgdefs/SUNWswig/Makefile
I don't think you need the empty 'DATAFILES=' line
3. Everything else looks okay to me
Paul
________________________________
From: Bruce.Rothermal at Sun.COM [mailto:Bruce.Rothermal at Sun.COM]
Sent: 26 June 2008 22:30
To: Cunningham, Paul - UK
Cc: sfwnv-discuss at opensolaris.org
Subject: Re: [sfwnv-discuss] Request for code review - swig
utility
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/
<http://cr.opensolaris.org/%7Ebruce_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/SUNWex
pect/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/20080630/c41022d7/attachment.html>
