threads / patch / 19502

patch, 2 partsgit-am foreign patch support

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

## tl;dr

7 messages between May 25, 2009 and May 25, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Giuseppe Bilotta· May 25, 2009, 19:14 UTC · lore

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(-)
Giuseppe Bilotta· May 25, 2009, 19:14 UTC · re: Giuseppe Bilotta · lore

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

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(-)
Show changes to git-am.sh +105 −3
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
Giuseppe Bilotta· May 25, 2009, 19:14 UTC · re: Giuseppe Bilotta · lore

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

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(-)
Show changes to git-am.sh +32 −0
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
Junio C Hamano· May 25, 2009, 22:23 UTC · re: Giuseppe Bilotta · lore

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

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
Show 9 quoted lines
> 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...

Show 12 quoted lines
> +	# 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.

Show 8 quoted lines
> +# 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?

Show 10 quoted lines
> +	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?
Giuseppe Bilotta· May 25, 2009, 22:49 UTC · re: Junio C Hamano · lore

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

On Tue, May 26, 2009 at 12:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> 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 ?
Show 11 quoted lines
>        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?
Show 21 quoted lines
>> +# 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.
Show 5 quoted lines
>> +     # (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.)

Show 12 quoted lines
>> +     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
Sverre Rabbelier· May 25, 2009, 19:19 UTC · re: Giuseppe Bilotta · lore

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

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
Giuseppe Bilotta· May 25, 2009, 19:24 UTC · re: Sverre Rabbelier · lore

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

On Mon, May 25, 2009 at 9:19 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 7 quoted lines
> 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

← back to recent threads