Sorry I didn't get this to you last night. I was sidetracked. At any
rate, I took another pass through your webrev. It's looking better, but
the following issues still exist.
usr/src/Targetdirs
usr/src/cmd/Makefile
usr/src/cmd/subversion/Makefile.sfw
usr/src/pkgdefs/Makefile
All have mismerge errors in them that cause them to lose changes
that aren't part of your code change. It looks like you updated
your workspace and somehow while resolving the conflicts, you
managed to mismerge these files, probably via filemerge. You should
only see changes that you made for your project in the webrev.
usr/src/cmd/swig/Makefile.sfw
* line 33 should be CFLAGS += --norunpath
* add CFLAGS="$(CFLAGS)" \ to the environment between lines 38 and 39.
* add INSTALL="$(INSTALL_PROTO)" \ between lines 50 and 51.
* lines 55 and 56 should be (cd $(VER); $(SH) ./install-swig).
usr/src/pkgdefs/SUNWswig/prototype_com
* Please don't install writable files in /usr (many of your files
are 0644). If you fix the prototype_com file entries, proto-fix
will make sure that the proto area is ok during the build.
-Norm
Bruce Rothermal wrote:
> Made changes from review and some responses:
>
> http://cr.opensolaris.org/~bruce_r/swig/
>
> usr/src/cmd/subversion/Makefile.sfw
>
> * remove line 35
> * remove a ".WAIT" from line 36
> * put lines 41, 60-61. 97-98, 134-135 back from the original (this
> appears to have been a mismerge and Mike will be unhappy if you
> undo his fix :-( )
> * remove the SWIG related contents from lines 171-185, 189
> *All Done.*
>
> usr/src/cmd/subversion/swig*
>
> * remove (use "wx rm ..." to do this)
> *Done
>
> *usr/src/cmd/swig/Makefile.sfw
>
> * line 33, why are you overriding "--with-swiglibdir" ? Is the
> default value somehow broken?
> DESTDIR=$(ROOT) fixes
> * line 38, make sure that you are linking against the proto area,
> make sure you add --norunpath and check if you need the standard
> C++ libraries.
> Add to CFLAGS
>
> * lines 38-41, add CFLAGS="$(CFLAGS)" to the environment.
> passed via -e in call to gmake
>
> * line 45, remove
> done
>
> * lines 48-52, remove
> done
>
> * lines 56-57, add "INSTALL=$(INSTALL_PROTO)" and MANSCRIPT to your
> environment
> MANSCRIPT done
> Adding INSTALL=$(INSTALL_PROTO) causes configure to error and exit.
>
> * lines 59-60, remove and use proto-fix to get the perms right on
> the proto area (see a2ps, hpijs, cups, or other component for an
> example)
> Added the proto-fix
>
> usr/src/cmd/swig/install-swig
>
> * incorporate the swig.1 manpage install into Makefile.sfw (and
> anything else that "cd $VER; make install" didn't cover) and
> remove this. It may not be necessary.
>
> SWIG only installs the binary and .i files. My install-swig adds the man
> page and also all the html and pdf docs which was required by the ARC
> review.
>
> usr/src/pkgdefs/Makefile
>
> * put the changes on lines 283, 285-295 back (mismerge, same as above)
> Done
>
> usr/src/pkgdefs/SUNWswig/Makefile
> usr/src/pkgdefs/SUNWswig/depend
>
> * If you only have the "standard" dependencies, use DATAFILE in the
> Makefile and don't deliver your own depend file.
> Done
>
> Bruce
>
> Norm Jacobs wrote:
>
>> Here are some comments.
>>
>> -Norm
>>
>>
>> usr/src/cmd/subversion/Makefile.sfw
>>
>> * remove line 35
>> * remove a ".WAIT" from line 36
>> * put lines 41, 60-61. 97-98, 134-135 back from the original (this
>> appears to have been a mismerge and Mike will be unhappy if you
>> undo his fix :-( )
>> * remove the SWIG related contents from lines 171-185, 189
>>
>> usr/src/cmd/subversion/swig*
>>
>> * remove (use "wx rm ..." to do this)
>>
>> usr/src/cmd/swig/Makefile.sfw
>>
>> * line 33, why are you overriding "--with-swiglibdir" ? Is the
>> default value somehow broken?
>> * line 38, make sure that you are linking against the proto area,
>> make sure you add --norunpath and check if you need the standard
>> C++ libraries.
>> * lines 38-41, add CFLAGS="$(CFLAGS)" to the environment.
>> * line 45, remove
>> * lines 48-52, remove
>> * lines 56-57, add "INSTALL=$(INSTALL_PROTO)" and MANSCRIPT to your
>> environment
>> * lines 59-60, remove and use proto-fix to get the perms right on
>> the proto area (see a2ps, hpijs, cups, or other component for an
>> example)
>>
>> usr/src/cmd/swig/install-swig
>>
>> * incorporate the swig.1 manpage install into Makefile.sfw (and
>> anything else that "cd $VER; make install" didn't cover) and
>> remove this. It may not be necessary.
>>
>> usr/src/pkgdefs/Makefile
>>
>> * put the changes on lines 283, 285-295 back (mismerge, same as above)
>>
>> usr/src/pkgdefs/SUNWswig/Makefile
>> usr/src/pkgdefs/SUNWswig/depend
>>
>> * If you only have the "standard" dependencies, use DATAFILE in the
>> Makefile and don't deliver your own depend file.
>>
>>
>>
>>
>> Bruce Rothermal wrote:
>>
>>> Could someone please review swig changes. Changed subversion Makefile
>>> to use new swig.
>>>
>>> http://cr.opensolaris.org/~bruce_r/swig/
>>>
>>>
>>>
>
> _______________________________________________
> sfwnv-discuss mailing list
> sfwnv-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>
