[m-rev.] for review: Add build_src_dist script for building the source distribution

Julien Fischer jfischer at opturion.com
Sat Oct 19 03:49:46 AEDT 2013


Hi,

On Wed, 16 Oct 2013, Paul Bone wrote:

> Branches: master
>
> For review by Julien as he wrote the original script.

If by "wrote", you mean cut-and-pasted the relevant bits of the
test_mercury script, then yes I did ...

> Add build_src_dist script for building the source distribution
>
> This new script can build source distributions.  It is derived from the
> script Julien had been using.
>
> tools/build_src_dist:
>    As above

I think the script should be called build_srcdist (i.e. no underscore
between "src" and "dist"), since that's the form in which "srcdist"
appears in the package names.

> ---
> tools/build_src_dist | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100755 tools/build_src_dist
>
> diff --git a/tools/build_src_dist b/tools/build_src_dist
> new file mode 100755
> index 0000000..f866ead
> --- /dev/null
> +++ b/tools/build_src_dist
> @@ -0,0 +1,92 @@
> +#!/bin/sh
> +# Copyright (C) 2013 The University of Melbourne
> +#
> +# This script builds the Mercury source distribution.
> +#
> +
> +set -e
> +
> +DATE=`date '+%Y-%m-%d'`
> +ROTD_VERSION=rotd-$DATE
> +CC=gcc
> +RUN=no
> +PARALLEL=1
> +
> +USAGE="$0 < -r | -v VER | -n > [-j PAR] [-f] [-c CC]

This usage message is unclear; I suggest you follow the style used in
usage message for the bootcheck script.

> +
> +\t-r     Use a ROTD style version number with today's date.
> +\t-v VER Use VER as the version number.
> +\t-n     Don't update VERSION file (default).

-r should be the default IMO.

> +\t-f     Without this option the build_src_dist will not run, it will
> +\t       warn the user that it is distructive and then exit.

s/distructive/destructive/

What is '-f' supposed to stand for?  I don't think it's neccessary, the
script is only as destructive as doing an mmake realclean on your
workspace.


> +\t-j PAR Run this many jobs in parallel.
> +\t-c CC  Use CC as the C compiler.

Where are you intending to run this from?  It would be better to allow
for an optional argument to the script that specifies the directory
contain the workspace to build from.  In the absence of that argument
you can try to build from the current working directory.

> +"
> +
> +while : ; do

It would be more usual to do:

     while test $# -gt 0; do

(or just use getopts, if you don't care about compatibility with really
old shells)

> +    case "$1" in
> +        -h | --help)
> +            echo "$USAGE"
> +            exit 0
> +            ;;
> +        -r)
> +            RELEASE_VERSION=$ROTD_VERSION
> +            ;;
> +        -v)
> +            shift
> +            RELEASE_VERSION=$1
> +            ;;

I suggest doing this the other way around, e.g.

      RELEASE_VERSION=$2
      shift
      ;;
> +        -n)
> +            RELEASE_VERSION=""
> +            ;;
> +        -c)
> +            shift
> +            CC=$1
> +            ;;
> +        -f)
> +            RUN=yes
> +            ;;
> +        -j)
> +            shift
> +            PARALLEL=$1
> +            ;;


You should also handle the case where there is no whitespace between -j
and its argument, i.e. where $1 matches -j*.

> +        *)
> +            if [ -z "$1" ]; then
> +                break
> +            else
> +                echo "Unknown option \"$1\""
> +                echo "$USAGE"
> +                exit 1
> +            fi
> +            ;;
> +    esac
> +    shift
> +done
> +
> +if [ "$RUN" = "no" ]; then
> +    echo "Warning: Thie script is disruptive, it will 'git clean' your"

s/Thie/This/

> +    echo "workspace.  If you're okay with this and want to proceed, then"
> +    echo "add -f to the command line"
> +    exit 2
> +fi
> +
> +NUM_TAG_BITS=2
> +BITS_PER_WORD=32
> +UNBOXED_FLOATS=no
> +
> +# git checkout -- VERSION
> +git clean -d -f -x

You should probably abort in the absence of a .git directory.
(Eventually we should allow builds from a non-git source tree, in which
case would need to do an mmake realclean instead of git clean.)

> +if [ -n "$RELEASE_VERSION" ]; then
> +    sed "s/VERSION=.*/VERSION=$RELEASE_VERSION/" VERSION > VERSION.new
> +    mv VERSION.new VERSION
> +fi
> +/bin/rm -f Mmake.params Mmake.stage.params
> +aclocal -I m4 &&
> +autoconf &&
> +mercury_cv_low_tag_bits=$NUM_TAG_BITS \
> +mercury_cv_bits_per_word=$BITS_PER_WORD \
> +mercury_cv_unboxed_floats=$UNBOXED_FLOATS \
> +sh configure --with-llds-base-grade=none --with-cc="$CC" &&
> +mmake GRADE=hlc.gc.pregen MMAKEFLAGS="EXTRA_MCFLAGS='-O5 --opt-space --cross-compiling --no-smart-indexing' -j$PARALLEL" tar

Cheers,
Julien.



More information about the reviews mailing list