Mark Fenwick wrote:
> 
> This is a request for code review for:
> 
> 6712365 Intergration of stunnel into Solaris
> 
> http://cr.opensolaris.org/~markfen/stunnel/webrev/
[snip]

5min race through
http://cr.opensolaris.org/~markfen/stunnel/webrev/stunnel.patch (patch
code is quoted with "> "):
> --- /dev/null Mon Jun  9 13:18:34 2008
> +++ new/usr/src/cmd/stunnel/Makefile.sfw      Mon Jun  9 13:18:34 2008
>@@
[snip]
> +all: $(VER)/config.status
> +     (cd $(VER); env \

Please change this to "env - ..." to make sure the environment is
completely cleared and then only pass the environment variables which
are needed. I'm currently preparing a tree-wide patch for that to get
rid of the sporadic build failures in SFWNV which are caused by
unexpected environment variables "bleeding through" into "configure"&co.

> +         PATH=$(SFW_PATH) \
> +         MAKE=$(CCSMAKE) $(CCSMAKE))
> +
> +install: all
> +     $(SH) ./install-sfw

See below...

> +
> +$(VER)/config.status: $(VER)/configure
> +     (cd $(VER); env \

Please use $ env - ... # - see above...

> +         CC=$(CC) \
> +         "LDFLAGS=-L/usr/sfw/lib -R/usr/sfw/lib" \
> +         PATH=$(SFW_PATH) \
> +         MAKE=$(CCSMAKE) \
> +         ./configure --prefix=/usr --with-ssl=/usr/sfw \
> +         --libdir=/usr/sfw/lib --sysconfdir=/etc)
> +     (cd $(VER)/src; \
> +         gpatch -p0 <../../libwrap.c_patch)

Erm... why are you patching the code after "configure" ?


> --- /dev/null Mon Jun  9 13:18:34 2008
> +++ new/usr/src/cmd/stunnel/install-sfw       Mon Jun  9 13:18:34 2008
> @@ -0,0 +1,50 @@
> +#!/bin/sh -e

Erm... the "-e" is useless since you call the script via "sh
install-sfw" and the interpreter only sees this as comment line...

> +#
> +# CDDL HEADER START
> +#
> +# The contents of this file are subject to the terms of the
> +# Common Development and Distribution License (the "License").
> +# You may not use this file except in compliance with the License.
> +#
> +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
> +# or http://www.opensolaris.org/os/licensing.
> +# See the License for the specific language governing permissions
> +# and limitations under the License.
> +#
> +# When distributing Covered Code, include this CDDL HEADER in each
> +# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
> +# If applicable, add the following below this CDDL HEADER, with the
> +# fields enclosed by brackets "[]" replaced with your own identifying
> +# information: Portions Copyright [yyyy] [name of copyright owner]
> +#
> +# CDDL HEADER END
> +#
> +#
> +# Copyright 2008 Sun Microsystems, Inc.   All rights reserved.
> +# Use is subject to license terms.
> +#
> +# install objects in the proto area, since it would
> +# really like to install locally, but that doesn't scale.
> +#
> +#ident       "%Z%%M% %I%     %E% SMI"

AFAIK it may be nice to use /usr/bin/ksh93 (or "bash" if it supports the
"errexit" flag) for this script and then do 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 666 new A350 planes via your Ebay-account etc. =:-) ))

> +
> +VERS=4.21
> +PKGVERS=stunnel-${VERS}
> +PREFIX=${ROOT}/usr
> +SFWPREFIX=${ROOT}/usr/sfw
> +XMLDIR=${ROOT}/var/svc/manifest/network
> +XMLFILE=stunnel.xml
> +BINDIR=${PREFIX}/bin
> +SHAREDIR=${PREFIX}/share
> +MAN8DIR=${SHAREDIR}/man/man8
> +
> +. ${SRC}/tools/install.subr

Please use ...
-- snip --
source "${SRC}/tools/install.subr"
-- snip --
(see
http://opensolaris.org/os/project/shell/shellstyle/#use_source_not_dot)

> +
> +_install N ${XMLFILE} ${XMLDIR}/${XMLFILE} 444
> +_install N stunnel.sample ${ROOT}/etc/stunnel/stunnel.sample 644
> +
> +cd ${PKGVERS}
> +_install N doc/stunnel.8 ${MAN8DIR}/stunnel.8 444
> +_install E src/stunnel ${BINDIR}/stunnel 555
> +
> +exit 0

AFAIK the remaining stuff looks Ok...

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.mainz at nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 7950090
 (;O/ \/ \O;)

Reply via email to