Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Laszlo Ersek <lersek@redhat.com>]

comments below

On 05/31/13 18:39, Jeff Cody wrote:

> +usage() {
> +    echo ""
> +    echo "$0 [OPTIONS]"
> +    echo "$desc"
> +    echo ""
> +    echo "OPTIONS:"
> +    echo "      -r git range
> +            optional; default is '$range'
> +                    "
> +    echo "      -c configure options
> +            optional; default is '$config_opt'
> +                    "
> +    echo "      -m make options
> +            optional; default is '$make_opt'
> +                    "
> +    echo "      -d log dir
> +            optional; default is '$logdir'
> +                    "
> +    echo "      -l log filename
> +            optional; default is output-PROCID, where PROCID is the bash process id
> +            note: you may specify a full path for the log filename here, and exclude the
> +            -d option.
> +                    "
> +    echo "      -f force a git reset and clean
> +            this will cause a 'git reset --hard; git clean -fdx' to be run between checkouts.
> +            !! WARNING: This may cause data loss in your git tree.
> +                        READ THE git-clean and git-reset man pages and make
> +                        sure you understand the implications of
> +                        'git clean -fdx' and 'git reset --hard' before using !!
> +                    "
> +    echo "      -h help"
> +}

Sorry for not noticing this before: is there any reason for the trailing
spaces on most lines in the usage text?

Plus I suggest lower-casing the "THE" in "READ THE".

> +
> +while getopts ":r:c:m:l:d:hf" opt
> +do
> +    case $opt in
> +        r) range=$OPTARG
> +            ;;
> +        c) config_opt=$OPTARG
> +            ;;
> +        m) make_opt=$OPTARG
> +            ;;
> +        d) logdir=$OPTARG
> +            ;;
> +        l) logfile=$OPTARG
> +            ;;
> +        f) force_clean='y'
> +            ;;
> +        h) usage
> +           exit
> +            ;;
> +        \?) echo "Unknown option: -$OPTARG" >&2
> +            usage
> +            exit 1
> +            ;;
> +    esac
> +done
> +
> +# append a '/' to logdir if $logdir was specified without one
> +[[ -n "$logdir" ]] && [[ ${logdir:${#logdir}-1} != "/" ]] && logdir="${logdir}/"
> +
> +logfile="${logdir}${logfile}"
> +
> +head=`git rev-parse HEAD`
> +total=`git rev-list "$range" |wc -l`
> +
> +echo "log output: $logfile"
> +
> +rm -f "$logfile"

rm -f -- "$logfile" is safer, but I doubt anyone would pass a pathname
starting with "-"...

> +date > "$logfile"
> +echo "git compile check for $range." >> "$logfile"
> +echo "* configure options='$config_opt'" >> "$logfile"
> +echo "* make options='$make_opt'" >> "$logfile"
> +echo "Performing a test compile on $total patches" | tee -a "$logfile"
> +echo "-------------------------------------------------------------" >> "$logfile"
> +echo "" | tee -a "$logfile"
> +
> +clean_repo() {
> +    if [[ $force_clean == 'y' ]]
> +    then
> +        git reset --hard >> "$logfile" 2>&1 || true
> +        git clean -fdx -e "$logfile" >> "$logfile" 2>&1 || true
> +    fi
> +}

Does "-e" mean "except"? It's not supported on RHEL-6.

> +
> +# we want to cleanup and return the git tree back to the previous head
> +trap cleanup EXIT
> +
> +cleanup() {
> +    echo ""
> +    echo -n "Cleaning up..."
> +    clean_repo
> +    git checkout $head > /dev/null 2>&1
> +    echo "done."
> +}
> +
> +cnt=1
> +# don't pipe the git job into read, to avoid subshells
> +while read hash
> +do
> +    txt=`git log --pretty=tformat:"%h: %s" $hash^!`
> +    echo "${cnt}/${total}: compiling: $txt" | tee -a "$logfile"
> +    let cnt=$cnt+1;
> +    echo "####################" >> "$logfile"
> +    clean_repo
> +    make clean > /dev/null 2>&1 || true
> +    git checkout $hash          >> "$logfile" 2>&1 && \
> +        ./configure $config_opt >> "$logfile" 2>&1 && \
> +        make $make_opt          >> "$logfile" 2>&1 ||
> +    (
> +        echo "" | tee -a "$logfile"
> +        echo "ERROR: commit $hash failed to build!" | tee -a "$logfile"
> +        git show --stat $hash | tee -a "$logfile"
> +        exit 1
> +    )
> +done < <(git log $range --pretty=tformat:"%H" --reverse)
> +
> +echo "
> +All patches in $range compiled successfully!" | tee -a "$logfile"
> +exit 0
> 

I think my remarks are not important, the script should work in any
practical environment. We should get this merged and small fixes can be
posted incrementally if needed.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>