# [PATCH] format-patch: Use --mbox by default. 6 messages from 2006-01-08 to 2006-01-13. Participants: Andreas Ericsson, Junio C Hamano, H. Peter Anvin. Thread: https://gitlist.dev/t/3006 ## Andreas Ericsson, 2006-01-08 14:01 Subject: [PATCH] format-patch: Use --mbox by default. Message-ID: <20060108140109.909925BE8D@nox.op5.se> URL: https://gitlist.dev/e/20060108140109.909925BE8D%40nox.op5.se ``` This patch enables a slightly modified and more correct --mbox output by default. The --date, --author and --mbox options are obsoleted and removed. Previously, author-info and date was by default only printed if the commit was made by someone else, irrespective of the obsoleted options. Since it does no harm having ones own info there too it shouldn't really matter much, and should lessen the confusion regarding how to use git-format-patch with git-am. The patches generated also had bad mail headers for sendmail (of postfix 2.2.2 at least) to use directly (From: line must be on top, Subject: last of header), which isn't really an issue but annoying just the same. I haven't actually tried this new format, but at least 'file' now recognize it as "news or mail text", rather than "ASCII English text". Note that this reverts commit aa66c7ec77d474b737da607d6cb2d07f56628def, "format-patch/commit: Quote single quote in the author name properly" which is no longer needed since the whosepatchScript now prints the headers directly rather than passing them to eval. These changes allow us to remove two conditional statements and the invokation of a sub-shell in the process_one function, so it should be a bit faster too. Signed-off-by: Andreas Ericsson --- Documentation/git-format-patch.txt | 18 ++----------- git-format-patch.sh | 49 +++++++----------------------------- 2 files changed, 12 insertions(+), 55 deletions(-) b38d1ffeb27ba32de849f33a9c416fda4cd8eecb diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 47705de..73e1358 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -9,7 +9,7 @@ git-format-patch - Prepare patches for e SYNOPSIS -------- [verse] -'git-format-patch' [-n | -k] [-o | --stdout] [-s] [-c] [--mbox] +'git-format-patch' [-n | -k] [-o | --stdout] [-s] [-c] [--diff-options] [] DESCRIPTION @@ -27,9 +27,8 @@ When -n is specified, instead of "[PATCH line is formatted as "[PATCH N/M] Subject", unless you have only one patch. -When --mbox is specified, the output is formatted to resemble -UNIX mailbox format, and can be concatenated together for -processing with applymbox. +The output is formatted to resemble UNIX mailbox format, and can +be concatenated together for processing with gitlink::git-am[1]. OPTIONS @@ -45,11 +44,6 @@ OPTIONS Do not strip/add '[PATCH]' from the first line of the commit log message. --a|--author, -d|--date:: - Output From: and Date: headers for commits made by - yourself as well. Usually these are output only for - commits made by people other than yourself. - -s|--signoff:: Add `Signed-off-by:` line to the commit message, using the committer identity of yourself. @@ -61,12 +55,6 @@ OPTIONS has a SP character immediately followed by a TAB character. --m|--mbox:: - Format the output files for closer to mbox format by - adding a phony Unix "From " line, so they can be - concatenated together and fed to `git-applymbox`. - Implies --author and --date. - --stdout:: This flag generates the mbox formatted output to the standard output, instead of saving them into a file per diff --git a/git-format-patch.sh b/git-format-patch.sh index 818059f..047246e 100755 --- a/git-format-patch.sh +++ b/git-format-patch.sh @@ -3,7 +3,7 @@ # Copyright (c) 2005 Junio C Hamano # -USAGE='[-n | -k] [-o | --stdout] [--signoff] [--check] [--mbox] [--diff-options] []' +USAGE='[-n | -k] [-o | --stdout] [--signoff] [--check] [--diff-options] []' LONG_USAGE='Prepare each commit with its patch since our-head forked from upstream, one file per patch, for e-mail submission. Each output file is numbered sequentially from 1, and uses the first line of the commit @@ -15,9 +15,8 @@ the current working directory. When -n is specified, instead of "[PATCH] Subject", the first line is formatted as "[PATCH N/M] Subject", unless you have only one patch. -When --mbox is specified, the output is formatted to resemble -UNIX mailbox format, and can be concatenated together for processing -with applymbox.' +The output is formatted to resemble UNIX mailbox format, and can be +concatenated together for processing with "git am".' . git-sh-setup # Force diff to run in C locale. @@ -32,14 +31,8 @@ outdir=./ while case "$#" in 0) break;; esac do case "$1" in - -a|--a|--au|--aut|--auth|--autho|--author) - author=t ;; -c|--c|--ch|--che|--chec|--check) check=t ;; - -d|--d|--da|--dat|--date) - date=t ;; - -m|--m|--mb|--mbo|--mbox) - date=t author=t mbox=t ;; -k|--k|--ke|--kee|--keep|--keep-|--keep-s|--keep-su|--keep-sub|\ --keep-subj|--keep-subje|--keep-subjec|--keep-subject) keep_subject=t ;; @@ -48,7 +41,7 @@ do -s|--s|--si|--sig|--sign|--signo|--signof|--signoff) signoff=t ;; --st|--std|--stdo|--stdou|--stdout) - stdout=t mbox=t date=t author=t ;; + stdout=t ;; -o=*|--o=*|--ou=*|--out=*|--outp=*|--outpu=*|--output=*|--output-=*|\ --output-d=*|--output-di=*|--output-dir=*|--output-dire=*|\ --output-direc=*|--output-direct=*|--output-directo=*|\ @@ -173,12 +166,12 @@ titleScript=' whosepatchScript=' /^author /{ - s/'\''/'\''\\'\'\''/g - s/author \(.*>\) \(.*\)$/au='\''\1'\'' ad='\''\2'\''/p + s/^author \([^>]*>\)[\t ]*\(.*\)$/From: \1\nDate: \2/p q }' process_one () { + sed -ne "$whosepatchScript" $commsg mailScript=' /./d /^$/n' @@ -191,26 +184,7 @@ process_one () { ;; esac mailScript="$mailScript"' - s|^|Subject: |' - case "$mbox" in - t) - echo 'From nobody Mon Sep 17 00:00:00 2001' ;# UNIX "From" line - ;; - esac - - eval "$(sed -ne "$whosepatchScript" $commsg)" - test "$author,$au" = ",$me" || { - mailScript="$mailScript"' - a\ -From: '"$au" - } - test "$date,$au" = ",$me" || { - mailScript="$mailScript"' - a\ -Date: '"$ad" - } - - mailScript="$mailScript"' + s|^|Subject: | a\ : body @@ -218,7 +192,7 @@ Date: '"$ad" n b body' - (cat $commsg ; echo; echo) | + (cat $commsg; echo; echo) | sed -ne "$mailScript" | git-stripspace @@ -239,12 +213,7 @@ Date: '"$ad" git-diff-tree -p $diff_opts "$commit" echo "-- " echo "@@GIT_VERSION@@" - - case "$mbox" in - t) - echo - ;; - esac + echo } total=`wc -l <$series | tr -dc "[0-9]"` -- 1.0.7-gb38d1 ``` ## Junio C Hamano, 2006-01-08 20:28 Subject: Re: [PATCH] format-patch: Use --mbox by default. Message-ID: <7vhd8ezcol.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vhd8ezcol.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060108140109.909925BE8D@nox.op5.se> ``` Andreas Ericsson writes: > This patch enables a slightly modified and more correct --mbox > output by default. The --date, --author and --mbox options > are obsoleted and removed. I always use format-patch with -k -m (or -k --stdout), so I have no objections to making --mbox the default. I do not think anybody uses --date or --author in their scripts, but removing the --mbox option _might_ break things for people's scripts. Maybe it is worthwhile to keep it as a no-op option for a while. > Previously, author-info and date was by default only printed if the > commit was made by someone else, irrespective of the obsoleted options. > Since it does no harm having ones own info there too it shouldn't really > matter much, and should lessen the confusion regarding how to use > git-format-patch with git-am. True. Originally it was really "format my changes to submission" tool (--signoff option is a sure sign to tell you that) and propagating authorship info was added as afterthought. > The patches generated also had bad mail headers for sendmail (of > postfix 2.2.2 at least) to use directly (From: line must be on top, > Subject: last of header),... I do not think RFC2822 specifies that. See "3.6 Field definitions", third paragraph, first two sentences. In that sense, technically they are not "bad mail headers", but at the same time, since it is valid in any order, if _one_ popular MTA is unconfortable with a particular ordering, we could order things differently to make it happier (mind you, this would not work for more than one such broken MTAs). Another thing I've been considering is to handle UTF-8 (or i18n.commitencoding in general) a bit better. We currently do not have Content-Type: header, so if you pipe the output to "am", it must be invoked without "-u" to make sure that it takes things as-is ("am -u" assumes things are in latin1 without Content-Type:). If we start to tackle that issue, we might also want to do RFC2047 in headers. ``` ## H. Peter Anvin, 2006-01-08 22:14 Subject: Re: [PATCH] format-patch: Use --mbox by default. Message-ID: <43C18EE1.4050807@zytor.com> URL: https://gitlist.dev/e/43C18EE1.4050807%40zytor.com In-Reply-To: <20060108140109.909925BE8D@nox.op5.se> ``` Andreas Ericsson wrote: > > The patches generated also had bad mail headers for sendmail (of > postfix 2.2.2 at least) to use directly (From: line must be on top, > Subject: last of header) Sounds like a bug report against postfix is in order. -hpa ``` ## Andreas Ericsson, 2006-01-09 02:01 Subject: Re: [PATCH] format-patch: Use --mbox by default. Message-ID: <43C1C3FA.8070609@op5.se> URL: https://gitlist.dev/e/43C1C3FA.8070609%40op5.se In-Reply-To: <7vhd8ezcol.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Andreas Ericsson writes: > > >>This patch enables a slightly modified and more correct --mbox >>output by default. The --date, --author and --mbox options >>are obsoleted and removed. > > > I always use format-patch with -k -m (or -k --stdout), so I have > no objections to making --mbox the default. I do not think > anybody uses --date or --author in their scripts, but removing > the --mbox option _might_ break things for people's scripts. > Maybe it is worthwhile to keep it as a no-op option for a while. > The reworked patch (which works for sending patches to myself, so far), prints a warning to stderr that a deprecated option is used and then silently ignores it. > >>The patches generated also had bad mail headers for sendmail (of >>postfix 2.2.2 at least) to use directly (From: line must be on top, >>Subject: last of header),... > > > I do not think RFC2822 specifies that. See "3.6 Field > definitions", third paragraph, first two sentences. > > In that sense, technically they are not "bad mail headers", but > at the same time, since it is valid in any order, if _one_ > popular MTA is unconfortable with a particular ordering, we > could order things differently to make it happier (mind you, > this would not work for more than one such broken MTAs). > The only thing I've seen break is if Subject: is placed on top. It's easier, script-wise, to put it last and that seems to work with qmail and postfix both. > Another thing I've been considering is to handle UTF-8 (or > i18n.commitencoding in general) a bit better. We currently do > not have Content-Type: header, so if you pipe the output to > "am", it must be invoked without "-u" to make sure that it takes > things as-is ("am -u" assumes things are in latin1 without > Content-Type:). If we start to tackle that issue, we might also > want to do RFC2047 in headers. > > You'll have to ask someone else for that. I have no idea how it works and I wouldn't know how to begin doing it in shell. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ``` ## Andreas Ericsson, 2006-01-09 02:05 Subject: Re: [PATCH] format-patch: Use --mbox by default. Message-ID: <43C1C4D7.7060107@op5.se> URL: https://gitlist.dev/e/43C1C4D7.7060107%40op5.se In-Reply-To: <43C18EE1.4050807@zytor.com> ``` H. Peter Anvin wrote: > Andreas Ericsson wrote: > >> >> The patches generated also had bad mail headers for sendmail (of >> postfix 2.2.2 at least) to use directly (From: line must be on top, >> Subject: last of header) > > > Sounds like a bug report against postfix is in order. > No need. It works with 2.2.8. The 2.2.2 it failed with is the default for FC4. I have no interest investigating all the possibilities, and I doubt Vietse will look for bugs in old versions unless they're security related. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ``` ## Junio C Hamano, 2006-01-13 23:56 Subject: [PATCH] format-patch: always --mbox and show sane Date: Message-ID: <7v1wzb1y2r.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1wzb1y2r.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060108140109.909925BE8D@nox.op5.se> ``` Make --mbox, --author, and --date options a no-op, and always use --mbox output, and rewrite the commit log formatting in Perl. This makes it easier to output Date: header in RFC 2822 format, so do that as well. Inspiration for this patch came from Andreas Ericsson's earlier patch. Signed-off-by: Junio C Hamano --- * I admit that trying to avoid Perl and sticking to multiple sed invocations in this script was a lost battle. This patch is still minimalistic and it does not go all the way to rewrite the main loop of the program, which probably should happen someday. I would not mind if someday somebody rewrote most of the barebone Porcelainish in Python or Perl, but the rewrite should not be for the sake of using that language. It should have other justifications: "there is too much text processing there it would not make sense to do that in shell". I think this patch is borderline to qualify to be in that camp. As the version string at the end shows, this was prepared with the script source itself before building, as a test case. git-format-patch.sh | 143 +++++++++++++++++++++++++++------------------------ 1 files changed, 75 insertions(+), 68 deletions(-) 8b4271ef24818e6d52926bc615e5ce78ba0ed01b diff --git a/git-format-patch.sh b/git-format-patch.sh index d3979d7..7e67c4e 100755 --- a/git-format-patch.sh +++ b/git-format-patch.sh @@ -34,14 +34,12 @@ outdir=./ while case "$#" in 0) break;; esac do case "$1" in - -a|--a|--au|--aut|--auth|--autho|--author) - author=t ;; -c|--c|--ch|--che|--chec|--check) check=t ;; - -d|--d|--da|--dat|--date) - date=t ;; - -m|--m|--mb|--mbo|--mbox) - date=t author=t mbox=t ;; + -a|--a|--au|--aut|--auth|--autho|--author|\ + -d|--d|--da|--dat|--date|\ + -m|--m|--mb|--mbo|--mbox) # now noop + ;; -k|--k|--ke|--kee|--keep|--keep-|--keep-s|--keep-su|--keep-sub|\ --keep-subj|--keep-subje|--keep-subjec|--keep-subject) keep_subject=t ;; @@ -173,80 +171,89 @@ titleScript=' q ' -whosepatchScript=' -/^author /{ - s/'\''/'\''\\'\'\''/g - s/author \(.*>\) \(.*\)$/au='\''\1'\'' ad='\''\2'\''/p - q -}' - process_one () { - mailScript=' - /./d - /^$/n' - case "$keep_subject" in - t) ;; - *) - mailScript="$mailScript"' - s|^\[PATCH[^]]*\] *|| - s|^|[PATCH'"$num"'] |' - ;; - esac - mailScript="$mailScript"' - s|^|Subject: |' - case "$mbox" in - t) - echo 'From nobody Mon Sep 17 00:00:00 2001' ;# UNIX "From" line - ;; - esac + perl -w -e ' +my ($keep_subject, $num, $signoff, $commsg) = @ARGV; +my ($signoff_pattern, $done_header, $done_subject, $signoff_seen, + $last_was_signoff); + +if ($signoff) { + $signoff = `git-var GIT_COMMITTER_IDENT`; + $signoff =~ s/>.*/>/; + $signoff_pattern = quotemeta($signoff); +} - eval "$(sed -ne "$whosepatchScript" $commsg)" - test "$author,$au" = ",$me" || { - mailScript="$mailScript"' - a\ -From: '"$au" - } - test "$date,$au" = ",$me" || { - mailScript="$mailScript"' - a\ -Date: '"$ad" - } +my @weekday_names = qw(Sun Mon Tue Wed Thu Fri Sat); +my @month_names = qw(Jan Feb Mar Apr May Jun Jul Aug Sep Oct Nov Dec); - mailScript="$mailScript"' - a\ +sub show_date { + my ($time, $tz) = @_; + my $minutes = abs($tz); + $minutes = ($minutes / 100) * 60 + ($minutes % 100); + if ($tz < 0) { + $minutes = -$minutes; + } + my $t = $time + $minutes * 60; + my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday) = gmtime($t); + return sprintf("%s %s %d %02d:%02d:%02d %d %+05d", + $weekday_names[$wday], + $month_names[$mon], + $mday, $hour, $min, $sec, + $year+1900, $tz); +} - : body - p - n - b body' +print "From nobody Mon Sep 17 00:00:00 2001\n"; +open FH, "git stripspace <$commsg |" or die "open $commsg pipe"; +while () { + unless ($done_header) { + if (/^$/) { + $done_header = 1; + } + elsif (/^author (.*>) (.*)$/) { + my ($author_ident, $author_date) = ($1, $2); + my ($utc, $off) = ($author_date =~ /^(\d+) ([-+]?\d+)$/); + $author_date = show_date($utc, $off); - (cat $commsg ; echo; echo) | - sed -ne "$mailScript" | - git-stripspace - - test "$signoff" = "t" && { - offsigner=`git-var GIT_COMMITTER_IDENT | sed -e 's/>.*/>/'` - line="Signed-off-by: $offsigner" - grep -q "^$line\$" $commsg || { - echo - echo "$line" - echo - } + print "From: $author_ident\n"; + print "Date: $author_date\n"; } - echo - echo '---' - echo + next; + } + unless ($done_subject) { + unless ($keep_subject) { + s/^\[PATCH[^]]*\]\s*//; + s/^/[PATCH$num] /; + } + print "Subject: $_"; + $done_subject = 1; + next; + } + + $last_was_signoff = 0; + if (/Signed-off-by:/i) { + if ($signoff ne "" && /Signed-off-by:\s*$signoff_pattern$/i) { + $signoff_seen = 1; + } + } + print $_; +} +if (!$signoff_seen && $signoff ne "") { + if (!$last_was_signoff) { + print "\n"; + } + print "$signoff\n"; +} +print "\n---\n\n"; +close FH or die "close $commsg pipe"; +' "$keep_subject" "$num" "$signoff" $commsg + git-diff-tree -p $diff_opts "$commit" | git-apply --stat --summary echo git-diff-tree -p $diff_opts "$commit" echo "-- " echo "@@GIT_VERSION@@" - case "$mbox" in - t) - echo - ;; - esac + echo } total=`wc -l <$series | tr -dc "[0-9]"` -- @@GIT_VERSION@@ ```