Vita Batrla wrote:
>
> Hello all,
>
> I prepared a small change for nightly(1), adding a new option:
>
> -s suppress noise differences since last build
>
> The webrev:
> http://cr.opensolaris.org/~vita78/nightly_6680889/
Quick 5min race through the diff:
-------- snip --------
> --- old/usr/src/tools/scripts/nightly.sh Thu Apr 30 13:13:21 2009
> +++ new/usr/src/tools/scripts/nightly.sh Thu Apr 30 13:13:21 2009
> @@ -673,9 +673,12 @@
> if [ ! -f $SRC/${NOISE}.ref ]; then
> cp $SRC/${NOISE}.out $SRC/${NOISE}.ref
> fi
> - echo "\n==== Build noise differences ($LABEL) ====\n" \
> - >>$mail_msg_file
> - diff $SRC/${NOISE}.ref $SRC/${NOISE}.out >>$mail_msg_file
> + if [ "$s_FLAG" = "n" ]; then
Please change this to use the "[[ ]]" test operator instead of the "[ ]"
test opeartor (that saves at least a pattern matching pass).
> + echo "\n==== Build noise differences ($LABEL) ====\n"
> \
> + >> $mail_msg_file
Erm... I know... moved code... but is it possible to use "printf"
instead, e.g.
-- snip --
printf "\n==== Build noise differences (%s) ====\n\n" "$LABEL" >>
$mail_msg_file
-- snip --
That's more secure than "echo".
> + diff $SRC/${NOISE}.ref $SRC/${NOISE}.out \
> + >> $mail_msg_file
> + fi
> fi
>
> #
> @@ -837,10 +840,12 @@
> # above
> egrep -i '(warning|lint):' ${LINTNOISE}.out \
> | sort | uniq >> $mail_msg_file
> - echo "\n==== lint noise differences $base ====\n" \
> - >> $mail_msg_file
> - diff ${LINTNOISE}.ref ${LINTNOISE}.out \
> - >> $mail_msg_file
> + if [ "$s_FLAG" = "n" ]; then
Please change this to use the "[[ ]]" test operator instead of the "[ ]"
test opeartor.
> + echo "\n==== lint noise differences $base ====\n" \
> + >> $mail_msg_file
"printf", please...
> + diff ${LINTNOISE}.ref ${LINTNOISE}.out \
> + >> $mail_msg_file
> + fi
> fi
> }
>
> @@ -1128,6 +1133,7 @@
> -p create packages
> -r check ELF runtime attributes in the proto area
> -t build and use the tools in $SRC/tools
> + -s suppress noise differences since last build
> -u update proto_list_$MACH and friends in the parent workspace;
> when used with -f, also build an unrefmaster.out in the parent
> -w report on differences between previous and current proto areas
> @@ -1165,6 +1171,7 @@
> P_FLAG=n
> p_FLAG=n
> r_FLAG=n
> +s_FLAG=n
> T_FLAG=n
> t_FLAG=y
> U_FLAG=n
> @@ -1377,7 +1384,7 @@
> #
> NIGHTLY_OPTIONS=-${NIGHTLY_OPTIONS#-}
> OPTIND=1
> -while getopts AaBCDdFfGIilMmNnOoPpRrS:TtUuWwXxz FLAG $NIGHTLY_OPTIONS
> +while getopts AaBCDdFfGIilMmNnOoPpRrS:sTtUuWwXxz FLAG $NIGHTLY_OPTIONS
> do
> case $FLAG in
> A ) A_FLAG=y
> @@ -1430,6 +1437,8 @@
> S )
> set_S_flag $OPTARG
> ;;
> + s ) s_FLAG=y
> + ;;
You need to test for "+s", too, e.g. the case item should be:
-- snip --
s ) s_FLAG=y
;;
+s ) s_FLAG=n
;;
-- snip --
(since boolean flags can be turned-on and -off in one command line
argument sequence).
-------- snip --------
The remaining patch looks Ok except the repeated "[[ ]] vs. [ ]" and
"echo vs. printf" stuff described above.
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.ma...@nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
_______________________________________________
tools-discuss mailing list
tools-discuss@opensolaris.org
