George,

This mainly looks good to me, see my comments below ...

Paul

George Vasick wrote:
> 
> I would appreciate your help in reviewing my changes for upgrading 
> binutils to 2.19, the latest stable release:
> 
> http://cr.opensolaris.org/~gvasick/binutils/
> 
> Highlights:
> 
> - binutils 2.19 installed in /usr/gnu/bin with g-prefixed links placed 
> in /usr/bin.  a new package, SUNWgnu-binutils, will be created.
> 
> - existing binutils 2.15 will be retained in /usr/sfw.  binutils 2.19 
> will not build ON without source source changes:
> 
> /export/george/onnv/usr/src/tools/proto/opt/onbld/bin/i386/aw [...] -o 
> debug64/i86_subr.o ../../intel/ia32/ml/i86_subr.s
> ../../intel/ia32/ml/i86_subr.s: Assembler messages:
> ../../intel/ia32/ml/i86_subr.s:1252: Error: suffix or operands invalid 
> for `push'
> ../../intel/ia32/ml/i86_subr.s:1258: Error: suffix or operands invalid 
> for `pop'
> 
> It appears we need to keep binutils 2.15 around until the ON source can 
> be ported to the newer version of the x86 assembler.  Note, the ON build 
> succeeds on Sparc using binutils 2.19.  The compatibility problem is 
> limited to x86.

=== Start of Comments ===

1. usr/src/cmd/gnu-binutils/Makefile.sfw
    The /usr/gnu directories should have been created as
    per Targetdirs, so I don't think you need the following ...
     109     # add pathnames not created by install
     110     if [ ! -d $(ROOT)/usr/bin ]; then \
     111       $(MKDIR) -p $(ROOT)/usr/bin; \
     112     fi
     113     if [ ! -d $(ROOT)/usr/share/man/man1 ]; then \
     114       $(MKDIR) -p $(ROOT)/usr/share/man/man1; \
     115     fi

    Will the following work if some other package (now or in the
    future) has put something in them ? ...
      97    rmdir $(ROOT)/usr/gnu/lib
      98    rmdir $(ROOT)/usr/gnu/include
    107 rmdir $(ROOT)/usr/gnu/$(MACH)-$(MACH_TYPE)-solaris$(SOL_REV)/bin

2. usr/src/pkgdefs/SUNWgnu-binutils/copyright
    The disclaimer at the top, this doesn't look like the full
    version of the disclaimer. And shouldn't is really say ..
     "Sun Microsystems elects to have this ..."

3. usr/src/pkgdefs/SUNWgnu-binutils/pkginfo.tmpl
    For consistency, put the version number on the DESC= line
    in brackets, eg. ..
      DESC="...... (2.19)"

4. usr/src/pkgdefs/SUNWgnu-binutils/prototype_com
     & usr/src/pkgdefs/SUNWgnu-binutils/prototype_i386.tmpl
     & usr/src/pkgdefs/SUNWgnu-binutils/prototype_sparc.tmpl
    Cosmetic:
    Remove the extra blank '#' comment line ..
      1 #

    Change ...
      # SUNWbinutils
    to
      # SUNWgnu-binutils

5. usr/src/cmd/Makefile
     & usr/src/pkgdefs/Makefile
    Changes to these are no in the webrev

6. usr/src/pkgdefs/SUNWgnu-binutils/Makefile
    Is the following required ? ....
       42 prototype_$(MACH): prototype_$(MACH).tmpl
       43         @rm -f prototype_$(MACH)
       44         sed s,{SOL_REV},$(SOL_REV),g \
       45         < prototype_$(MACH).tmpl > prototype_$(MACH)

       31 SOL_REV:sh=uname -r | sed "s,^5\.,2\.,"
     Also, if not then I don't think the  prototype_*.tmpl files
     need to be .tmpl files.

=== End of Comments =====
-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to