On Wed, Nov 14, 2007 at 01:04:50AM +0100, Roland Mainz wrote:

> > +all: $(VER)/makefile
> > +   (cd $(VER); env \
> > +       LD_OPTIONS="-M $(SRC)/cmd/mapfile_noexstk" \
> > +       MAKE=/usr/ccs/bin/make \
> > +       /usr/ccs/bin/make -e all3)
> > +   @find . -name core -exec rm -f {} \;
> > +
> 
> AFAIK it may be nice to add "-xstrconst" to CFLAGS and check whether the
> suggestions in
> http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6628305 make
> any difference for this application.

I was actually completely unable to get p7zip to build with Studio.  It's
mostly in C++, and is a horrendous pile that I don't want to start to try
to fix as long as it works.  So while I might investigate this, I think it
would be a good deal of work, and given that it works as-is, I'd like to
defer that investigation, or pass it off to someone who's more passionate
about it than I.

> > --- /dev/null   Tue Nov 13 12:49:36 2007
> > +++ new/usr/src/cmd/p7zip/install-p7zip Tue Nov 13 12:49:36 2007
> > @@ -0,0 +1,55 @@
> > +#!/bin/ksh
> 
> Please use "#!/usr/bin/ksh -p" or "#!/usr/bin/ksh93" (e.g. /bin -->
> /usr/bin/ and ksh88 needs "-p" (ksh93 doesn't need this anymore))

Okay.

> [sinp]
> > +# ident        "@(#)install-p7zip      1.1     07/11/13 SMI"
> 
> General note for this type of script: Please add a $ set -o errexit # at
> the beginning that the script - the "errexit" flag causes the shell to
> immediately abort the script when it hits a command which returns a
> failure instead of continuing operation (and maybe running amok (and
> ordering 12 refurbished F/A-18B via your EBay-account etc. =:-) ))

This violates the spirit of "make -k" which is the default -- do as much
work as is possible even if there are failures.  And if there are any
failures, none of the commands are dependent on previous ones having
succeeded, so it makes sense to keep on going.  Besides, if you've got a
weirdo environment that lets scripts order high-tech weaponry over ebay
without your explicit approval, well, that's your problem.  :)

> > +. ${SRC}/tools/install.subr
> 
> Plese change this to...
> -- snip --
> source "${SRC}/tools/install.subr"
> -- snip --

ksh(88) doesn't mention this in its manpage?

> > +
> > +cd ${VERS}
> > +
> > +for f in bin/7z bin/7za bin/7zr; do
> > +       _install E $f $BINDIR/${f#*/} 555
> 
> Please use quotes, e.g. change this to...
> -- snip --
> _install E "$f" "$BINDIR/${f#*/}" 555
> -- snip --

No.  There's zero reason to unless the shell is going crazy.  Which ksh
doesn't do, right?  :)

> ... AFAIK that's all what I can find in a 5min race...

Thanks!

Danek

Reply via email to