Hi Paul, Mike Sullivan advised me that the extra protofix lines may have been added by them and are required for incrementals. He was right so I've put them back. While I was at it I found that the patches were being applied twice during incrementals so I manipulated the make targets a little.
http://cr.opensolaris.org/~tekgrrl/CR-6871383-01/ Has the changes, the only chnages are in $SRC/cmd/ruby18/Makefile.sfw Could you possibly look over them for me? In the meantime can someone else review the webrev please? Also, how do you change permissions on a file under SCCS control? Last time, I had to remove the file from SCCS and recreate it. Thanks Amanda Amanda Waite wrote: > Paul Cunningham wrote: >> Amanda, >> >> This mainly looks good to me, but below are a few observations (feel >> free to ignore) ... >> >> >> Amanda Waite wrote: >>> >>> Can I get a code review for an update to Ruby 1.8.7. This updates >>> Ruby 1.8.7 to patch level 174 and updates RubyGems to v1.3.5. >>> >>> The webrev is here: http://cr.opensolaris.org/~tekgrrl/CR-6871383 >>> >>> The patch file ${SRC}/cmd/ruby18/patches/ruby_nogdbm.patch had to be >>> removed and re-added so as to reset its permissions. >> >> 1. usr/src/cmd/ruby18/Makefile.sfw >> Why is the 'protofix' done under the 'all: rule rather than >> under the 'install:' rule? >> >> Why are ruby and rubygem builds done under the 'install-*' rules >> rather than under the 'all:' or another rule(s)? (ie. separating >> them from the 'install') > > Much of it is legacy, I didn't want to change what already worked. > There were two places where protofix was being run, one of them before > anything was even built. I basically just removed that and cleaned up > the configure options. It works good so I probably won't change it > unless you have strenuous objections. >> >> Line 95, maybe the 'env ' should be 'env - ' > > In this case it's predictable which environment variables will be used > by the script. With configure scripts and Makefiles it's not so > straight forward and it's best to clean out the environment before > calling them, that's not the case when calling the install-sfw script. > I can change it if you want me to. > >> >> >> 2. usr/src/pkgdefs/SUNWruby18u/prototype_com >> Should you have the write-permission bit set on the files in here? > > These are all under /usr/ruby/1.8, I think that we've covered all the > files in the standard /usr paths. These are the permissions that Ruby > causes to be set during the build and IMO it would be a mistake to > change them, it would also be a lot of work and potentially > error-prone. This works today and it worked before so I've no plans to > change them. > > Thanks for the review > > Amanda > >> >> >> Paul > > _______________________________________________ > sfwnv-discuss mailing list > sfwnv-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
