From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ukxtj-0000At-FH for qemu-devel@nongnu.org; Fri, 07 Jun 2013 10:44:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ukxte-0002jA-F4 for qemu-devel@nongnu.org; Fri, 07 Jun 2013 10:44:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8701) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ukxte-0002iJ-3S for qemu-devel@nongnu.org; Fri, 07 Jun 2013 10:44:06 -0400 Date: Fri, 7 Jun 2013 10:44:00 -0400 From: Jeff Cody Message-ID: <20130607144400.GC409@localhost.localdomain> References: <51B04F31.4040908@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51B04F31.4040908@redhat.com> Subject: Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: kwolf@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, alex.williamson@redhat.com, stefanha@redhat.com On Thu, Jun 06, 2013 at 10:58:25AM +0200, Laszlo Ersek wrote: > 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? > No, not particularly, mainly just formatting with an extra newline, and trying to keep the code somewhat lined up. > Plus I suggest lower-casing the "THE" in "READ THE". > I agree. > > + > > +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 "-"... You are right, and no reason not to use '--', so I should add that in. > > > +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. > Yes - this way it will preserve the log output (the default log path is in the current working directory). Looks like the -e option was not introduced until 1.7.3, and RHEL-6 is on 1.7.1. I'll add a check for the git version, and drop the -e if git is too old to support it (and in that case, the user will need to make sure if they supply the -f option to this script that they specify a different log location than the cwd). > > + > > +# 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 > Thanks. I can either do the above changes for a v2, or as follow on patches.