Martina Tomisova wrote:
> I've prepared changes for including ngrep into Nevada. There is a webrev so 
> you can check it. Any comments are welcomed. PSARC is not approved yet.
> 
> http://cr.opensolaris.org/~martina/ngrep/

Quick 5min race through
http://cr.opensolaris.org/~martina/ngrep/ngrep.patch (patch code is
quoted with "> "):
> --- /dev/null   Tue Jul  1 06:05:52 2008
> +++ new/usr/src/cmd/ngrep/Makefile.sfw  Tue Jul  1 06:05:52 2008
> @@ -0,0 +1,67 @@
[snip]
> +
> +VER=ngrep-1.45
> +
> +include ../Makefile.cmd
> +
> +CFLAGS += -DSOLARIS 

Please add $(XPG6MODE) $(XSTRCONST) before -DSOLARIS 

[snip]
> +all: $(VER)/config.status
> +       (cd $(VER); env \

Please use "env -" to make sure the environment is cleared and only pass
environment variables which are really needed, e.g. that "gmake"+build
do not pick-up random env values from the build environment (which is
right now one of the top5 reasons why SFWNV may fail to build in some
cases).

> +           CC=$(CC) CXX=$(CCC) \
> +           "CFLAGS=$(CFLAGS)" \
> +           PATH=$(SFW_PATH) \
> +           MAKE=$(GMAKE) \
> +           $(GMAKE) -e)
> +
> +install: all
> +       (DESTDIR=$(ROOT) $(SH) ./install.sfw)
> +
> +$(VER)/config.status: $(VER)/configure
> +       (cd $(VER); env \

Please use "env -" to make sure the environment is cleared and only pass
environment variables which are really needed, e.g. that "configure"
doesn't pick-up random env values from the build environment (which is
right now one of the top5 reasons why SFWNV may fail to build in some
cases).

> +           CC=$(CC) CXX=$(CCC) \
> +           "CFLAGS=$(CFLAGS)" \
> +           PATH=$(SFW_PATH) \
> +           MAKE=$(GMAKE) \
> +           ./configure $(CONFIGURE_OPTIONS))
[snip]
> --- /dev/null   Tue Jul  1 06:05:56 2008
> +++ new/usr/src/cmd/ngrep/install.sfw   Tue Jul  1 06:05:55 2008
[snip]

Plese add "set -o errexit" and use either "ksh93" or "bash" to make sure
the script immediately stops if an error occurs.

> +VERS=ngrep-1.45
> +
> +PREFIX=${ROOT}/usr
> +BINDIR=${PREFIX}/bin
> +SHAREDIR=${PREFIX}/share
> +MAN1mDIR=${SHAREDIR}/man/man1m
> +
> +. ${SRC}/tools/install.subr

Please use "source" instead of "." (see
http://www.opensolaris.org/os/project/shell/shellstyle/#use_source_not_dot).

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.mainz at nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL <currently fluctuating>
 (;O/ \/ \O;)

Reply via email to