Evgeny, :-) Evgeny Bessonov wrote: > > >> 3. usr/src/cmd/jruby/install-jruby > >> You don't need the 'mkdir -p ..' stuff as the dir > >> is already in Targetdirs. > > Doesn't look as though you changed this > > But prev. line(36) contains rm ?rf the-same-dir, so it should be > recreated after possible removal (mkdir ?p on next line 37).
I meant you should remove the 'rm -rf ...' as well. The build process ensures that all the directories listed in 'Targetdirs' are created at the start of the build - so if your dir is in there you shouldn't need the 'rm' + 'mkdir' lines. > > >> 8. usr/src/pkgdefs/SUNWjruby/pkginfo.tmpl > >> Some people put the pkg version at the end of the > >> DESC= line (feel free to ignore this) > > I think you probably misunderstood what I meant; > > I don't mean 'move the DESC= line to the end of the file', > > what I meant was 'add something like > > the following to that line' ... > > DESC="Java implementation of the Ruby programming language (1.1.3)" > > and it's probably better to move the line back to > > where it was (rather than it the end of the file). > > Yes. Sure. It's my fault. I'll add version and move DESC back. > > >> 9. usr/src/pkgdefs/SUNWjruby/prototype_com > >> Do all these files really need the write permissions > >> bit set? > > What was your conclusion on this? > > As I see in packages tree there are a lot of examples of 755 for > folders/ runable stuff and 644 permissions set for the rest. Yes I know, but you don't need the write permissions on the files so why have them (unless you really need them) Paul > > Paul Cunningham wrote: > >> Evgeny, >> >> I took another quick look .... >> >> >> 3. usr/src/cmd/jruby/install-jruby >> >> You don't need the 'mkdir -p ..' stuff as the dir >> >> is already in Targetdirs. >> Doesn't look as though you changed this >> >> >> 8. usr/src/pkgdefs/SUNWjruby/pkginfo.tmpl >> >> Some people put the pkg version at the end of the >> >> DESC= line (feel free to ignore this) >> I think you probably misunderstood what I meant; I don't mean 'move >> the DESC= line to the end of the file', what I meant was 'add >> something like the following to that line' ... >> DESC="Java implementation of the Ruby programming language (1.1.3)" >> and it's probably better to move the line back to where it was (rather >> than it the end of the file). >> >> >> 9. usr/src/pkgdefs/SUNWjruby/prototype_com >> >> Do all these files really need the write permissions >> >> bit set? >> What was your conclusion on this? >> >> >> The rest looks okay to me >> >> Paul >> >> >> Evgeny Bessonov wrote: >> >>> Hi Paul, >>> >>> thank you for your review! >>> >>> Code was fixed according to your/ Roland's comments >>> with exception of p3.3 (cp vs _install). >>> I prefer to stay with cp because a lot of >>> stuff (11668 files/ 1858 folders) should be copied. >>> I've updated webrev page: >>> >>> http://cr.opensolaris.org/~jinb/jruby2 >>> >>> Is it better now? >>> >>> Thanks a lot. >>> With best regards >>> Evgeny >>> >>> >>> Paul Cunningham wrote: >>> >>>> Evgeny, >>>> >>>> See below for my comments on this ... >>>> >>>> Paul >>>> >>>> Evgeny Bessonov wrote: >>>> >>>>> >>>>> I'm working on poring JRuby (Java implementation >>>>> of the Ruby language) 1.1.3 into OpenSolaris. >>>>> And here is webrev for jruby/SUNWjruby: >>>>> http://cr.opensolaris.org/~jinb/jruby1/ >>>>> >>>>> We have not enough time till svn_99/100 gate closing >>>>> so could you please review/ approve/ provide your feedback/notes ASAP? >>>> >>>> >>>> >>>> === Start of Comments ==== >>>> >>>> 1. usr/src/cmd/jruby/METADATA >>>> Add line to say where the source code came from 'URL:...' >>>> >>>> 2. usr/src/cmd/jruby/Makefile.sfw >>>> You also might want to use 'env - ' rather than 'env ' >>>> >>>> 3. usr/src/cmd/jruby/install-jruby >>>> You might want to >>>> look at 'Roland Mainz' comments on other people's >>>> install-* scripts, re the shell used and . command, >>>> and apply those comments - look back through >>>> sfwnv-discuss for them. >>>> >>>> You don't need the 'mkdir -p ..' stuff as the dir >>>> is already in Targetdirs. >>>> >>>> Do the files copied into proto with 'cp' need their >>>> permissions set? Why not do it with '_install N ...'. >>>> >>>> 4. usr/src/cmd/jruby/test >>>> Where is this used ? >>>> >>>> 5. usr/src/pkgdefs/SUNWjruby/Makefile >>>> Copyright year is wrong? >>>> >>>> You could remove the null DATAFILES= line >>>> >>>> 6. usr/src/pkgdefs/SUNWjruby/depend >>>> Copyright year is wrong? >>>> >>>> Move copyright lines to after 'CDDL HEADER END' >>>> header. >>>> >>>> 7. usr/src/pkgdefs/SUNWjruby/pkginfo >>>> You don't need this checked-in as it is auto >>>> generated from SUNWjruby/pkginfo.tmpl >>>> >>>> 8. usr/src/pkgdefs/SUNWjruby/pkginfo.tmpl >>>> Some people put the pkg version at the end of the >>>> DESC= line (feel free to ignore this) >>>> >>>> 9. usr/src/pkgdefs/SUNWjruby/prototype_com >>>> Do all these files really need the write permissions >>>> bit set? >>>> >>>> Format and year of the copyright lines are wrong >>>> >>>> 10. usr/src/pkgdefs/SUNWjruby/prototype_i386 >>>> & usr/src/pkgdefs/SUNWjruby/prototype_sparc >>>> Copyright year is wrong? >>>> >>>> === End of Comments ====== >>>> >> -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products General Dynamics Itronix Europe Ltd. Pioneer House Chivers Way Histon, Cambridgeshire, UK, CB24 9NL Ph: +44 (0)1223 200648 FAX: +44 870 4324162 Email: paul.cunningham at tadpole.com This email message is for the sole use of the intended recipient(s) and may contain GDC4S confidential or privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message
