Hi Paul, thank you very much for explanations.
I removed rm/mkdir, edited/moved back DESC and set 444 permissions for files. Updated version of JRuby webrev available here: http://cr.opensolaris.org/~jinb/jruby3/ thanks With best regards, Evgeny Paul Cunningham wrote: > 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 ====== >>>>> >>> >
