[151215] contrib/mp-buildbot

[Clemens Lang]

Hi Mojca,

a couple of review comments inline below.

On Wed, Aug 10, 2016 at 11:09:30AM -0700, mojca at macports.org wrote:
> Revision: 151215
>           https://trac.macports.org/changeset/151215
> Author:   mojca at macports.org
> Date:     2016-08-10 11:09:29 -0700 (Wed, 10 Aug 2016)
> Log Message:
> -----------
> mp-buildbot: create a log file with progress of installed dependencies
> 
> +    # prepare the log file
> +    if ! [[ -d "${option_logdir}" ]] ; then
> +        mkdir -p "${option_logdir}"
> +    fi

The double brackets aren't necessary here. Additionally, testing and
then creating a directory has an implicit race condition (somebody else
could create it in between), so it's good that you use -p here, but then
you could just do mkdir -p directly and avoid the if completely.

> +    log_status_dependencies=${option_logdir}/dependencies-progress.txt
> +    # make ure to start with an empty file

Typo, should be make *s*ure.

> +    echo -n "" > $log_status_dependencies

Instead of explicitly printing nothing into a file, you can just run
 > "$log_status_dependencies"
which does the same thing. Note that you should quote the redirect
target, just in case $option_logdir contains spaces. This applies to all
other uses of $log_status_dependencies as well.

> +    echo "" >> $log_status_dependencies

echo "" is the same as just using echo without argument.

> +        text="Installing dependency ($dependencies_counter of $dependencies_count) '${depname}', variants: '${depvariants}'"
> +        echo "----> ${text}"
> +        echo -n "${text}' ... " >> $log_status_dependencies

You have an additional closing single quote after ${text} here, which
leads to
 Installing dependency (41 of 205) 'xorg-xcb-proto', variants: '+python27'' ...  [OK]


Thanks for the work on this, though; it's much easier to read!

-- 
Clemens