Paul, Thanks for you comments. Updated webrev has been posted:
http://cr.opensolaris.org/~gvasick/sfwnv/ See comments below. Paul Cunningham wrote: > George, > > See below for my comments from my quick skip through ... > > Paul > > George Vasick wrote: >> >> Please review this webrev for upgrading binutils from 2.15 to 2.19 in >> the SFW consolidation: >> >> http://cr.opensolaris.org/~gvasick/sfwnv/ > > === Start of Comments ==== > > 1. CDDL HEADER and top of files (various changed files) > Change so that they conform to that in ... > "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; > See ... > http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines CDDL and copyrights updated for changed files. > > 2. METADATA > You don't seem to have updated the METADATA file. If there > is not one you need to add it, see ... > http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines Added. > > 3. usr/src/cmd/binutils/Makefile.sfw > Extract the VER= info from the METADATA, see recent pkg > integrations for examples. > Plus maybe add 'TARBALL=$(VER).tar.bz2' then replace > $(VER).tar.bz2 with $(TARBALL) elsewhere Done. > > Change as follows ... > Roland Mainz wrote: > > use "env - ..." and not "env ..." in the Makefiles > > to make sure "configure"&&"make" only see the env > > variables they should really get (and not pick-up > > any random env variable) > > use either $(SHELL) or /usr/bin/bash for "configure" > > calls (so we know which one is used and "configure" > > doesn't pick one itself) Done. > > Update Copyright year > > After using 'make install' to install the files, go you > need to fix the permissions/owner/group on files using ? .. > 'tools/protofix --pkg $$pkg --perm' Done. > > Does it really need the lines ? ... > 67 find $(VER) -type d -exec /usr/bin/chmod 755 "{}" \; > 68 find $(VER) -type f -exec /usr/bin/chmod ugo+r "{}" \; > If not remove them; if it does then change \; to \+ (faster) Removed. > > Does it really need the line ... > 41 @find . -name core -exec rm -f {} \; > If it does, why? Removed. > > 4. Copyright year (various if not all file) > The Copyright year needs updating to 2009 Done. > > 5. symbolic links /usr/sfw to /usr/gnu > I think you need to add these into the proto area. > That is normally done in a install-sfw script (I > think) Links added so proto area matches package prototype. > > 6. usr/src/pkgdefs/SUNWbinutils/prototype_com > Line ... > 43 # SUNWgmake - GNU make > SUNWgmake ??? > > and copyright year Fixed. > > 7. usr/src/pkgdefs/SUNWbinutils/prototype_com > & usr/src/pkgdefs/SUNWbinutils/prototype_i386.tmpl > & usr/src/pkgdefs/SUNWbinutils/prototype_sparc.tmpl > Why have you added the write permission bit to the > new/moved files installed. I believe they should not > have it set in /usr The file permissions were chosen to match the permissions produced by a GNU make install inside of the GNU binutils source tree. In particular, the GNU install generally sets the write bit for user, e.g. 644. > > 8. usr/src/pkgdefs/SUNWbinutils/prototype_i386.tmpl > & usr/src/pkgdefs/SUNWbinutils/prototype_sparc.tmpl > Copyright statement is incorrect and in the wrong place Fixed. > > 9. man pages & sunman-stability > Don't you need to add the sunman-stability stuff to the > pkg's installed man pages; I can't see that you are doing it. sunman step added to binutils/Makefile.sfw > > === End of Comments ====== Thanks again for your help on this. George
