Paul Cunningham wrote: > Chris, > > I'm not to sure if you were say in your email that you do *not* want > to do this "Ruby patch level update" any more, but if not ... > > Here are my comments from my quick skip through of ... > > See http://cr.opensolaris.org/~chriszhu/CR6721879-webrev/ for > > webrev. > > Note: the CR6721879 link can not be viewed externally from the SWAN, > so you need to post links that can be viewed on sfwnv-discuss. > Here the page http://cr.opensolaris.org/~chriszhu/CR6721879-webrev/ is for review. And it's updated according to following comments, please see inline. > Paul > > === Start of Comments === > > 1. usr/src/pkgdefs/SUNWruby18r/pkginfo.tmpl > & usr/src/pkgdefs/SUNWruby18u/pkginfo.tmpl > You don't need the version numbers on the NAME= lines. > Expand the DESC= lines so they are more descriptive. I just follow your advice, and use "Ruby & RubyGems (Root)" and "Ruby & RubyGems (Usr)" as the NAME of packages. So we don't need to update pkginfo and depend files if we do upgrade again. > > The VERSION= line doesn't look right to me, it normally > has something like the following on it ... > VERSION="SFWVERS,REV=0.0.0" > Corrected for both SUNWruby18u and SUNWruby18r > > 2. usr/src/pkgdefs/SUNWruby18u/depend > Line 60: P SUNWruby18r; you don't need all the version > stuff on this line. If you remove it, next time you > rev the version(s) you will not need to change this file. See 1 > > 3. usr/src/pkgdefs/SUNWruby18u/prototype_sparc.tmpl > & usr/src/pkgdefs/SUNWruby18u/prototype_i386.tmpl > Why are these .tmpl files, they are not normally? There are some build specific variables used in the prototype_*.tmpl files that get replaced when make is run on the packages. Directories get created based on the platform. And we are not planning on making any changes to this at the moment. > > 4. usr/src/pkgdefs/SUNWruby18u/prototype_com > & in SUNWruby18u/prototype_i386.tmpl > & SUNWruby18u/prototype_sparc.tmpl > The files (f ) delivered in /usr shouldn't really have > the write permission bit set. You may want to change > that with this update. We will add it on the list of things to do in next update, but keep it as it was at the moment.
> > 5. METADATA > Doesn't the version in the usr/src/cmd/ruby18/METADATA > file need updating? If you haven't got a METADATA file > you should probably add one with all the standard fields. Added > > 6. usr/src/cmd/ruby18/Makefile.sfw > All the normal comments apply maybe, so you might want to > apply them ... > > 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. > Christopher Mi wrote: > > Use the method define in Makefile.master > > since you have a standard METADATA file. > > > > VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) > > TARBALL =$(VER).tar.bz2 > It seems not work, so we define the TARBALL name by ourself. > + > Lines 124 & 125 and 107 & 108, do you really need to > do these 'chmod's, if not remove them (might reduce > build time a tiny bit). I think it makes sure that all directory have the permission of 755, and all users have the right to read files. Maybe it's not so necessary, but I think it's a good habit to make sure that all the files and directories extracted from the tar ball have basic permission for building. So I just keep them as before. Chris > > === End of comments ===== > > > chris zhu wrote: >> Hi all, >> The review for upgrade of ruby to p287 has been blocked for a period >> of time, as we want to solve the problem of "db:migrate core dump >> after install mysql gem" before upgrade. >> After we found out the problem of db:migrate core dump after install >> mysql gem. We raise the request for review the CR6721879 >> <http://monaco.sfbay.sun.com/detail.jsp?cr=6721879> again. >> >> The problem of db:migrate core dump after install mysql gem is shown >> as following: >> 1) installed mysql gem by "gem install mysql -- >> --with-mysql-dir=/usr/mysql/5.0" >> 2) run the db migrate test in web20bench_ror by >> "/usr/ruby/1.8/bin/rake db:migrate", ruby core dump. >> >> The bug is proved to the problem of install mysql gem not the problem >> of ruby itself. >> I test the db migrate in 3 way >> 1) install mysql gem by " gem install mysql -- >> --with-mysql-include=/usr/mysql/5.0/include >> --with-mysql-lib=/usr/mysql/5.0/lib/mysql" >> db migrate runs well, and ruby used the libmysqlclient.so in >> /usr/mysql/5.0/lib/mysql which is compatible with define files in >> /usr/mysql/5.0/include >> 2) install mysql gem by " gem install mysql" >> db migrate runs well, and ruby used the libmysqlclient.so in >> /usr/sfw/lib which is compatible with define files in /usr/sfw/include >> 3) install mysql gem by "gem install mysql -- >> --with-mysql-dir=/usr/mysql/5.0" >> db migrate core dumped, because ruby used the libmysqlclient. in >> /usr/sfw/lib but using the /usr/mysql/5.0/include for mysql/mysql.h >> files, the structure used in running is incompatible with what's >> defined in the include file. >> >> So we prefer not change ruby at all. The webrev is what it looks like >> before. See http://cr.opensolaris.org/~chriszhu/CR6721879-webrev/ >> for webrev. > >> >> >> Chris Zhu wrote: >>> Hi all, >>> >>> Please help to review the CR6721879 >>> <http://monaco.sfbay.sun.com/detail.jsp?cr=6721879> Ruby patch level >>> update to p287. >>> >>> And the webrev is ready on >>> http://cr.opensolaris.org/~chriszhu/CR6721879-webrev/ >>> > >
