Venky wrote:
>
> I'm porting Privoxy[1] to SFW. Would appreciate a code review of my
> changes. This is my first SFW integration, so it is possible there
> are some beginner's mistakes.
>
> The webrev is here:
> http://cr.opensolaris.org/~venkytv/privoxy/
1. AFAIK these things may be nice (most of these item are very generic
things which are "usual suspects" for build problems in the tree):
- Plese use "env - ..." and not "env ..." in the Makefiles to make sure
"configure"&&"make" only see the env variables they should really get
(and not pick-up any random env variable)
- Please use either $(SHELL) or /usr/bin/bash for "configure" calls (so
we know which one is used and "configure" doesn't pick one itself)
- 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)
2. Non-generic issues:
> --- /dev/null Tue Aug 12 07:00:27 2008
> +++ new/usr/src/cmd/privoxy/Makefile.sfw Tue Aug 12 07:00:27 2008
> @@ -0,0 +1,69 @@
[snip]
> +$(VER)/privoxy: $(VER)/GNUmakefile
> + (cd $(VER); \
> + env \
Please use "env - " here...
> + PATH=$(SFW_PATH) \
Please add double-quotes around "PATH=$(SFW_PATH)".
> + LD_OPTIONS="-M $(SRC)/cmd/mapfile_noexstk" \
> + MAKE=$(GMAKE) \
Double-quote, please...
> + $(GMAKE))
> +
> +test:
> +
> +install: $(VER)/privoxy
> + VERS=$(VER) /usr/bin/ksh ./install-privoxy
[snip]
> --- /dev/null Tue Aug 12 07:00:31 2008
> +++ new/usr/src/cmd/privoxy/install-privoxy Tue Aug 12 07:00:31 2008
> @@ -0,0 +1,109 @@
> +#!/usr/bin/ksh -pe
- This line is a no-op since the script is called via $ <interpreter>
<scriptname> # and not $ ./<scriptname> #. IMO you want to add an
explicit $ set -o xtrace # directly under the license template
- Please use "function foo" instead of "foo()" (see
http://www.opensolaris.org/os/project/shell/shellstyle/#use_ksh_style_function_syntax)
- Please make sure the script doesn't generate errors with $ ksh93 -n
<scriptname> # (this is the same as
http://www.opensolaris.org/os/project/shell/shellstyle/#shell_lint)
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
