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

[Paul Bone]

On Sat, Oct 19, 2013 at 03:49:46AM +1100, Julien Fischer wrote:
>
> 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.

Okay.

>> ---
>> 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.

Okay.

>> +
>> +\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.
>

Okay, I can see that that's how it would be used most often.

> 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.

No it's more destructive than that.  It will do git clean -fdx which means
that if you have some extra file like "compiler/par_conj.m.experiment"
sitting in your workspace it will destroy it.  Yes, it it reasonable for
people to run this in their workspaces as I needed to this week.


>> +\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.

I'm using the current working directory.  If anyone wants a script to build
sourcedists from git HEAD then they can write a new script that does:

    $ git clone URL new_checkout
    $ cd new_checkout
    $ ./tools/build_srcdist ...

I'm not keen on build_srcdist being copied outside of a workspace and
executed there.  That means that you have multiple copies of the same script
to keep in sync.

>> +"
>> +
>> +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)

It makes sense to use getopts.

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

getopts seems to handle this even though bash(1) says it does not.  Anyway,
if I choose getopts then I'm at it's mercy.

>> +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.)
>

Okay.

One thing I want to add (later) is that I'd like to get the hash ID of the
current git version and put it inside the tarball.  Or tag the git
repository (and push that tag up to github) with the version that's being
built.  That way, we can use this information to awnser things like "Is my
change to blah in version foo?".

Thanks.

-- 
Paul Bone
http://www.bone.id.au