[PATCH RFC] aiaiai: don't allow arbitrary validator command

[Keller, Jacob E]

This patch depends on the other, but is part of the move we wanted to
make in the TODO. This may have to change if the implementation I used
for the variables in the config is not accepted. Again, this hasn't been
tested yet.

Thanks,
Jake

On Fri, 2014-02-28 at 16:01 -0800, Jacob Keller wrote:
> Since aiaiai-email-test-patchset is the only validator that makes sense,
> and we should extend it if new features are desired, we don't need to
> support multiple validators, as this is clunky, and difficult to
> configure for the user. Now that the configuration file supports all the
> options from the aiaiai-email-test-patchset, we can just directly call
> it instead of having to use a passed in parameter.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> ---
>  doc/TODO.txt                         |  6 ------
>  email/aiaiai-email-dispatcher        | 18 ++++++++++--------
>  email/aiaiai-email-dispatcher-helper | 26 +++++++++++++-------------
>  3 files changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/doc/TODO.txt b/doc/TODO.txt
> index 64bbbf7..da63cbb 100644
> --- a/doc/TODO.txt
> +++ b/doc/TODO.txt
> @@ -19,12 +19,6 @@ implementing them.
>      everythig via options. E.g., would could run
>      'aiaia-test-patchset --config <path>/aiaiai.cfg' and have all the
>      details defined in 'aiaiai.cfg'.
> -  * Additional item for the previous one: aiaiai-email-dispatcher is too
> -    difficutl to execute because it requires the "<validator>" argument.
> -    But there is only one validator which makes sense in Aiaiai context
> -    - "aiaiai-email-test-patchset". So remove the silly argument and
> -    just run the validator, without exposing unnecessary complexity to
> -    the user.
>    * Start assigning versions to the scripts and do official releases, with
>      signed tags, and tarballs.
>    * Provide packages for various distributions. Including packages for the
> diff --git a/email/aiaiai-email-dispatcher b/email/aiaiai-email-dispatcher
> index 9057fd3..72e3222 100755
> --- a/email/aiaiai-email-dispatcher
> +++ b/email/aiaiai-email-dispatcher
> @@ -26,17 +26,19 @@ fi
>  show_usage()
>  {
>  	cat <<-EOF
> -Usage: $PROG [options] <queuedir> <validator>
> +Usage: $PROG [options] <queuedir> <cfgfile.ini>
>  
>  This program dispatches the incoming queue of patches in the <queuedir>
> -directory and runs the <validator> program for each patch (or patch series).
> -Patches are supposed to be in mbox format.
> +directory and runs the aiaiai-email-test-patchset program for each patch (or
> +patch series). Patches are supposed to be in mbox format. <cfgfile.ini> is used
> +for general configuration of aiaiai-email-test-patchset.
>  
>  <queuedir>         - the directory containing the queue of patches
> -<validator>        - the validation command.
> +<cfgfile.ini>      - aiaiai-email-test-patchset's configuration file
>  
>  Options:
> -  -J, --bigjobs=N    how many validators can be run at the same time (default 1);
> +  -J, --bigjobs=N    how many patches (or patch series) can be checked
> +                     at the same time (default 1);
>    -v, --verbose      be verbose;
>    -h, --help         show this text and exit.
>  EOF
> @@ -129,7 +131,7 @@ program_required "inotifywait" ""
>  
>  mkdir $verbose -p -- "$1" 1>&2
>  queuedir="$(readlink -fv -- "$1")"; shift
> -validator="$1"; shift
> +cfgfile="$1"; shift
>  
>  in_fifo="$(mktemp -dt "$PROG.in_fifo.XXXX")"
>  tmpdir="$(mktemp -dt "$PROG.tmpdir.XXXX")"
> @@ -142,10 +144,10 @@ mkfifo -- "$fifo"
>  # -P option of xargs provides us the parallelism.
>  #
>  # We execute the below command as a separate process - it will run the
> -# validator for every file name read from the fifo.
> +# aiaiai-email-test-patchset for every file name read from the fifo.
>  tail -f -- "$fifo" | xargs -I{} -P"$bigjobs" -- \
>  	aiaiai-email-dispatcher-helper $verbose -- "$in_fifo/{}" "$tmpdir/STOP" \
> -				      "$queuedir" "$validator" >&2 &
> +				      "$queuedir" "$cfgfile" >&2 &
>  
>  # Loop forever, wait for new files in the queue directory using inotify and
>  # validate them
> diff --git a/email/aiaiai-email-dispatcher-helper b/email/aiaiai-email-dispatcher-helper
> index 63c56e1..998c6ac 100755
> --- a/email/aiaiai-email-dispatcher-helper
> +++ b/email/aiaiai-email-dispatcher-helper
> @@ -24,22 +24,22 @@ fi
>  show_usage()
>  {
>  	cat <<-EOF
> -Usage: $PROG [options] <mbox> <fail_file> <queuedir> <validator>
> +Usage: $PROG [options] <mbox> <fail_file> <queuedir> <cfgfile.ini>
>  
> -This is a helper program which runs the <validator> program for the
> -<mbox> file. If the validator returns success - just exit. Otherwise
> -it moves the <mbox> file back to the <queuedir> directory and create
> +This is a helper program which runs the aiaiai-email-test-patchset program for
> +the <mbox> file. If aiaiai-email-test-patchset returns success - just exit.
> +Otherwise it moves the <mbox> file back to the <queuedir> directory and create
>  file <fail_file>.
>  
> -The validator program should return failure only in case of internal
> -bug or error, and not if any test on the patch <mbox> failed. So the
> -reason we move <mbox> back to the queue is to make sure we do not
> -lose the patch in case of an internal bug.
> +aiaiai-email-test-patchset will return failure only in case of internal bug or
> +error, and not if any test on the patch <mbox> failed. So the reason we move
> +<mbox> back to the queue is to make sure we do not lose the patch in case of an
> +internal bug.
>  
> -<mbox>             - the mbox file to feed to the <validator>
> +<mbox>             - the mbox file to feed to aiaiai-email-test-patchset
>  <fail_file>        - the file to create in case of failure
>  <queuedir>         - the directory containing the queue of patches
> -<validator>        - the validation command.
> +<cfgfile.ini>      - aiaiai-email-test-patchset's configuration
>  
>  Options:
>    -v, --verbose      be verbose;
> @@ -88,17 +88,17 @@ done
>  mbox="$(readlink -fv -- "$1")"; shift
>  fail_file="$1"; shift
>  queuedir="$(readlink -fv -- "$1")"; shift
> -validator="$1"; shift
> +cfgfile="$1"; shift
>  
>  tmpdir="$(mktemp -dt "$PROG.XXXX")"
>  
>  verbose "Validating \"$mbox\", queue directory is: $queuedir"
> -verbose "Validator command: $validator"
> +verbose "Configuration file: $cfgfile"
>  
>  mv $verbose -- "$mbox" "$tmpdir" >&2
>  file="$tmpdir/${mbox##*/}"
>  
> -if ! cat -- "$file" | eval "$validator"; then
> +if ! cat -- "$file" | aiaia-email-test-patchset "$verbose" "$cfgfile"; then
>  	verbose "Validation failed"
>  	touch -- "$fail_file"
>  	mv $verbose -- "$file" "$queuedir" >&2