[m-rev.] for review: use xargs in mmake

Fergus Henderson fjh at cs.mu.OZ.AU
Sun Feb 17 18:23:43 AEDT 2002


On 15-Feb-2002, Peter Ross <peter.ross at miscrit.be> wrote:
> Index: compiler/modules.m
> +		"\t-echo $(", MakeVarName, ".dirs) | xargs rm -rf \n",
> +		"\t-echo $(", MakeVarName, ".cs) ", InitCFileName,
> +				" | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_ss) ", InitAsmFileName,
> +				" | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_pic_ss) ",
> +					InitAsmFileName, " | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_os) ", InitObjFileName,
> +				" | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_pic_os) ",
> +					InitPicObjFileName, " | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".c_dates) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".il_dates) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_s_dates) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".all_pic_s_dates) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".useds) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".ils) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".profs) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".errs) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".foreign_cs) | xargs rm -f\n",
> +		"\t-echo $(", MakeVarName, ".schemas) | xargs rm -f\n"

Rather than invoking `xargs rm -f' multiple times,
you should invoke it just once.

	{ echo this; \
	  echo that; \
	  echo the other; \
	} | xargs rm -f

Of course that won't solve the problem of exceeding command
line length limits when `make' invokes `sh'.

Maybe you could solve that problem by having `mmc --generate-dependencies foo'
also generate a file `foo.clean_files', and then have the pattern rule

	%.clean: %.clean_files
		xargs rm -f < $<

Likewise for the `realclean' target.

> +++ scripts/mmake.in	15 Feb 2002 09:23:20 -0000
> @@ -287,7 +287,7 @@
>  	echo MERCURY_DEFAULT_GRADE=$MERCURY_DEFAULT_GRADE
>  	echo export MERCURY_DEFAULT_GRADE
>  	echo cat ${MMAKE_VARS} $dvs $ds $include_makefile $mmake $deps \
> -		${MMAKE_RULES} ">>" $tmp
> +		${MMAKE_RULES}">>" $tmp
>  	echo ${MMAKE_MAKE} ${MMAKE_MAKE_OPTS} -f $tmp -r ${set_target_asm} "$@"
>  fi
>  export MMAKE

That should be undone; it just makes the code harder to read.

> @@ -295,7 +295,17 @@
>  export MMAKE_USE_SUBDIRS
>  export MERCURY_INT_DIR
>  export MERCURY_DEFAULT_GRADE
> -cat ${MMAKE_VARS} $dvs $ds $include_makefile $mmake $deps ${MMAKE_RULES} > $tmp
> +# XXX The $dvs and $ds variables can be so long as to overflow the
> +# command line size limits, so we use xargs.  However echo doesn't quote
> +# the file names correctly, but this is not a problem in practice because
> +# the way the file names are constructed no special characters are
> +# included currently.  If fixed, the fix needs to be replicated in
> +# `modules.m'.
> +{
> +cat ${MMAKE_VARS}
> +echo $dvs $ds | xargs cat
> +cat $include_makefile $mmake $deps ${MMAKE_RULES}
> +} > $tmp

s/However/Unfortunately/
(it just reads better that way)

s/the file names/the $dvs and $ds file names/
because what the comment says is not true of the other file names.

Indeed, there should be double-quotes around ${MMAKE_VARS} and ${MMAKE_RULES},
to avoid problems in case those contains spaces.
(This is an existing problem in the script, but if you're adding comments
about the quoting in the code, might as well get it right for "cat"
as well as for "echo".)

Also you should document why using "echo ... | xargs" doesn't
overflow the command-line limits for "echo" (the reason is
because on the systems which have low command-line limits,
i.e. Windows, we're using bash, and "echo" is a built-in
command in bash, so the command-line limits don't apply).

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  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