{"thread":{"id":"1539","subject":"Re: [RFC] Patches exchange is bad?","startedAt":"2005-08-16T21:47:07Z","lastAt":"2005-08-19T21:41:50Z","messageCount":15,"participants":["Marco Costalba","Junio C Hamano","Linus Torvalds","Jeff Garzik","Johannes Schindelin","David Kågedal","Sam Ravnborg"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"7389","messageId":"20050816214707.6842.qmail@web26302.mail.ukl.yahoo.com","threadId":"1539","inReplyTo":null,"subject":"Re: [RFC] Patches exchange is bad?","fromName":"Marco Costalba","fromEmail":"mcostalba@yahoo.it","sentAt":"2005-08-16T21:47:07Z","receivedAt":"2005-08-16T21:47:07Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n\n>\n>I would like to know a bit about \"git format-patch\" adding extra\n>info that you needed to get rid of.  It shouldn't be necessary.\n>\n\nAs example, in the rev d5a63b99835017d2638e55a7e34a35a3c1e80f1f from git\nthe original subject is:\n\n'   Alternate object pool mechanism updates.'\n\nwhile, after the round-trip git-format-patch + git applymbox I have\n\n'   [PATCH] Alternate object pool mechanism updates.'\n\nThe extra '[PATCH]' in the subject was introduced by git-format-patch --mbox.\n\n\nPerpahs I have made something wrong.\n\n\nMarco\n\n\n\n\t\t\n____________________________________________________\nStart your day with Yahoo! - make it your home page \nhttp://www.yahoo.com/r/hs \n \n"},{"id":"7392","messageId":"7vek8t7bva.fsf@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"20050816214707.6842.qmail@web26302.mail.ukl.yahoo.com","subject":"[PATCH] git-format-patch fix","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-16T22:31:05Z","receivedAt":"2005-08-16T22:31:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Introduces --keep-subjects flag to tell it not to munge the\nfirst line of the commit message.  Running \"git applymbox\" on\nthe output from \"git format-patch -m -k\" would preserve the\noriginal commit information better this way.\n\nAt the same time, prefix Subject: on the first line of the\ncommit, to help people cut&copy.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\nMarco Costalba <mcostalba@yahoo.it> writes:\n\n> The extra '[PATCH]' in the subject was introduced by git-format-patch --mbox.\n> Perhaps I have made something wrong.\n\nNo, sorry, I noticed that myself after I wrote that message.\nThere should be an option to tell --mbox format not do that.  My\nthinko. \n\nAdding [PATCH] on the subject line even in --mbox format should\nalso be supported for people who run \"git send-email\" to send\nthings out.  So there is a new option '-k' to keep the original\nsubject line for the \"cherry-pick\" operation.\n\nThis also changes it to always prefix \"Subject: \".  I think it\nmakes the output look consistent, even without --mbox option,\nand would help people cutting and pasting the Subject line,\ndepending on their MUAs (it would certainly help me), but at the\nsame time it may make cutting and pasting cumbersome for some\nother people.  Opinions?  Objections?\n\n------------\n\n git-format-patch-script |   31 ++++++++++++++++++++-----------\n 1 files changed, 20 insertions(+), 11 deletions(-)\n\n32128d94ab63ea65bb86cb7fa7425e51e0dfeeb0\ndiff --git a/git-format-patch-script b/git-format-patch-script\n--- a/git-format-patch-script\n+++ b/git-format-patch-script\n@@ -6,7 +6,7 @@\n . git-sh-setup-script || die \"Not a git archive.\"\n \n usage () {\n-    echo >&2 \"usage: $0\"' [-n] [-o dir] [--mbox] [--check] [--signoff] [-<diff options>...] upstream [ our-head ]\n+    echo >&2 \"usage: $0\"' [-n] [-o dir] [--keep-subject] [--mbox] [--check] [--signoff] [-<diff options>...] upstream [ our-head ]\n \n Prepare each commit with its patch since our-head forked from upstream,\n one file per patch, for e-mail submission.  Each output file is\n@@ -44,6 +44,9 @@ do\n     date=t ;;\n     -m|--m|--mb|--mbo|--mbox)\n     date=t author=t mbox=t ;;\n+    -k|--k|--ke|--kee|--keep|--keep-|--keep-s|--keep-su|--keep-sub|\\\n+    --keep-subj|--keep-subje|--keep-subjec|--keep-subject)\n+    keep_subject=t ;;\n     -n|--n|--nu|--num|--numb|--numbe|--number|--numbere|--numbered)\n     numbered=t ;;\n     -s|--s|--si|--sig|--sign|--signo|--signof|--signoff)\n@@ -64,6 +67,11 @@ do\n     shift\n done\n \n+case \"$keep_subject$numbered\" in\n+tt)\n+\tdie '--keep-subject and --numbered are incompatible.' ;;\n+esac\n+\n revpair=\n case \"$#\" in\n 2)\n@@ -142,21 +150,22 @@ do\n     {\n \tmailScript='\n \t/./d\n-\t/^$/n\n-\ts|^\\[PATCH[^]]*\\] *||'\n-\n-\tcase \"$mbox\" in\n-\tt)\n-\t    echo 'From nobody Mon Sep 17 00:00:00 2001' ;# UNIX \"From\" line\n-\t    mailScript=\"$mailScript\"'\n-\t    s|^|Subject: [PATCH'\"$num\"'] |'\n-\t    ;;\n+\t/^$/n'\n+\tcase \"$keep_subject\" in\n+\tt)  ;;\n \t*)\n \t    mailScript=\"$mailScript\"'\n+\t    s|^\\[PATCH[^]]*\\] *||\n \t    s|^|[PATCH'\"$num\"'] |'\n \t    ;;\n \tesac\n-\n+\tmailScript=\"$mailScript\"'\n+\ts|^|Subject: |'\n+\tcase \"$mbox\" in\n+\tt)\n+\t    echo 'From nobody Mon Sep 17 00:00:00 2001' ;# UNIX \"From\" line\n+\t    ;;\n+\tesac\n \teval \"$(sed -ne \"$whosepatchScript\" $commsg)\"\n \ttest \"$author,$au\" = \",$me\" || {\n \t\tmailScript=\"$mailScript\"'\n"},{"id":"7422","messageId":"7vk6il2jz8.fsf_-_@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"7vek8t7bva.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Teach applymbox to keep the Subject: line.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-17T05:47:07Z","receivedAt":"2005-08-17T05:47:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a companion patch to the previous format-patch fix.\nWith \"-k\", format-patch can be told not to remove the [PATCH] in\nthe original commit, nor to add the [PATCH] on its own.\n\nHowever, applymbox toolchain has a code to remove [PATCH] (among\nother things) from the Subject: line, which is the right thing\nto do when dealing with real e-mailed patches, but it interferes\nwith the \"format-patch to applymbox\" cherry-picking.  The -k\nflag disables this.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n tools/git-applymbox  |   11 ++++++++---\n tools/git-applypatch |    5 ++++-\n tools/mailinfo.c     |   15 +++++++++++++++\n 3 files changed, 27 insertions(+), 4 deletions(-)\n\n6bff6a60680ef402f614abae8189c2cb198cfa49\ndiff --git a/tools/git-applymbox b/tools/git-applymbox\n--- a/tools/git-applymbox\n+++ b/tools/git-applymbox\n@@ -9,7 +9,7 @@\n ## You give it a mbox-format collection of emails, and it will try to\n ## apply them to the kernel using \"applypatch\"\n ##\n-## applymbox [ -q ] (-c .dotest/msg-number | mail_archive) [Signoff_file]\"\n+## applymbox [ -k ] [ -q ] (-c .dotest/msg-number | mail_archive) [Signoff_file]\"\n ##\n ## The patch application may fail in the middle.  In which case:\n ## (1) look at .dotest/patch and fix it up to apply\n@@ -18,10 +18,11 @@\n ## use a Signoff_file, because applypatch wants to append the sign-off\n ## message to msg-clean every time it is run.\n \n-query_apply= continue= resume=t\n+keep_subject= query_apply= continue= resume=t\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n+\t-k)\tkeep_subject=-k ;;\n \t-q)\tquery_apply=t ;;\n \t-c)\tcontinue=\"$2\"; resume=f; shift ;;\n \t-*)\tusage ;;\n@@ -41,6 +42,9 @@ esac\n case \"$query_apply\" in\n t)\ttouch .dotest/.query_apply\n esac\n+case \"$keep_subject\" in\n+-k)\t: >.dotest/.keep_subject\n+esac\n \n signoff=\"$1\"\n set x .dotest/0*\n@@ -52,7 +56,8 @@ do\n     f,$i)\tresume=t;;\n     f,*)\tcontinue;;\n     *)\n-\t    git-mailinfo .dotest/msg .dotest/patch <$i >.dotest/info || exit 1\n+\t    git-mailinfo $keep_subject \\\n+\t\t.dotest/msg .dotest/patch <$i >.dotest/info || exit 1\n \t    git-stripspace < .dotest/msg > .dotest/msg-clean\n \t    ;;\n     esac\ndiff --git a/tools/git-applypatch b/tools/git-applypatch\n--- a/tools/git-applypatch\n+++ b/tools/git-applypatch\n@@ -16,6 +16,7 @@ final=.dotest/final-commit\n ## If this file exists, we ask before applying\n ##\n query_apply=.dotest/.query_apply\n+keep_subject=.dotest/.keep_subject\n MSGFILE=$1\n PATCHFILE=$2\n INFO=$3\n@@ -30,8 +31,10 @@ export SUBJECT=\"$(sed -n '/^Subject/ s/S\n if [ -n \"$signoff\" -a -f \"$signoff\" ]; then\n \tcat $signoff >> $MSGFILE\n fi\n+patch_header=\n+test -f \"$keep_subject\" || patch_header='[PATCH] '\n \n-(echo \"[PATCH] $SUBJECT\" ; if [ -s $MSGFILE ]; then echo ; cat $MSGFILE; fi ) > $final\n+(echo \"$patch_header$SUBJECT\" ; if [ -s $MSGFILE ]; then echo ; cat $MSGFILE; fi ) > $final\n \n f=0\n [ -f $query_apply ] || f=1\ndiff --git a/tools/mailinfo.c b/tools/mailinfo.c\n--- a/tools/mailinfo.c\n+++ b/tools/mailinfo.c\n@@ -9,6 +9,7 @@\n \n static FILE *cmitmsg, *patchfile;\n \n+static int keep_subject = 0;\n static char line[1000];\n static char date[1000];\n static char name[1000];\n@@ -101,6 +102,8 @@ static void check_line(char *line, int l\n \n static char * cleanup_subject(char *subject)\n {\n+\tif (keep_subject)\n+\t\treturn subject;\n \tfor (;;) {\n \t\tchar *p;\n \t\tint len, remove;\n@@ -242,8 +245,20 @@ static void usage(void)\n \texit(1);\n }\n \n+static const char mailinfo_usage[] =\n+\"git-mailinfo [-k] msg patch <mail >info\";\n int main(int argc, char ** argv)\n {\n+\twhile (1 < argc && argv[1][0] == '-') {\n+\t\tif (!strcmp(argv[1], \"-k\"))\n+\t\t\tkeep_subject = 1;\n+\t\telse {\n+\t\t\tfprintf(stderr, \"usage: %s\\n\", mailinfo_usage);\n+\t\t\texit(1);\n+\t\t}\n+\t\targc--; argv++;\n+\t}\n+\n \tif (argc != 3)\n \t\tusage();\n \tcmitmsg = fopen(argv[1], \"w\");\n"},{"id":"7446","messageId":"Pine.LNX.4.58.0508170830050.3553@g5.osdl.org","threadId":"1539","inReplyTo":"7vk6il2jz8.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-17T15:38:28Z","receivedAt":"2005-08-17T15:38:28Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 16 Aug 2005, Junio C Hamano wrote:\n>\n> This is a companion patch to the previous format-patch fix.\n> With \"-k\", format-patch can be told not to remove the [PATCH] in\n> the original commit, nor to add the [PATCH] on its own.\n\nI think this might be the point in time to just make the \"[PATCH]\" prefix \ngo away.\n\nIt made much more sense with BK than it does with git: since git keeps \ntrack of \"author\" and \"committer\" separately, you can always see when the \ncommitter wasn't the author of the change, which is what the \"[PATCH]\" \nthing was all about. \n\nIn other words, at least for the kernel, [PATCH] was a marker that said \n\"the author didn't commit this directly\". Git already has that information \nexplicitly in the git data.\n\n(Also, with proper \"Signed-off-by:\" lines it's also always clear that \nthere were other people involved, and that the author of the patch is \ndifferent from the person who applied it).\n\nSo I would personally not mind if we just made the \"[PATCH]\" prefix go \naway entirely, or perhaps had a separate flag to \"git-applymbox\" that told \nit to prepend a specific prefix to the Subject: line (which might not be \n\"[PATCH] \" at all) defaulting to \"no prefix\".\n\n\t\t\tLinus\n\nPS. Another historical reason for marking patches explicitly was that\npeople were worried that introducing BK would somehow make the old\npatch-based submissions be \"second-class citizens\". It was easy to make\nstatistics and show that at least half the real changes (as opposed to\nmerges) were still patch-based. So again, the \"PATCH\" marker had some \nhistorical relevance, but I don't think it matters any more.\n"},{"id":"7454","messageId":"430375B3.3050307@pobox.com","threadId":"1539","inReplyTo":"7vk6il2jz8.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Jeff Garzik","fromEmail":"jgarzik@pobox.com","sentAt":"2005-08-17T17:36:51Z","receivedAt":"2005-08-17T17:36:51Z","isPatch":true,"sender":{"key":"jgarzik@pobox.com","avatar":null},"body":"\nIf someone is thus motivated, I have two requests in this area:\n\n1) Fix applymbox such that it understands RFC822-valid Subject lines \nwhich wrap across multiple text lines.\n\n2) Teach it to understand MIME, and not treat the MIME headers like part \nof the message.\n"},{"id":"7460","messageId":"7vek8swej5.fsf@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"430375B3.3050307@pobox.com","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-17T19:26:38Z","receivedAt":"2005-08-17T19:26:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Garzik <jgarzik@pobox.com> writes:\n\n> 1) Fix applymbox such that it understands RFC822-valid Subject lines \n> which wrap across multiple text lines.\n\nI thought I did this in mailinfo (read_one_header() function)\nsome time ago.  I'd appreciate it if you could point me at a\nsample message that fails to get processed correctly.\n\n> 2) Teach it to understand MIME, and not treat the MIME headers like part \n> of the message.\n\nI share the same gripe; I always end up running applymbox -q and\nhand-fixing things for this reason.\n"},{"id":"7465","messageId":"Pine.LNX.4.58.0508171253190.3553@g5.osdl.org","threadId":"1539","inReplyTo":"430375B3.3050307@pobox.com","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-17T19:56:50Z","receivedAt":"2005-08-17T19:56:50Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 17 Aug 2005, Jeff Garzik wrote:\n> \n> 1) Fix applymbox such that it understands RFC822-valid Subject lines \n> which wrap across multiple text lines.\n\nIt already should do this.\n\n> 2) Teach it to understand MIME, and not treat the MIME headers like part \n> of the message.\n\nBut this one I really realyl disagree with.\n\nThe fact is, anybody who doesn't edit the emails that come in is BROKEN. \nThere are two kinds of emails:\n\n - the nicely formatted ones where the author follows all the rules\n\n   This kind of email doesn't need MIME decoding anyway.\n\n - the others\n\n   This kind might need MIME decoding, but since it _also_ needs \n   hand-editing to remove all the \"Hi, please apply this patch\" etc crud\n   that inevitably go along with this kind of patch, trying to handle it \n   automatically is WRONG WRONG WRONG.\n\n   And if it's mime-encoded you often have trouble editing it anyway.\n\nErgo: if somebody sends you mime-encoded patches, hit them with a baseball \nbat (politely) and teach them not to do that. \"Fixing\" the tools really \nwill just make things worse if it means that you apply raw emails without \nhaving edited them.\n\n\t\tLinus\n"},{"id":"7471","messageId":"7vfyt8thv2.fsf@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508171253190.3553@g5.osdl.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-17T20:42:57Z","receivedAt":"2005-08-17T20:42:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> The fact is, anybody who doesn't edit the emails that come in is BROKEN. \n> There are two kinds of emails:\n>\n>  - the nicely formatted ones where the author follows all the rules\n>\n>    This kind of email doesn't need MIME decoding anyway.\n\nThat depends on what the rules are, but I consider detecting B\nencodings in the header fields and transliterating it into UTF-8\na good idea (although that sometimes is a lossy conversion\ndepending on the original charset).\n\nRemember this one that prompted me to do the header folding?\n\n    From: YOSHIFUJI Hideaki / =?iso-2022-jp?B?GyRCNUhGIzFRTEAbKEI=?= \n            <yoshfuji@linux-ipv6.org>\n\n>  - the others\n\n>    ..., trying to handle it \n>    automatically is WRONG WRONG WRONG.\n>    And if it's mime-encoded you often have trouble editing it anyway.\n>\n> Ergo: if somebody sends you mime-encoded patches, hit them with a baseball \n> bat (politely) and teach them not to do that. \"Fixing\" the tools really \n> will just make things worse if it means that you apply raw emails without \n> having edited them.\n\nI agree with you in principle and that is why I always run\napplymbox with the -q flag.  Maybe I should start trying the\nbaseball bat approach to see what happens.\n"},{"id":"7476","messageId":"Pine.LNX.4.63.0508172333180.17758@wgmdd8.biozentrum.uni-wuerzburg.de","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508171253190.3553@g5.osdl.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-08-17T21:36:00Z","receivedAt":"2005-08-17T21:36:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 17 Aug 2005, Linus Torvalds wrote:\n\n> On Wed, 17 Aug 2005, Jeff Garzik wrote:\n> > \n> > 2) Teach it to understand MIME, and not treat the MIME headers like part \n> > of the message.\n> \n> [...]\n> \n> Ergo: if somebody sends you mime-encoded patches, hit them with a baseball \n> bat (politely) and teach them not to do that. \"Fixing\" the tools really \n> will just make things worse if it means that you apply raw emails without \n> having edited them.\n\nI'd say that QP and the MIME/alternate formats are not really the fault of \nthe sender, but rather the mailer. So there might be value in supporting \nat least these, to make the life of maintainers easier.\n\nCiao,\nDscho\n"},{"id":"7496","messageId":"u5tbr3vy1pi.fsf@lysator.liu.se","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508171253190.3553@g5.osdl.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2005-08-18T10:32:57Z","receivedAt":"2005-08-18T10:32:57Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Wed, 17 Aug 2005, Jeff Garzik wrote:\n>> \n>> 1) Fix applymbox such that it understands RFC822-valid Subject lines \n>> which wrap across multiple text lines.\n>\n> It already should do this.\n>\n>> 2) Teach it to understand MIME, and not treat the MIME headers like part \n>> of the message.\n>\n> But this one I really realyl disagree with.\n>\n> The fact is, anybody who doesn't edit the emails that come in is BROKEN. \n> There are two kinds of emails:\n>\n>  - the nicely formatted ones where the author follows all the rules\n>\n>    This kind of email doesn't need MIME decoding anyway.\n\nUnless they want to write something that doesn't fit in ASCII, as\ndiscussed in another thread here.\n\nBut maybe you are only talking about MIME attachments, and not about\nMIME content encodings?  We probably need to separate the two.\n\nNote that I'm not really talking about your patch handling for Linux;\nyou are free to disallow my name in Linux patches if you want to.  But\nI'd like to see a way to get rid of that limitation for other uses of\ngit.\n\n-- \nDavid Kågedal\n"},{"id":"7512","messageId":"20050818172646.GA18459@mars.ravnborg.org","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508170830050.3553@g5.osdl.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Sam Ravnborg","fromEmail":"sam@ravnborg.org","sentAt":"2005-08-18T17:26:46Z","receivedAt":"2005-08-18T17:26:46Z","isPatch":true,"sender":{"key":"sam@ravnborg.org","avatar":"https://gravatar.com/avatar/168a912606ed0742d840bb365e3cc21db390c36531a58341dc7a069cc1f15f62?d=mp&s=160"},"body":"> (Also, with proper \"Signed-off-by:\" lines it's also always clear that \n> there were other people involved, and that the author of the patch is \n> different from the person who applied it).\n\nI almost always handedit my mails and I find myself forgetting to add\n\"Signed-off-by\" from time to time.\nIs there a simple way to implment a trigger that can check that _I_\nsigned off the patch before applying it?\n\nI prefer to add it myself rather than to have it added automatically -\nbut mayve thats you me being a bit mistrusting.\n\nBtw. I'm a Cogito user if that makes a difference.\nThe only git- command I use today is git-applymbox.\n\n\tSam\n"},{"id":"7515","messageId":"Pine.LNX.4.58.0508181257320.3412@g5.osdl.org","threadId":"1539","inReplyTo":"20050818172646.GA18459@mars.ravnborg.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-18T20:07:41Z","receivedAt":"2005-08-18T20:07:41Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 18 Aug 2005, Sam Ravnborg wrote:\n> \n> I almost always handedit my mails and I find myself forgetting to add\n> \"Signed-off-by\" from time to time.\n> Is there a simple way to implment a trigger that can check that _I_\n> signed off the patch before applying it?\n\nWell, Junio has been talking about adding commit hooks. I don't think\nthat's been done. The idea being that you could verify that the thing \nyou're committing follows certain rules (no bad whitespace added in the \ndiff, sign-offs in the messages, whatever).\n\nThat said, git-applypatch (which is what git-applymbox ends up calling) \ndoes not use the general \"git commit\" script. So it would have to have its \nown hook. The script is pretty easy to read, though: just look at \ngit-applypatch, and notice that the last stages are:\n\n\t...\n\tgit-apply --index $PATCHFILE || exit 1\n\ttree=$(git-write-tree) || exit 1\n\techo Wrote tree $tree\n\tcommit=$(git-commit-tree $tree -p $(cat .git/HEAD) < $final) || exit 1\n\techo Committed: $commit\n\techo $commit > .git/HEAD\n\nand that just before this thing you could easily add some sanity checking \nby hand. The commit message at that point is in the \"$final\" file, and the \npatch is obviously in $PATCHFILE, so you can verify either of those to \nyour hearts content.\n\nThe only question is what the hook/trigger should look like. just put\nsomething like\n\n\t[ -x .git/hooks/applypatch-hook ] &&\n\t     .git/hooks/applypatch-hook \"$tree\" \"$PATCHFILE\" || exit\n\nat the line before that \"git-apply\" perhaps? Then, you could install your \nown applypatch hook which looks at the message or the patch?\n\n\t\tLinus\n"},{"id":"7533","messageId":"7vfyt6enyq.fsf@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"20050818172646.GA18459@mars.ravnborg.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-19T01:04:45Z","receivedAt":"2005-08-19T01:04:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Ravnborg <sam@ravnborg.org> writes:\n\n> I prefer to add it myself rather than to have it added automatically -\n> but mayve thats you me being a bit mistrusting.\n>\n> The only git- command I use today is git-applymbox.\n\nIf you did not have that \"add it myself\" preference, I would\nhave recommended the (not counting the flags) second parameter\nto git-applymbox.\n\nWhile we are on the topic of applymbox, currently it takes this\nform:\n\n$ applymbox [ -k ] [ -q ] (-c .dotest/msg_num | mail_archive) [Signoff_file]\"\n\nIt may make more sense to change it to:\n\n    applymbox [-k] [-q] [-s <signoff>] ( -c .dotest/<msg_num> | <mbox>... )\n"},{"id":"7534","messageId":"7vacjeenyi.fsf@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508181257320.3412@g5.osdl.org","subject":"Re: [PATCH] Teach applymbox to keep the Subject: line.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-19T01:04:53Z","receivedAt":"2005-08-19T01:04:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Well, Junio has been talking about adding commit hooks. I don't think\n> that's been done.\n\nNo, notyet.  But tonight.\n\n> The only question is what the hook/trigger should look like. just put\n> something like\n>\n> \t[ -x .git/hooks/applypatch-hook ] &&\n> \t     .git/hooks/applypatch-hook \"$tree\" \"$PATCHFILE\" || exit\n>\n> at the line before that \"git-apply\" perhaps? Then, you could install your \n> own applypatch hook which looks at the message or the patch?\n\nSounds sensible.  The hook would probably want a handy access to\nthe commit object as well to catch:\n\n     - the author name may be spelled wrong or has funky\n       B-encodings still left.\n\n     - some people might want to even run spellchecker on the\n       commit message.\n\n     - lack of S-O-B line.\n"},{"id":"7576","messageId":"7v4q9lvc2p.fsf_-_@assigned-by-dhcp.cox.net","threadId":"1539","inReplyTo":"Pine.LNX.4.58.0508181257320.3412@g5.osdl.org","subject":"[PATCH] Add hooks to tools/git-applypatch.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-19T21:41:50Z","receivedAt":"2005-08-19T21:41:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This teachs git-applypatch, which is used from git-applymbox, three\nhooks, similar to what git-commit-script uses.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n    This is still in the proposed updates branch, along with the\n    hooks for \"git commit\".  Are people comfortable with the\n    three-hook arrangements these two commands use?\n\n templates/hooks--applypatch-msg |   14 ++++++\n templates/hooks--pre-applypatch |   14 ++++++\n tools/git-applypatch            |   87 +++++++++++++++++++++++++++++++--------\n 3 files changed, 97 insertions(+), 18 deletions(-)\n create mode 100644 templates/hooks--applypatch-msg\n create mode 100644 templates/hooks--pre-applypatch\n\nb5df0d94bf045627e74cf7faef3f51ce5e567aa4\ndiff --git a/templates/hooks--applypatch-msg b/templates/hooks--applypatch-msg\nnew file mode 100644\n--- /dev/null\n+++ b/templates/hooks--applypatch-msg\n@@ -0,0 +1,14 @@\n+#!/bin/sh\n+#\n+# An example hook script to check the commit log message taken by\n+# applypatch from an e-mail message.\n+#\n+# The hook should exit with non-zero status after issuing an\n+# appropriate message if it wants to stop the commit.  The hook is\n+# allowed to edit the commit message file.\n+#\n+# To enable this hook, make this file executable.\n+\n+test -x \"$GIT_DIR/hooks/commit-msg\" &&\n+\texec \"$GIT_DIR/hooks/commit-msg\" ${1+\"$@\"}\n+:\ndiff --git a/templates/hooks--pre-applypatch b/templates/hooks--pre-applypatch\nnew file mode 100644\n--- /dev/null\n+++ b/templates/hooks--pre-applypatch\n@@ -0,0 +1,14 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed\n+# by applypatch from an e-mail message.\n+#\n+# The hook should exit with non-zero status after issuing an\n+# appropriate message if it wants to stop the commit.\n+#\n+# To enable this hook, make this file executable.\n+\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+\texec \"$GIT_DIR/hooks/pre-commit\" ${1+\"$@\"}\n+:\n+\ndiff --git a/tools/git-applypatch b/tools/git-applypatch\n--- a/tools/git-applypatch\n+++ b/tools/git-applypatch\n@@ -10,58 +10,109 @@\n ##\t$3 - \"info\" file with Author, email and subject\n ##\t$4 - optional file containing signoff to add\n ##\n-signoff=\"$4\"\n+. git-sh-setup-script || die \"Not a git archive.\"\n+\n final=.dotest/final-commit\n ##\n ## If this file exists, we ask before applying\n ##\n query_apply=.dotest/.query_apply\n+\n+## We do not munge the first line of the commit message too much\n+## if this file exists.\n keep_subject=.dotest/.keep_subject\n+\n+\n MSGFILE=$1\n PATCHFILE=$2\n INFO=$3\n-EDIT=${VISUAL:-$EDITOR}\n-EDIT=${EDIT:-vi}\n+SIGNOFF=$4\n+EDIT=${VISUAL:-${EDITOR:-vi}}\n \n export GIT_AUTHOR_NAME=\"$(sed -n '/^Author/ s/Author: //p' .dotest/info)\"\n export GIT_AUTHOR_EMAIL=\"$(sed -n '/^Email/ s/Email: //p' .dotest/info)\"\n export GIT_AUTHOR_DATE=\"$(sed -n '/^Date/ s/Date: //p' .dotest/info)\"\n export SUBJECT=\"$(sed -n '/^Subject/ s/Subject: //p' .dotest/info)\"\n \n-if [ -n \"$signoff\" -a -f \"$signoff\" ]; then\n-\tcat $signoff >> $MSGFILE\n+if test '' != \"$SIGNOFF\"\n+then\n+\tif test -f \"$SIGNOFF\"\n+\tthen\n+\t\tSIGNOFF=`cat \"$SIGNOFF\"` || exit\n+\telif case \"$SIGNOFF\" in yes | true | me | please) : ;; *) false ;; esac\n+\tthen\n+\t\tSIGNOFF=`git-var GIT_COMMITTER_IDENT | sed -e '\n+\t\t\t\ts/>.*/>/\n+\t\t\t\ts/^/Signed-off-by: /'\n+\t\t`\n+\telse\n+\t\tSIGNOFF=\n+\tfi\n+\tif test '' != \"$SIGNOFF\"\n+\tthen\n+\t\tLAST_SIGNED_OFF_BY=`\n+\t\t\tsed -ne '/^Signed-off-by: /p' \"$MSGFILE\" |\n+\t\t\ttail -n 1\n+\t\t`\n+\t\ttest \"$LAST_SIGNED_OFF_BY\" = \"$SIGNOFF\" ||\n+\t\techo \"$SIGNOFF\" >>\"$MSGFILE\"\n+\tfi\n fi\n+\n patch_header=\n test -f \"$keep_subject\" || patch_header='[PATCH] '\n \n-(echo \"$patch_header$SUBJECT\" ; if [ -s $MSGFILE ]; then echo ; cat $MSGFILE; fi ) > $final\n+{\n+\techo \"$patch_header$SUBJECT\"\n+\tif test -s \"$MSGFILE\"\n+\tthen\n+\t\techo\n+\t\tcat \"$MSGFILE\"\n+\tfi\n+} >\"$final\"\n \n-f=0\n-[ -f $query_apply ] || f=1\n+interactive=yes\n+test -f \"$query_apply\" || interactive=no\n \n-while [ $f -eq 0 ]; do\n+while [ \"$interactive\" = yes ]; do\n \techo \"Commit Body is:\"\n \techo \"--------------------------\"\n-\tcat $final\n+\tcat \"$final\"\n \techo \"--------------------------\"\n \techo -n \"Apply? [y]es/[n]o/[e]dit/[a]ccept all \"\n \tread reply\n-\tcase $reply in\n-\t\ty|Y) f=1;;\n+\tcase \"$reply\" in\n+\t\ty|Y) interactive=no;;\n \t\tn|N) exit 2;;\t# special value to tell dotest to keep going\n-\t\te|E) $EDIT $final;;\n-\t\ta|A) rm -f $query_apply\n-\t\t     f=1;;\n+\t\te|E) \"$EDIT\" \"$final\";;\n+\t\ta|A) rm -f \"$query_apply\"\n+\t\t     interactive=no ;;\n \tesac\n done\n \n+if test -x \"$GIT_DIR\"/hooks/applypatch-msg\n+then\n+\t\"$GIT_DIR\"/hooks/applypatch-msg \"$final\" || exit\n+fi\n+\n echo\n echo Applying \"'$SUBJECT'\"\n echo\n \n-git-apply --index $PATCHFILE || exit 1\n+git-apply --index \"$PATCHFILE\" || exit 1\n+\n+if test -x \"$GIT_DIR\"/hooks/pre-applypatch\n+then\n+\t\"$GIT_DIR\"/hooks/pre-applypatch || exit\n+fi\n+\n tree=$(git-write-tree) || exit 1\n echo Wrote tree $tree\n-commit=$(git-commit-tree $tree -p $(cat .git/HEAD) < $final) || exit 1\n+commit=$(git-commit-tree $tree -p $(cat \"$GIT_DIR\"/HEAD) < \"$final\") || exit 1\n echo Committed: $commit\n-echo $commit > .git/HEAD\n+echo $commit > \"$GIT_DIR\"/HEAD\n+\n+if test -x \"$GIT_DIR\"/hooks/post-applypatch\n+then\n+\t\"$GIT_DIR\"/hooks/post-applypatch\n+fi\n"}]}