Hello Paul,
I will fix my code according to your comments.
All of your comments are clear to me except the number 2.
ad using cp instead of _install: I need to copy ~800 files (complete
directory structure with subdirectories), so it is easier for me to use
just cp. I suppose that _install does only one file. Or is there any
simple way to use _install on whole directory structure? (I know I can
prepare a list of files to be copied, but it seems too complex to me if
I compare it with simple cp command.) And yes, I need to fix
permissions after cp, but only for ~10 files, other ~790 files are OK...
ad Roland Mainz comment: so should I put "set -o xtrace" at the
beginning of my script? (I did not see this in other scripts from SFW
repository...)
and should I replace ". ${SRC}/tools/install.subr" with "source
${SRC}/tools/install.subr""even if other scripts in SFW repository use
the "." convention? (I believe that "." and "source" behave exactly the
same...)
Thank you for your review!
Petr
Paul Cunningham wrote:
> Petr,
>
> See my comments from my quick skip through below ...
>
> Paul
>
> Petr Slechta wrote:
>>
>> my task is to integrate Grails web framework (http://grails.org/)
>> into OpenSolaris 2008.11.
>>
>> I would like to ask you to review my code:
>> http://cr.opensolaris.org/~pslechta/grails/
>
> === Start of Comments ===
>
> 1. usr/src/cmd/grails/METADATA
> Add the URL of the place you got the source code from
>
> 2. usr/src/cmd/grails/install-grails
> The directories created by the mkdirs should
> be in Targetdirs.
>
> Why are you not doing the file copies/chmod (cp ...)
> using the '_install ? ...' function?
>
> Do any of the files copied during the directory
> copy need their permissions corrected afterwards?
>
> Roland Mainz wrote:
> > Please use /usr/bin/ksh93 or /usr/bin/bash for install-sfw*
> > and add a
> > $ set -o xtrace # at the beginning and replace ".
> > ${SRC}/tools/install.subr" with "source ${SRC}/tools/install.subr"
> > (the idea is to catch failures in the script and abort it at that
> > point, right now the script will just continue)
>
> 3. usr/src/pkgdefs/SUNWgrails/depend
> Move Copyright lines down to after the
> "CDDL HEADER END" header and change copyright year
>
> 4. usr/src/pkgdefs/SUNWgrails/Makefile
> You don't need the null DATAFILES=
>
> 5. usr/src/pkgdefs/SUNWgrails/prototype_i386
> & usr/src/pkgdefs/SUNWgrails/prototype_sparc
> Copyright year is wrong
>
> === End of Comments =====
>
