[PATCH 0/2] git-am foreign patch support

[PATCH 0/2] git-am foreign patch support

[Giuseppe Bilotta]

This small patchset introduces a framework to allow git-am to support
patchsets not in mailbox format.

The first patch introduces a new patch_format variable that holds the
(autodetected) patch format. All patches are assumed to be in the same
format. Autodetection for Mercurial and StGIT patches is also
implemented, including series index expansion for StGIT.

The second patch introduces actual support for StGIT patches by using
some awk code to convert them to mailbox format.

Giuseppe Bilotta (2):
  git-am foreign patch support: format autodetection
  git-am foreign patch support: StGIT

 git-am.sh |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 137 insertions(+), 3 deletions(-)

[PATCH 1/2] git-am foreign patch support: format autodetection

[Giuseppe Bilotta]

This patch is the first step towards the introduction of a framework to
allow git-am to import patches not in mailbox format.

Currently detected formats are
  * the mailbox format itself, which is assumed by default if input is
    form stdin
  * Mercurial's output from 'hg export'
  * Stacked Git's output from 'stg export' with the default export
    template; StGIT patch series are also detected and expanded.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-am.sh |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 578780b..3508b7e 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -274,10 +274,112 @@ else
 		done
 		shift
 	fi
-	git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||  {
-		rm -fr "$dotest"
+
+	# format of the patch(es) to be processed. we assume they are all
+	# of the same kind
+	patch_format=none
+
+	# from stdin we only accept mboxes, because peeking at stdin
+	# to detect the format is destructive
+	case $# in
+	0)
+		patch_format=mbox
+		;;
+	1)
+		if test x"$1" = x"-"
+		then
+			# stdin, so assume mbox
+			patch_format=mbox
+		else
+# a single non-stdin argument was passed, check if it's a StGit patch series
+# index by checking if the first line begins with '# This series'
+			{
+				read l1
+				case "$l1" in
+				'# This series '*)
+# replace the argument list with the files listed in the series index,
+# prefixing them with the series index dirname, skipping comment lines
+					series_dir=`dirname "$1"`
+					while read filename
+					do
+						set "$@" "$series_dir/$filename"
+					done
+					# remove the series index name
+					shift
+					# set the patch format appropriately
+					patch_format=stgit
+					;;
+				*)
+					;;
+				esac
+			} < "$1"
+		fi
+		;;
+	*)
+		;;
+	esac
+
+	# if the format is not defined yet, we can look at the first patch
+	# (which is not stdin) to try to understand the format.
+	if test $patch_format = none
+	then
+		echo "$patch_format"
+		# if it's a directory, assume mbox format
+		# TODO we could suppot StGIT patch series here too
+		if test -d "$1"
+		then
+			patch_format=mbox
+		else
+			# read the first four lines
+			{
+				read l1
+				read l2
+				read l3
+				read l4
+			} < "$1"
+			case "$l1" in
+			"# HG changeset patch")
+				patch_format=hg
+				;;
+			From\ *)
+				patch_format=mbox
+				;;
+			From:\ *)
+				patch_format=mbox
+				;;
+			*)
+				# if the second and fourth lines are empty,
+				# this might be an StGIT patch
+				if test x"$l2$l4" = x
+				then
+					case "$l3" in
+						From:\ *)
+							patch_format=stgit
+							;;
+						Author:\ *)
+							patch_format=stgit
+							;;
+						*)
+							;;
+					esac
+				fi
+				;;
+			esac
+		fi
+	fi
+
+	case "$patch_format" in
+	mbox)
+		git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||  {
+			rm -fr "$dotest"
+			exit 1
+		}
+		;;
+	*)
+		echo "Patch format $patch_format is not currently handled, sorry"
 		exit 1
-	}
+		;;
+	esac
 
 	# -s, -u, -k, --whitespace, -3, -C and -p flags are kept
 	# for the resuming session after a patch failure.
-- 
1.6.3.1.245.g4529.dirty

[PATCH 2/2] git-am foreign patch support: StGIT

[Giuseppe Bilotta]

Convert StGIT patches to mbox format so that they can be imported.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-am.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 3508b7e..5199354 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -375,6 +375,38 @@ else
 			exit 1
 		}
 		;;
+	stgit)
+		this=0
+		for stgit in "$@"
+		do
+			this=`expr "$this" + 1`
+			msgnum=`printf "%0${prec}d" $this`
+			touch "$dotest/$msgnum"
+			# Awk version of StGIT parse_patch
+			awk 'BEGIN { subject=0 }
+			{
+				if (subject > 1)
+					print ;
+				else if (/^$/) next ;
+				else if (/^Author:/) print sub("Author", "From"), $ORS ;
+				else if (/^(From|Date)/) print ;
+				else if (subject) {
+					subject = 2 ;
+					print "" ;
+					print ;
+				} else {
+					print "Subject:", $0 ;
+					subject = 1;
+				}
+			}' "$stgit" > "$dotest/$msgnum" || {
+				echo "Failed to import $patch_format patch $stgit"
+				exit 1
+			}
+		done
+		echo "$this" > "$dotest/last"
+		this=
+		msgnum=
+		;;
 	*)
 		echo "Patch format $patch_format is not currently handled, sorry"
 		exit 1
-- 
1.6.3.1.245.g4529.dirty

Re: [PATCH 1/2] git-am foreign patch support: format autodetection

[Junio C Hamano]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> This patch is the first step towards the introduction of a framework to
> allow git-am to import patches not in mailbox format.
>
> Currently detected formats are
>   * the mailbox format itself, which is assumed by default if input is
>     form stdin
>   * Mercurial's output from 'hg export'
>   * Stacked Git's output from 'stg export' with the default export
>     template; StGIT patch series are also detected and expanded.

