{"thread":{"id":"59033","subject":"[PATCH] hooks: add sendemail-validate-series","startedAt":"2023-01-03T23:11:41Z","lastAt":"2023-04-03T00:05:59Z","messageCount":7,"participants":["Robin Jarry","Tim Culverhouse","Nicolas Dichtel","Bagas Sanjaya","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469777","messageId":"20230103231133.64050-1-robin@jarry.cc","threadId":"59033","inReplyTo":null,"subject":"[PATCH] hooks: add sendemail-validate-series","fromName":"Robin Jarry","fromEmail":"robin@jarry.cc","sentAt":"2023-01-03T23:11:33Z","receivedAt":"2023-01-03T23:11:41Z","isPatch":true,"sender":{"key":"robin@jarry.cc","avatar":"https://avatars.githubusercontent.com/u/472286?v=4"},"body":"When sending patch series (with a cover-letter or not)\nsendemail-validate is called with every email/patch file independently\nfrom the others. When the one of the patches depend on a previous one to\napply, it may not be possible to use this hook in a meaningful way.\n\nChanging sendemail-validate to take all patches as arguments would break\nbackward compatibility.\n\nAdd a new hook to allow validating patch series instead of patch by\npatch. The patch files are provided in the hook script standard input.\n\ngit hook run cannot be used since it closes the hook standard input. Run\nthe hook directly.\n\nSigned-off-by: Robin Jarry \n---\n Documentation/git-send-email.txt | 1 +\n Documentation/githooks.txt | 17 +++++++++++++\n git-send-email.perl | 42 ++++++++++++++++++++++++++++++++\n 3 files changed, 60 insertions(+)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 765b2df8530d..45113b928593 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -438,6 +438,7 @@ have been specified, in which case default to 'compose'.\n +\n --\n \t\t*\tInvoke the sendemail-validate hook if present (see linkgit:githooks[5]).\n+\t\t*\tInvoke the sendemail-validate-series hook if present (see linkgit:githooks[5]).\n \t\t*\tWarn of patches that contain lines longer than\n \t\t\t998 characters unless a suitable transfer encoding\n \t\t\t('auto', 'base64', or 'quoted-printable') is used;\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c8e..e2dc0b49eda5 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -588,6 +588,23 @@ the name of the file that holds the e-mail to be sent. Exiting with a\n non-zero status causes `git send-email` to abort before sending any\n e-mails.\n \n+sendemail-validate-series\n+~~~~~~~~~~~~~~~~~~~~~~~~~\n+\n+This hook is invoked by linkgit:git-send-email[1]. It allows performing\n+validation on a complete patch series at once, instead of patch by patch with\n+`sendemail-validate`.\n+\n+`sendemail-validate-series` takes no arguments, but for each e-mail to be sent\n+it receives on standard input a line of the format:\n+\n+ LF\n+\n+where `` is a name of a file that holds an e-mail to be sent,\n+\n+If the hook exits with non-zero status, `git send-email` will abort before\n+sending any e-mails.\n+\n fsmonitor-watchman\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5861e99a6eb2..30bce599b565 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -793,6 +793,7 @@ sub is_format_patch_arg {\n \t\t\tvalidate_patch($f, $target_xfer_encoding);\n \t\t}\n \t}\n+\tvalidate_patch_series(@files)\n }\n \n if (@files) {\n@@ -2118,6 +2119,47 @@ sub validate_patch {\n \treturn;\n }\n \n+sub validate_patch_series {\n+\tmy @files = @_;\n+\n+\tunless ($repo) {\n+\t\treturn;\n+\t}\n+\n+\tmy $hook_name = 'sendemail-validate-series';\n+\tmy $hooks_path = $repo->command_oneline('rev-parse', '--git-path', 'hooks');\n+\trequire File::Spec;\n+\tmy $validate_hook = File::Spec->catfile($hooks_path, $hook_name);\n+\tmy $hook_error;\n+\tunless (-x $validate_hook) {\n+\t\treturn;\n+\t}\n+\n+\t# The hook needs a correct cwd and GIT_DIR.\n+\trequire Cwd;\n+\tmy $cwd_save = Cwd::getcwd();\n+\tchdir($repo->wc_path() or $repo->repo_path()) or die(\"chdir: $!\");\n+\tlocal $ENV{\"GIT_DIR\"} = $repo->repo_path();\n+\t# cannot use git hook run, it closes stdin before forking the hook\n+\topen(my $stdin, \"|-\", $validate_hook) or die(\"fork: $!\");\n+\tchdir($cwd_save) or die(\"chdir: $!\");\n+\tfor my $fn (@files) {\n+\t\tunless (-p $fn) {\n+\t\t\t$fn = Cwd::abs_path($fn);\n+\t\t\t$stdin->print(\"$fn\\n\");\n+\t\t}\n+\t}\n+\tclose($stdin); # calls waitpid\n+\tif ($? & 0x7f) {\n+\t\tmy $sig = $? & 0x7f;\n+\t\tdie(\"fatal: hook $hook_name killed by signal $sig\")\n+\t} elsif ($? >> 8) {\n+\t\tmy $err = $? >> 8;\n+\t\tdie(\"fatal: hook $hook_name rejected patch series (exit code $err)\")\n+\t}\n+\treturn;\n+}\n+\n sub handle_backup {\n \tmy ($last, $lastlen, $file, $known_suffix) = @_;\n \tmy ($suffix, $skip);\n-- \n2.39.0\n\n"},{"id":"470795","messageId":"CPX5VXD0CIF1.20UPXAWFLGISQ@ringo","threadId":"59033","inReplyTo":"20230103231133.64050-1-robin@jarry.cc","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Robin Jarry","fromEmail":"robin@jarry.cc","sentAt":"2023-01-20T16:34:53Z","receivedAt":"2023-01-20T16:36:11Z","isPatch":true,"sender":{"key":"robin@jarry.cc","avatar":"https://avatars.githubusercontent.com/u/472286?v=4"},"body":"Hi all,\n\nThis is a gentle bump to see if that feature could be of interest for\nanyone.\n\nThanks in advance.\n"},{"id":"472780","messageId":"CQTEEHZA7IWG.I7NYXWYIBLBE@spunky","threadId":"59033","inReplyTo":"20230103231133.64050-1-robin@jarry.cc","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Tim Culverhouse","fromEmail":"tim@timculverhouse.com","sentAt":"2023-02-27T14:03:05Z","receivedAt":"2023-02-27T14:03:15Z","isPatch":true,"sender":{"key":"tim@timculverhouse.com","avatar":null},"body":"Hey everyone -\n\nBumping this one again and adding my name as someone who would greatly benefit\nfrom this feature.\n\n-- \nTim Culverhouse\n"},{"id":"474503","messageId":"7c2cf6dd-8586-af27-f4a1-eb8902de2344@6wind.com","threadId":"59033","inReplyTo":"20230103231133.64050-1-robin@jarry.cc","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Nicolas Dichtel","fromEmail":"nicolas.dichtel@6wind.com","sentAt":"2023-03-31T13:50:51Z","receivedAt":"2023-03-31T13:51:02Z","isPatch":true,"sender":{"key":"nicolas.dichtel@6wind.com","avatar":null},"body":"Le 04/01/2023 à 00:11, Robin Jarry a écrit :\n> When sending patch series (with a cover-letter or not)\n> sendemail-validate is called with every email/patch file independently\n> from the others. When the one of the patches depend on a previous one to\n> apply, it may not be possible to use this hook in a meaningful way.\n> \n> Changing sendemail-validate to take all patches as arguments would break\n> backward compatibility.\n> \n> Add a new hook to allow validating patch series instead of patch by\n> patch. The patch files are provided in the hook script standard input.\n> \n> git hook run cannot be used since it closes the hook standard input. Run\n> the hook directly.\n> \n> Signed-off-by: Robin Jarry \n\nAny news on this patch?\n\n\nRegards,\nNicolas\n"},{"id":"474547","messageId":"9a0948e2-d030-ac89-a009-9247054f71f9@gmail.com","threadId":"59033","inReplyTo":"20230103231133.64050-1-robin@jarry.cc","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2023-04-01T02:54:08Z","receivedAt":"2023-04-01T02:54:16Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 1/4/23 06:11, Robin Jarry wrote:\n> +sendemail-validate-series\n> +~~~~~~~~~~~~~~~~~~~~~~~~~\n> +\n> +This hook is invoked by linkgit:git-send-email[1]. It allows performing\n> +validation on a complete patch series at once, instead of patch by patch with\n> +`sendemail-validate`.\n> +\n> +`sendemail-validate-series` takes no arguments, but for each e-mail to be sent\n> +it receives on standard input a line of the format:\n> +\n> + LF\n> +\n> +where `` is a name of a file that holds an e-mail to be sent,\n> +\n\nIn most cases, the patch series is generated by git-format-patch(1). When the\ncommand is run, it will output:\n\n```\n$ git format-patch -o /tmp --cover-letter --base= \n/tmp/0000-cover-letter.patch\n/tmp/0001-.patch\n/tmp/0002-.patch\n/tmp/0003-.patch\n...\n```\n\nThe output can be fed to the hook (as you write).\n\nBut I think the hook should also take patch file arguments, for the sake of\ncompleteness with sendemail-validate hook; that is:\n\n```\nsendemail-validate-series ...\n```\n\nAlso, there should have a check that In-Reply-To must be the first patch in\nthe given series or the cover letter (if there is one).\n\nAnyway, rather than pinging by random people, I'd like to see [PATCH RESEND],\nrebased on latest git.git tree, ideally with Junio Cc'ed.\n\nThanks.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n\n"},{"id":"474665","messageId":"CRMHVBPXG6SW.2DL6RVNEMVERF@ringo","threadId":"59033","inReplyTo":"9a0948e2-d030-ac89-a009-9247054f71f9@gmail.com","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Robin Jarry","fromEmail":"robin@jarry.cc","sentAt":"2023-04-02T18:50:56Z","receivedAt":"2023-04-02T18:51:03Z","isPatch":true,"sender":{"key":"robin@jarry.cc","avatar":"https://avatars.githubusercontent.com/u/472286?v=4"},"body":"Hi,\n\nBagas Sanjaya, Apr 01, 2023 at 04:54:\n> In most cases, the patch series is generated by git-format-patch(1).\n> When the command is run, it will output:\n>\n> ```\n> $ git format-patch -o /tmp --cover-letter --base= \n> /tmp/0000-cover-letter.patch\n> /tmp/0001-.patch\n> /tmp/0002-.patch\n> /tmp/0003-.patch\n> ...\n> ```\n>\n> The output can be fed to the hook (as you write).\n>\n> But I think the hook should also take patch file arguments, for the\n> sake of completeness with sendemail-validate hook; that is:\n>\n> ```\n> sendemail-validate-series ...\n> ```\n\nI don't mind adding this but I am concerned with the maximum size of the\ncommand line arguments when sending large series. Standard input seems\nlike a safer solution.\n\n> Also, there should have a check that In-Reply-To must be the first\n> patch in the given series or the cover letter (if there is one).\n\nThis is really non-trivial as it depends on the --[no-]chain-reply-to\nand --[no-]thread options. Also, the validation occurs before the\nmessage id headers are generated. I'd prefer trusting git-format-patch\nto order the patch files properly based on their file names.\n\n> Anyway, rather than pinging by random people, I'd like to see [PATCH\n> RESEND], rebased on latest git.git tree, ideally with Junio Cc'ed.\n\nWill do. Thanks.\n"},{"id":"474670","messageId":"CAPig+cS_+a+yu92+hKJ+Nvuvi4ymPOESaUgorYUAUGMAj9oZAg@mail.gmail.com","threadId":"59033","inReplyTo":"CRMHVBPXG6SW.2DL6RVNEMVERF@ringo","subject":"Re: [PATCH] hooks: add sendemail-validate-series","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-04-03T00:05:43Z","receivedAt":"2023-04-03T00:05:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 2, 2023 at 3:19 PM Robin Jarry wrote:\n> Bagas Sanjaya, Apr 01, 2023 at 04:54:\n> > In most cases, the patch series is generated by git-format-patch(1).\n> > When the command is run, it will output:\n> >\n> > $ git format-patch -o /tmp --cover-letter --base= \n> > /tmp/0000-cover-letter.patch\n> > /tmp/0001-.patch\n> > /tmp/0002-.patch\n> > /tmp/0003-.patch\n> >\n> > The output can be fed to the hook (as you write).\n> >\n> > But I think the hook should also take patch file arguments, for the\n> > sake of completeness with sendemail-validate hook; that is:\n> >\n> > sendemail-validate-series ...\n>\n> I don't mind adding this but I am concerned with the maximum size of the\n> command line arguments when sending large series. Standard input seems\n> like a safer solution.\n\nI share your concern, and don't see a good reason for complicating the\nimplementation _and_ the API by feeding pathnames to the hook as both\nstdin and as arguments. Feeding them on stdin is the safer choice even\nif it makes the hook implementation a bit more clunky.\n\n> > Also, there should have a check that In-Reply-To must be the first\n> > patch in the given series or the cover letter (if there is one).\n>\n> This is really non-trivial as it depends on the --[no-]chain-reply-to\n> and --[no-]thread options. Also, the validation occurs before the\n> message id headers are generated. I'd prefer trusting git-format-patch\n> to order the patch files properly based on their file names.\n\nMoreover, enforcing In-Reply-To: like that would almost certainly be\nundesirable. It may be policy on _this_ project to chain rerolls like\nthat, but other projects have different policies, even to the point of\nprohibiting chaining.\n\nSuch In-Reply-To: enforcement is also well outside the scope of the\npatch under discussion, as well as being entirely orthogonal.\n\n> > Anyway, rather than pinging by random people, I'd like to see [PATCH\n> > RESEND], rebased on latest git.git tree, ideally with Junio Cc'ed.\n>\n> Will do. Thanks.\n\nGenerally speaking, once you've sent a patch or patch series, when you\nre-roll, you should _not_ rebase it onto Junio's latest \"master\" since\ndoing so makes it harder for him to pick up the new version. In this\ncase, your original patch didn't get picked up, so no harm done by\nrebasing it on \"master\", but it's good practice to avoid doing so\nunless there is a strong reason.\n"}]}