Made the changes and posted them at http://cr.opensolaris.org/~bruce_r/swig/

Had a problem with this change at this point I'm already in $(VER) and 
wish to cd to directory up out of the swig directory.

* lines 55 and 56 should be (cd $(VER); $(SH) ./install-swig)

I also have a question about CFLAGS="$(CFLAGS)". Since I'm using gmake 
with the -e option don't all these options get passed anyway.

Bruce

Norm Jacobs wrote:
> 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
>>   
>>     
>
> _______________________________________________
> sfwnv-discuss mailing list
> sfwnv-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>   


Reply via email to