Roland Mainz wrote:
> Petr Slechta wrote:
>
>> Paul Cunningham wrote:
>>
>>> See my comments from my quick skip through below ...
>>> 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/
>>>>
> [snip]
>
>> 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...
>>
>
> Are the other 790 files even Ok if the caller uses a different "umask" ?
>
Good catch! I need to setup umask in my script first...
>
>> 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...)
>>
>
> Erm... the "xtrace" was a mistake... I was thinking about "errexit",
> e.g. $ set -o errexit # (and "yes" ... some scripts in SFWNV use it and
> I really really _wish_ other scripts would adopt this flag, too - it
> makes any errors much more easy to detect (instead of reading a giant
> build log after the package creation failed)) ...
>
OK, I will do it.
>
>> 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...)
>>
>
> No, they are not exactly the same. In ksh93 "source" is an alias for
> "command ." which allows a script to trap an error if the scriptlet
> which should be included cannot be accessed instead of just exiting (the
> standard requires that if "." fails to read/execute the scriptlet the
> shell has to exit immediately - the extra "command" in front of "."
> allows the shell to continue ("command" just returns a non-zero exit
> code which the "errexit" mode will then catch)). The idea in this case
> is that the shell (in "errexit" mode) produces a more descriptive error
> message than just exit (the same applies to "bash").
>
OK, I see. I just wonder, is not it regular error if script cannot
include some file? (So should not it fail in case that the file which
should be included is not there?)
But anyway, I can change it the way you propose and test it...
Petr
> ----
>
> Bye,
> Roland
>
>
