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.

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.

    The VERSION= line doesn't look right to me, it normally
    has something like the following on it ...
      VERSION="SFWVERS,REV=0.0.0"


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.

3. usr/src/pkgdefs/SUNWruby18u/prototype_sparc.tmpl
     & usr/src/pkgdefs/SUNWruby18u/prototype_i386.tmpl
    Why are these .tmpl files, they are not normally?

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.

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.

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)

    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

    +
    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).

=== 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/
>>


-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products

Reply via email to