{"thread":{"id":"52382","subject":"[PATCH v2] contrib: git-cpcover: copy cover letter","startedAt":"2019-12-03T20:13:36Z","lastAt":"2019-12-09T15:49:41Z","messageCount":8,"participants":["Michael S. Tsirkin","Jonathan Nieder","Eric Sunshine","Denton Liu","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"387465","messageId":"20191203201233.661696-1-mst@redhat.com","threadId":"52382","inReplyTo":null,"subject":"[PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2019-12-03T20:13:27Z","receivedAt":"2019-12-03T20:13:36Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"My flow looks like this:\n1. git format-patch -v --cover-letter -o \n2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n\ncopy subject and blurb, avoiding patchset stats\n\n3. add changelog update blurb as appropriate\n\n4. git send-email /v-*\n\nThe following perl script automates step 2 above. Hacked together\nrather quickly, so I'm only proposing it for contrib for now. If others\nsee the need, we can add docs, tests and move it to git proper.\n\nSigned-off-by: Michael S. Tsirkin \n---\n\nFixes from v1: support multi-line To/Cc headers.\n\nAny feedback on this? Interest in taking this into contrib/ for now?\n\n contrib/git-cpcover | 84 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 84 insertions(+)\n create mode 100755 contrib/git-cpcover\n\ndiff --git a/contrib/git-cpcover b/contrib/git-cpcover\nnew file mode 100755\nindex 0000000000..fe7006a56d\n--- /dev/null\n+++ b/contrib/git-cpcover\n@@ -0,0 +1,84 @@\n+#!/usr/bin/perl -i\n+\n+use strict;\n+\n+die \"Usage: ${0} []\" unless $#ARGV == 0 or $#ARGV == 1;\n+\n+my $ffrom = shift @ARGV;\n+my @extraheaders = ();\n+\n+open(FROM, \"<\", $ffrom) || die \"Can not open $ffrom\";\n+\n+my @from = ();\n+while () {\n+\tpush @from, $_;\n+}\n+\n+close(FROM) || die \"error closing $ffrom\";\n+\n+#get subject\n+my $subj;\n+my $bodyi;\n+my $lastheader=\"\";\n+for (my $i = 0; $i <= $#from; $i++) {\n+\t$_ = $from[$i];\n+\t#print STDERR \"<$line>\\n\";\n+\tif (not defined ($subj) and s/^Subject: \\[[^]]+\\] //) {\n+\t\t$subj = $_;\n+\t\tchomp $subj;\n+\t}\n+\tif (m/^([A-Za-z0-9-_]*:)/) {\n+\t\t$lastheader = $1;\n+\t}\n+\tif (m/^(To|Cc):/ or (m/^\\s/ and $lastheader =~ m/^(To|Cc):/)) {\n+\t\tpush @extraheaders, $from[$i];\n+\t}\n+\tif (defined ($subj) and m/^$/) {\n+\t\t$bodyi = $i + 1;\n+\t\tlast;\n+\t}\n+}\n+\n+die \"No subject found in $ffrom\" unless defined($subj);\n+\n+die \"No body found in $ffrom\" unless defined($bodyi);\n+\n+my $bodyl;\n+my $statb;\n+my $state;\n+for (my $i = $#from; $i >= $bodyi; $i--) {\n+\t$_ = $from[$i];\n+\t$statb = $i if m/ [0-9]+ files changed, [0-9]+ insertions\\(\\+\\), [0-9]+ deletions\\(-\\)/;\n+\tnext unless defined($statb);\n+\t$state = $i if m/^$/;\n+\tnext unless defined($state);\n+\tnext if m/^$/;\n+\tnext if m/^ [^ ]/;\n+\tnext if m/\\([0-9]+\\):$/;\n+\t$bodyl = $i;\n+\tlast;\n+}\n+\n+die \"No body found in $ffrom\" unless defined($bodyl);\n+\n+#print STDERR $bodyi, \"-\", $bodyl, \"\\n\";\n+my $blurb = join(\"\", @from[$bodyi..$bodyl]);\n+\n+my $gotsubj = 0;\n+my $gotblurb = 0;\n+my $gotendofheaders = 0;\n+while (<>) {\n+\tif (not $gotsubj and\n+\t s/\\*\\*\\* SUBJECT HERE \\*\\*\\*/$subj/) {\n+\t\t$gotsubj = 1;\n+\t}\n+\tif (not $gotblurb and\n+\t s/\\*\\*\\* BLURB HERE \\*\\*\\*/$blurb/) {\n+\t\t$gotblurb = 1;\n+\t}\n+\tif (not $gotendofheaders and m/^$/) {\n+\t\tprint join(\"\", @extraheaders);\n+\t\t$gotendofheaders = 1;\n+\t}\n+\tprint $_;\n+}\n-- \nMST\n\n"},{"id":"387474","messageId":"20191204044449.GB226135@google.com","threadId":"52382","inReplyTo":"20191203201233.661696-1-mst@redhat.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-12-04T04:44:49Z","receivedAt":"2019-12-04T04:44:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMichael S. Tsirkin wrote:\n\n> My flow looks like this:\n> 1. git format-patch -v --cover-letter -o \n> 2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n>\n> copy subject and blurb, avoiding patchset stats\n>\n> 3. add changelog update blurb as appropriate\n>\n> 4. git send-email /v-*\n>\n> The following perl script automates step 2 above.\n\nNeat. I wonder, should \"git format-patch\" learn an option for this?\nE.g.\n\n\tgit format-patch -v --cover-letter \\\n\t\t--last-cover-letter=/v-0000-cover-letter.patch \\\n\t\t-o \n\nWhat would your ideal interface for this flow look like?\n\n[...]\n> Any feedback on this? Interest in taking this into contrib/ for now?\n\nI don't know what Junio's preferences are for new contrib/\ncontributions, but I kind of like it. If putting it in contrib/, my\nmain advice would be to put it in a subdirectory there with a README.\nThat way, we have a good place to document what it was replaced by\nonce it has graduated to a standard format-patch feature.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"387475","messageId":"CAPig+cTFbpAo5+kahLT+7E1zQe24S5icm0SSB=HF4xqsD2VdAA@mail.gmail.com","threadId":"52382","inReplyTo":"20191204044449.GB226135@google.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-12-04T05:18:28Z","receivedAt":"2019-12-04T05:18:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Dec 3, 2019 at 11:45 PM Jonathan Nieder wrote:\n> Michael S. Tsirkin wrote:\n> > My flow looks like this:\n> > 2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n> > copy subject and blurb, avoiding patchset stats\n> > 3. add changelog update blurb as appropriate\n> >\n> > The following perl script automates step 2 above.\n>\n> Neat. I wonder, should \"git format-patch\" learn an option for this?\n> git format-patch -v --cover-letter \\\n> --last-cover-letter=/v-0000-cover-letter.patch \\\n> -o \n\nThat was my first thought, as well, although, as this has similar\npurpose to the new git-format-patch --cover-from-description= option,\nperhaps a more suitable name might be --copy-cover-from= or something?\n\nI could even imagine a new option -V which has the combined effect\nof setting the re-roll count (like -v) and automagically copying the\ncover letter material from cover letter v located in .\n"},{"id":"387478","messageId":"20191204065846.GA3386115@generichostname","threadId":"52382","inReplyTo":"20191203201233.661696-1-mst@redhat.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-12-04T06:58:46Z","receivedAt":"2019-12-04T06:59:21Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Michael,\n\nAs others have been talking about in a sibling thread, I'd love to see\nsomething like this incorporated into format-patch itself. I considered\ndoing something like this myself but my workflow ended up becoming \"good\nenough\" with --cover-from-description that I never bothered to look into\nit further.\n\nOn Tue, Dec 03, 2019 at 03:13:27PM -0500, Michael S. Tsirkin wrote:\n> My flow looks like this:\n> 1. git format-patch -v --cover-letter -o \n> 2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n> \n> copy subject and blurb, avoiding patchset stats\n> \n> 3. add changelog update blurb as appropriate\n> \n> 4. git send-email /v-*\n> \n> The following perl script automates step 2 above. Hacked together\n> rather quickly, so I'm only proposing it for contrib for now. If others\n> see the need, we can add docs, tests and move it to git proper.\n> \n> Signed-off-by: Michael S. Tsirkin \n> ---\n> \n> Fixes from v1: support multi-line To/Cc headers.\n> \n> Any feedback on this? Interest in taking this into contrib/ for now?\n\nI took a brief look at it and I have one small suggestion. Would it be\npossible to grab the last Message-Id and use it to generate the\nIn-Reply-To header using this?\n\nThanks,\n\nDenton\n"},{"id":"387491","messageId":"xmqqlfrs5acs.fsf@gitster-ct.c.googlers.com","threadId":"52382","inReplyTo":"CAPig+cTFbpAo5+kahLT+7E1zQe24S5icm0SSB=HF4xqsD2VdAA@mail.gmail.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-04T16:22:43Z","receivedAt":"2019-12-04T16:22:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Dec 3, 2019 at 11:45 PM Jonathan Nieder wrote:\n>> Michael S. Tsirkin wrote:\n>> > My flow looks like this:\n>> > 2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n>> > copy subject and blurb, avoiding patchset stats\n>> > 3. add changelog update blurb as appropriate\n>> >\n>> > The following perl script automates step 2 above.\n>>\n>> Neat. I wonder, should \"git format-patch\" learn an option for this?\n>> git format-patch -v --cover-letter \\\n>> --last-cover-letter=/v-0000-cover-letter.patch \\\n>> -o \n>\n> That was my first thought, as well, although, as this has similar\n> purpose to the new git-format-patch --cover-from-description= option,\n> perhaps a more suitable name might be --copy-cover-from= or something?\n>\n> I could even imagine a new option -V which has the combined effect\n> of setting the re-roll count (like -v) and automagically copying the\n> cover letter material from cover letter v located in .\n\nI actually looked into doing something similar but without any new\noption (i.e. unconditionally --cover-letter with -v would check\nfor v-0000-cover.letter and does the right thing) some time\nago. I do not recall why I gave up (not that I tried very hard),\nbut IIRC, the current reroll-count was not passed down in the\ncallchain to make_cover_letter() to do this.\n\nBut I think that was even before we integrated the range-diff stuff,\nwhich does seem to use the \"given we are doing , let's compare\nwith \" thing, so perhaps it is not too difficult.\n\nI am just saying that I think the change would not have to be opt-in,\nbut can be unconditionally made, simply because replacing the BLURB\nHERE placeholder with *anything* written by human user previously is\na 100% improvement ;-)\n\nThanks.\n"},{"id":"387492","messageId":"CAPig+cREF8BSVbCoOUaRMPOyfD_bfD5PhxgM4QZgot7sziCNug@mail.gmail.com","threadId":"52382","inReplyTo":"xmqqlfrs5acs.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-12-04T16:32:20Z","receivedAt":"2019-12-04T16:32:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 4, 2019 at 11:23 AM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > I could even imagine a new option -V which has the combined effect\n> > of setting the re-roll count (like -v) and automagically copying the\n> > cover letter material from cover letter v located in .\n>\n> I actually looked into doing something similar but without any new\n> option (i.e. unconditionally --cover-letter with -v would check\n> for v-0000-cover.letter and does the right thing) some time\n> ago.\n\nYes, I like that better than a new option, and wanted to suggest it as\nwell, however... (see below)\n\n> But I think that was even before we integrated the range-diff stuff,\n> which does seem to use the \"given we are doing , let's compare\n> with \" thing, so perhaps it is not too difficult.\n\nYup.\n\n> I am just saying that I think the change would not have to be opt-in,\n> but can be unconditionally made, simply because replacing the BLURB\n> HERE placeholder with *anything* written by human user previously is\n> a 100% improvement ;-)\n\nI had started writing the same in my previous reply but then realized\nthat it could break existing tooling which uses -v and --cover-letter\ntogether and which searches for the well-known BLURB HERE placeholder\nto replace it automatically. If I'm wrong about possibly breaking\nexisting tooling, then I'd also vote for this behavior kicking in\nautomatically with -v and --cover-letter specified together.\n"},{"id":"387494","messageId":"xmqqd0d458ar.fsf@gitster-ct.c.googlers.com","threadId":"52382","inReplyTo":"CAPig+cREF8BSVbCoOUaRMPOyfD_bfD5PhxgM4QZgot7sziCNug@mail.gmail.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-04T17:07:08Z","receivedAt":"2019-12-04T17:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Wed, Dec 4, 2019 at 11:23 AM Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>> > I could even imagine a new option -V which has the combined effect\n>> > of setting the re-roll count (like -v) and automagically copying the\n>> > cover letter material from cover letter v located in .\n>>\n>> I actually looked into doing something similar but without any new\n>> option (i.e. unconditionally --cover-letter with -v would check\n>> for v-0000-cover.letter and does the right thing) some time\n>> ago.\n>\n> Yes, I like that better than a new option, and wanted to suggest it as\n> well, however... (see below)\n>\n>> But I think that was even before we integrated the range-diff stuff,\n>> which does seem to use the \"given we are doing , let's compare\n>> with \" thing, so perhaps it is not too difficult.\n>\n> Yup.\n>\n>> I am just saying that I think the change would not have to be opt-in,\n>> but can be unconditionally made, simply because replacing the BLURB\n>> HERE placeholder with *anything* written by human user previously is\n>> a 100% improvement ;-)\n>\n> I had started writing the same in my previous reply but then realized\n> that it could break existing tooling which uses -v and --cover-letter\n> together and which searches for the well-known BLURB HERE placeholder\n> to replace it automatically. If I'm wrong about possibly breaking\n> existing tooling, then I'd also vote for this behavior kicking in\n> automatically with -v and --cover-letter specified together.\n\nWell, they can now disable that \"copy over the material from the\nprevious\" step, which is good. I actually think we should add the\nwell-known BLURB HERE string *after* populating the new version of\nthe coer letter with the materials from old one to *force* users to\nproofread and adjust it to the updated reality, so if that happens,\nthen the existing tooling would also work as before ;-)\n"},{"id":"387791","messageId":"20191209102727-mutt-send-email-mst@kernel.org","threadId":"52382","inReplyTo":"20191204044449.GB226135@google.com","subject":"Re: [PATCH v2] contrib: git-cpcover: copy cover letter","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2019-12-09T15:49:29Z","receivedAt":"2019-12-09T15:49:41Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Dec 03, 2019 at 08:44:49PM -0800, Jonathan Nieder wrote:\n> Hi,\n> \n> Michael S. Tsirkin wrote:\n> \n> > My flow looks like this:\n> > 1. git format-patch -v --cover-letter -o \n> > 2. vi /v-0000-cover-letter.patch /v-0000-cover-letter.patch\n> >\n> > copy subject and blurb, avoiding patchset stats\n> >\n> > 3. add changelog update blurb as appropriate\n> >\n> > 4. git send-email /v-*\n> >\n> > The following perl script automates step 2 above.\n> \n> Neat. I wonder, should \"git format-patch\" learn an option for this?\n> E.g.\n> \n> \tgit format-patch -v --cover-letter \\\n> \t\t--last-cover-letter=/v-0000-cover-letter.patch \\\n> \t\t-o \n> \n> What would your ideal interface for this flow look like?\n\nI use it in several ways\n- new version - generate a new version from previous one\n\tgit format-patch -o patches/series ....\n\tgit cpcover patches/series/saved-cover-letter.patch patches/series/vN-cover-letter.patch\n\n\tSo ideally it would automatically pick up vN-1 cover letter if it's there.\n\n- update - I generate patches but don't post yet.\n\twrite a cover letter with e.g. an explanation, decide\n\tto make some changes before posting. Then:\n\tcp patches/series/vN-cover-letter.patch patches/series/saved-cover-letter.patch\n\tgit format-patch -o patches/series ....\n\tgit cpcover patches/series/saved-cover-letter.patch patches/series/vN-cover-letter.patch\n\n\n\tSo ideally if cover letter already exists, ask whether to copy it.\n\tor at least whether it's ok to over-write it...\n\n- series history\n\tI start with a non-versioned cover letter,\n and maintain it in a directory:\n\tcp patches/series/0000-cover-letter.patch patches/series/cover-letter.patch\n\tThis is where\n\tI then put notes about design changes, list questions\n\tto be answered, add people who contributed\n\tto the discussion and should be Cc'd on new versions\n\tThen each time I do\n\tgit format-patch -o patches/series ....\n\tgit cpcover patches/series/cover-letter.patch patches/series/vN-cover-letter.patch\n- public-inbox\n\tget series from public inbox (from someone else\n\tor myself) and work on it\n\n\tIdeally if cover-letter without a version exists, pick it up.\n\nDuring all of this, it's important to pick up description, subject, to/cc.\n\nAlso one thing I need to remember to do is update changelog.\n\nIf there's a standard way to document version changes, ideally it will\nbe possible to check whether latest version changes have been\ndocumented, and print a reminder if not.\n\n\n> [...]\n> > Any feedback on this? Interest in taking this into contrib/ for now?\n> \n> I don't know what Junio's preferences are for new contrib/\n> contributions, but I kind of like it. If putting it in contrib/, my\n> main advice would be to put it in a subdirectory there with a README.\n> That way, we have a good place to document what it was replaced by\n> once it has graduated to a standard format-patch feature.\n> \n> Thanks and hope that helps,\n> Jonathan\n\nOK so contrib/format-patch/ ?\n\n-- \nMST\n\n"}]}