I personally do not trust "autodetection" (especially done by others ;-),
and prefer to have an explicit override by the users, but that aside...

> +	# from stdin we only accept mboxes, because peeking at stdin
> +	# to detect the format is destructive
> +	case $# in
> +	0)
> +		patch_format=mbox
> +		;;
> +	1)
> +		if test x"$1" = x"-"
> +		then
> +			# stdin, so assume mbox
> +			patch_format=mbox
> +		else

	# have parseopt to set explicit patch_format before this part...

	if test -z "$patch_format" && {
           test $# = 0 || test "x$1" = x-
        }
        then
        	patch_format=mbox
	else
        	patch_format=$(guess_patch_format)
	fi

Having this extra logic inside the main codeflow makes it extremely harder
to read; have it in a separate shell function.

> +# a single non-stdin argument was passed, check if it's a StGit patch series
> +# index by checking if the first line begins with '# This series'
> +			{
> +				read l1
> +				case "$l1" in
> +				'# This series '*)
> +# replace the argument list with the files listed in the series index,
> +# prefixing them with the series index dirname, skipping comment lines

Can the "series-index-name" file begin with '-' (which would affect the
way how 'set "@"' works in the loop below)?  A standard trick would be to
do something like this.

	series_index="$1"
	shift ;# discard
        set x
        while ...
	do
        	set "$@" another
	done
        shift ;# discard 'x' protection

> +	# (which is not stdin) to try to understand the format.
> +	if test $patch_format = none

I do not understand this duplication and inconsistency.  Why have the
detection in two places?

> +	case "$patch_format" in
> +	mbox)
> +		git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||  {
> +			rm -fr "$dotest"
> +			exit 1
> +		}
> +		;;
> +	*)
> +		echo "Patch format $patch_format is not currently handled, sorry"
>  		exit 1

No fixing broken "Subject:" line for your format here?

Re: [PATCH 1/2] git-am foreign patch support: format autodetection

[Giuseppe Bilotta]

On Tue, May 26, 2009 at 12:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> This patch is the first step towards the introduction of a framework to
>> allow git-am to import patches not in mailbox format.
>>
>> Currently detected formats are
>>   * the mailbox format itself, which is assumed by default if input is
>>     form stdin
>>   * Mercurial's output from 'hg export'
>>   * Stacked Git's output from 'stg export' with the default export
>>     template; StGIT patch series are also detected and expanded.
>
> I personally do not trust "autodetection" (especially done by others ;-),
> and prefer to have an explicit override by the users, but that aside...

No problem. --patch-format or just --format ?

>        if test -z "$patch_format" && {
>           test $# = 0 || test "x$1" = x-
>        }
>        then
>                patch_format=mbox
>        else
>                patch_format=$(guess_patch_format)
>        fi
>
> Having this extra logic inside the main codeflow makes it extremely harder
> to read; have it in a separate shell function.

I assume you mean the patch format detection, yes?

>> +# a single non-stdin argument was passed, check if it's a StGit patch series
>> +# index by checking if the first line begins with '# This series'
>> +                     {
>> +                             read l1
>> +                             case "$l1" in
>> +                             '# This series '*)
>> +# replace the argument list with the files listed in the series index,
>> +# prefixing them with the series index dirname, skipping comment lines
>
> Can the "series-index-name" file begin with '-' (which would affect the
> way how 'set "@"' works in the loop below)?  A standard trick would be to
> do something like this.
>
>        series_index="$1"
>        shift ;# discard
>        set x
>        while ...
>        do
>                set "$@" another
>        done
>        shift ;# discard 'x' protection

Ah, good point. I'll do it that way.

>> +     # (which is not stdin) to try to understand the format.
>> +     if test $patch_format = none
>
> I do not understand this duplication and inconsistency.  Why have the
> detection in two places?

It's not in two places. The first part sets the patch format only if
we are either reading from stdin or have been passed a stgit patch
series. Otherwise, we still don't know what we're getting, so now we
inspect the first patch to see what format it's in. (Consider for
example the case of appication of a StGIT patch which is not part of a
series.)

>> +     case "$patch_format" in
>> +     mbox)
>> +             git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||  {
>> +                     rm -fr "$dotest"
>> +                     exit 1
>> +             }
>> +             ;;
>> +     *)
>> +             echo "Patch format $patch_format is not currently handled, sorry"
>>               exit 1
>
> No fixing broken "Subject:" line for your format here?

No, I put that in the second patch, because it was a different thing
(patch processing as opposed to format detection).


-- 
Giuseppe "Oblomov" Bilotta

Re: [PATCH 0/2] git-am foreign patch support

[Sverre Rabbelier]

Heya,

On Mon, May 25, 2009 at 21:14, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> Autodetection for Mercurial [...] patches is also implemented, [...]

Sweet! I now have a vested interest to see this accepted :P.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 0/2] git-am foreign patch support

[Giuseppe Bilotta]

On Mon, May 25, 2009 at 9:19 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
> On Mon, May 25, 2009 at 21:14, Giuseppe Bilotta
> <giuseppe.bilotta@gmail.com> wrote:
>> Autodetection for Mercurial [...] patches is also implemented, [...]
>
> Sweet! I now have a vested interest to see this accepted :P.

Keep in mind that presently it just detects them, it doesn't actually
import them. But their conversion to mbox format seems rather
straightforward, so it can be added easily.

-- 
Giuseppe "Oblomov" Bilotta