[m-rev.] for review: cvdd

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Apr 27 20:27:33 AEST 2001


On 27-Apr-2001, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> For review by anyone. Also, if anyone who wants to extend the script to handle
> other subdirectories (e.g. bindist) is welcome to do so.
> 
> The references to the deep subdirectory will be committed on the main branch
> only after deep profiling itself is committed.
> 
> tools/cvdd:
> 	A script for performing a diff between the main directories of
> 	two Mercury workspaces. It is meant to be useful if you are not
> 	connected to a network and therefore unable to "cvs add" files;
> 	in such circumstances, doing a "cvdd clean_ws work_ws" can be
> 	a sufficiently good substitute for a "cvd diff work_ws".
> 
> Zoltan.
> 
> #!/bin/sh
> if test $# -ne 2
> then
> 	echo "usage: cvdd dir1 dir2"
> 	exit 1
> fi

There should be a comment at the top of the script explaining what it is for.

> exclude_doc="
> library-menu*
> library-chapters*
> mercury_library.info*
> *.html
> *.1
> *.aux
> *.toc
> *.log
> *.info
> *.info-8
> "

Is that supposed to be *.info-* rather than *.info-8?

> [A-Z]*.m
...
> [A-Z]*.c
> [A-Z]*.h

Why are they excluded?

If this is related to a Zoltan-specific work habit ;-),
then maybe it would be better to put those in ~zs/.cvddrc
and have the script add the patterns in $HOME/.cvddrc
to the list of file name patterns to exclude.

> for d in browser compiler deep doc library runtime scripts trace tools util

Another alternative would be

	for d in `find . -type d -maxdepth 1`

I think that would be enough handle other directories such as bindist.

> do
> 	case $d in
> 	browser|compiler|deep|library)
> 		echo "$exclude_all $exclude_m" | sed -e '/^ *$/d' > $excludefile
> 		kind="m" ;;

It would be better to use a more expressive name than "m" here,
e.g. kind="mercury".

> 	runtime|trace|util)
> 		echo "$exclude_all $exclude_c" | sed -e '/^ *$/d' > $excludefile
> 		kind="c" ;;
> 	doc)
> 		echo "$exclude_all $exclude_doc" | sed -e '/^ *$/d' > $excludefile
> 		kind="d" ;;
>
> 	*)
> 		echo "$exclude_all" | sed -e '/^ *$/d' > $excludefile
> 		kind="o" ;;

Likewise for "d" and "o".

> 	exclude_opt="--exclude-from=$excludefile"
> 	diff -rubB $exclude_opt "$1"/$d "$2"/$d | egrep -v '^Binary' > $rawdiff

It would be nice to allow "cvdd [diff-options] dir1 dir2"
rather than hard-coding `-ubB' here.

> 	cat > $script << EOF
> BEGIN		{ kind = "$kind"; }
> \$1 == "Only"	{
> 			if (\$4 ~ /^\./ || \$4 ~ /^[xyz]/) {
> 				next;
> 			}

I think `.*' is already covered above anyway, isn't it?
And excluding [xyz]* seems like it is a bit too likely to get false misses.

> 			if (\$4 == "rl_file.m" || \$4 == "rl_out.m") {
> 				next;
> 			}

It would be nicer to handle that by including those file names
in the lists to exclude earlier.

Otherwise that looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list