[PATCH 01/16] email-lda: use cover letter subject if available

[Keller, Jacob E]

On Thu, 2014-02-06 at 18:30 +0200, Artem Bityutskiy wrote:
> From: Jacob Keller <jacob.e.keller at intel.com>
> 
> Often, a patch series will be sent with a cover letter which describes the
> series and is given the '0/n' patch number. Rather than dropping these
> patches, keep track of them in series. We do this by checking whether the 1/n
> patch has an "In-Reply-To" header. This usually means that their will be an
> associated cover letter. Once this is found, rather than sending the patch
> directly on to the test program, we use the cover letter's subject by using
> formail to add a special "Series-Subject". This makes it so that the
> aiaiai-email will end up as a reply to the cover letter which is a bit more
> aesthetically pleasing to users.
> 
> To help this process, a new "series_is_complete" function is added in place of
> the original series complete check. This function reports whether the series
> is completely queued, for both cases of cover letter and no cover letter.
> 
> Artem:
> 
> I've modified this patch.
> 
> 1. Removed a couple of '[[' bash constructs
> 2. Also preserved the Message ID of the cover letter to make the reply not only
>    have the subject of the cover letter, but also refer the cover letter
>    properly.
> 3. Use common prefix for the special cover letter e-mail headers that we add:
>    X-Aiaiai-Cover-Letter-Subject
>    X-Aiaiai-Cover-Letter-Message-Id
> 4. Dropped sponge dependency. Generally, this is a nice tool, and I did not
>    hear about it before. I'll definitely start using it when constructing pipes
>    in the console. But this tool is not that well-known, so it is not typically
>    present in systems. E.g., mine do not have it. And I decided to avoid adding
>    another dependency, we already require a lot of tools. Instead, I used
>    another trick, which is actual just as elegant as using sponge. Well, needs
>    more lines of code, but it is really nice trick anyway, I like it.
> 
>    I just open the mbox file, then unlink it, which means it won't be visible
>    in the file-system, but won't be deleted since we have the file descriptor
>    opened. Then I pipe the data from the file descriptor to 'formail', and
>    redirect the results to the mbox file. So a new mbox file will be created
>    and all the new contents will be there. I think this is elegant, and does
>    not add a new dependency.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy at linux.intel.com>
> ---
>  email/aiaiai-email-lda           | 60 ++++++++++++++++++++++++++++++++++++----
>  email/aiaiai-email-test-patchset | 11 ++++++--
>  2 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/email/aiaiai-email-lda b/email/aiaiai-email-lda
> index 336bd09..3b102be 100755
> --- a/email/aiaiai-email-lda
> +++ b/email/aiaiai-email-lda
> @@ -153,6 +153,28 @@ find_relatives()
>  	fi
>  }
>  
> +series_is_complete()
> +{
> +	local dir="$1"; shift
> +	local n="$1" ; shift
> +
> +	# First check if we have all the non-cover patches yet
> +	if [ "$(ls -1 --ignore=0 -- "$dir" | wc -l)" -eq "$n" ]; then
> +		local first_parent="$(fetch_header "In-Reply-To" < "$dir/1")"
> +		if [ -n "$first_parent" ] && ! [ -f "$dir/0" ]; then
> +			# Series is not complete, we are missing the cover
> +			# letter
> +			return 1
> +		else
> +			# Series is complete, no cover letter was sent
> +			return 0
> +		fi
> +	else
> +		# Don't have all the patches yet
> +		return 1
> +	fi
> +}
> +
>  move_to_series()
>  {
>  	local file="$1"; shift
> @@ -188,9 +210,9 @@ process_series_mbox()
>  	local fname
>  	local dir
>  
> -	# Only patch 1/n is allowed to have no parent
> +	# Only patch 0/n or 1/n is allowed to have no parent
>  	local parent_id="$(fetch_header "In-Reply-To" < "$mbox")"
> -	if [ -z "$parent_id" ] && [ "$m" != 1 ]; then
> +	if [ -z "$parent_id" ] && [ "$m" != 1 ] && [ "$m" != 0 ]; then
>  		reject_and_reply "$mbox" <<EOF
>  You sent a patch that does not conform to the requirements for Aiaiai's Local
>  Delivery Agent. This patch is part of a series as indicated by its subject
> @@ -252,12 +274,40 @@ EOF
>  	done
>  
>  	# If the series is complete - queue it
> -	if [ "$(ls -1 -- "$dir" | wc -l)" -eq "$n" ]; then
> +	if series_is_complete "$dir" "$n"; then
>  		message "Patch-set at \"$dir\" is complete, queue it"
> -		for fname in $(ls -A -- "$dir" | sort -n); do
> +		# Don't add the 0th patch to the final mbox, as it is just the
> +		# cover letter and does not contain any patch
> +		for fname in $(ls --ignore=0 -A -- "$dir" | sort -n); do
>  			cat -- "$dir/$fname" >> "$mbox"
>  			echo >> "$mbox"
>  		done
> +
> +		if [ -f "$dir/0" ]; then
> +			# Save the subject and message ID of the cover letter
> +			# in the final mbox in order to be able to reply to the
> +			# cover letter later.
> +			local subj="$(fetch_header "Subject" < "$dir/0")"
> +			subj="X-Aiaiai-Cover-Letter-Subject: $subj"
> +
> +			local id="$(fetch_header "Message-Id" < "$dir/0")"
> +			id="X-Aiaiai-Cover-Letter-Message-Id: $id"
> +
> +			# The below trick allows us to avoid creating a
> +			# separate temporary file: open the "$mbox" file,
> +			# unlink, use the open file descriptor for reading and
> +			# redirect the output to the new version of the "$mbox"
> +			# file. We could instead use the "sponge" tool, though.
> +			exec 3<$mbox
> +			rm $verbose "$mbox" >&2
> +			message "Adding \"$subj\""
> +			formail -s formail -I "$subj" <&3 > "$mbox"
> +
> +			exec 3<$mbox
> +			rm $verbose "$mbox" >&2
> +			message "Adding \"$id\""
> +			formail -s formail -I "$id" <&3 > "$mbox"
> +		fi
>  		queue_mboxfile
>  		rm $verbose -rf -- "$dir" >&2
>  	fi
> @@ -308,8 +358,6 @@ process_mbox()
>  		[ "$n" -ne 0 ] || \
>  			{ reject "$mbox" "Prefix \"$prefix_format\" cannot have n = 0";
>  			  return; }
> -		[ "$m" -ne 0 ] || \
> -			{ message "Dropping patch \"0/$n\""; return; }
>  		process_series_mbox "$id" "$m" "$n"
>  	fi
>  }
> diff --git a/email/aiaiai-email-test-patchset b/email/aiaiai-email-test-patchset
> index d9e768e..af38bb5 100755
> --- a/email/aiaiai-email-test-patchset
> +++ b/email/aiaiai-email-test-patchset
> @@ -259,9 +259,16 @@ if [ -z "$mbox" ]; then
>  	cat > "$mbox"
>  fi
>  
> -fetch_header_or_die subj "Subject" < "$mbox"
> +cat "$mbox"
> +# Use the cover letter subject and message ID if possible. If the cover letter
> +# was present, 'aiaiai-email-lda' would save them in special privat headers.
> +subj="$(fetch_header "X-Aiaiai-Cover-Letter-Subject" < "$mbox")"
> +[ -n "$subj" ] || fetch_header_or_die subj "Subject" < "$mbox"
> +
> +id="$(fetch_header "X-Aiaiai-Cover-Letter-Message-Id" < "$mbox")"
> +[ -n "$id" ] || fetch_header_or_die id "Message-Id" < "$mbox"
> +
>  fetch_header_or_die from "From" < "$mbox"
> -fetch_header_or_die id "Message-Id" < "$mbox"
>  to="$(fetch_header "To" < "$mbox")"
>  cc="$(fetch_header "Cc" < "$mbox")"
>  

Excellent. I didn't know of that trick. I use sponge quite often, but
it's better to find a trick that does this without sponge!

This looks quite good.

Regards,
Jake