{"thread":{"id":"54577","subject":"[PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","startedAt":"2020-11-05T12:16:27Z","lastAt":"2020-11-10T05:41:08Z","messageCount":15,"participants":["Hu Keping","Jeff King","Junio C Hamano","hukeping","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"409198","messageId":"20201105201548.2333425-1-hukeping@huawei.com","threadId":"54577","inReplyTo":null,"subject":"[PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"Hu Keping","fromEmail":"hukeping@huawei.com","sentAt":"2020-11-05T20:15:48Z","receivedAt":"2020-11-05T12:16:27Z","isPatch":true,"sender":{"key":"hukeping@huawei.com","avatar":"https://gravatar.com/avatar/780779d4f3d832126311fb2cc42177550b7f0b21299df4a06c497eb6afdad808?d=mp&s=160"},"body":"The file name of the patch generated by `git format-patch` usually be\ntruncated as there is less than 60 bytes left for the subject of commit\nmessage exclude the prefix patch number, say \"0001-\".\n\nAs per the kernel documentation[1], it suggest the length of subject no\nmore than 75.\n>\n> [...]\n> For these reasons, the ``summary`` must be no more than 70-75\n> characters, and it must describe both what the patch changes, as well\n> as why the patch might be necessary.\n>\n\nConsidered the prefix patch number \"0001-\" would take 5 characters, increase\nthe FORMAT_PATCH_NAME_MAX to 80.\n\n[1] https://www.kernel.org/doc/Documentation/process/submitting-patches.rst\n\nSigned-off-by: Hu Keping \n---\n log-tree.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/log-tree.h b/log-tree.h\nindex 8fa79289ec..021aee0d21 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -33,7 +33,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n-#define FORMAT_PATCH_NAME_MAX 64\n+#define FORMAT_PATCH_NAME_MAX 80\n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\n void fmt_output_email_subject(struct strbuf *, struct rev_info *);\n-- \n2.18.0.huawei.25\n\n"},{"id":"409203","messageId":"20201105150149.GA107127@coredump.intra.peff.net","threadId":"54577","inReplyTo":"20201105201548.2333425-1-hukeping@huawei.com","subject":"Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-05T15:01:49Z","receivedAt":"2020-11-05T15:01:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 05, 2020 at 08:15:48PM +0000, Hu Keping wrote:\n\n> The file name of the patch generated by `git format-patch` usually be\n> truncated as there is less than 60 bytes left for the subject of commit\n> message exclude the prefix patch number, say \"0001-\".\n> \n> As per the kernel documentation[1], it suggest the length of subject no\n> more than 75.\n> >\n> > [...]\n> > For these reasons, the ``summary`` must be no more than 70-75\n> > characters, and it must describe both what the patch changes, as well\n> > as why the patch might be necessary.\n> >\n> \n> Considered the prefix patch number \"0001-\" would take 5 characters, increase\n> the FORMAT_PATCH_NAME_MAX to 80.\n\nAs the code is written now, the length also includes the \".patch\"\nsuffix, as well as an extra byte (maybe for a NUL? Once upon a time I\nimagine we used static buffers, but these days it's all in a strbuf).\n\nA simple test with:\n\n git init\n for i in $(seq 8); do printf 1234567890; done |\n git commit --allow-empty -F -\n git format-patch -1\n\nshows us generating:\n\n 0001-1234567890123456789012345678901234567890123456789012.patch\n\nSo that's only 52 characters, from our constant of 64. Bumping to 80\ngives us 66, which is reasonable though probably still involves\noccasional truncation. But maybe keeping the total length to 80 (79,\nreally, because of the extra byte) may be worth doing.\n\nWhich is all a long-winded way of saying that your patch seems\nreasonable to me.\n\nLooking at the code which uses the constant, I suspect it could also be\nmade simpler:\n\n - the PATH_MAX check in open_next_file() seems pointless. Once upon a\n time it mattered for fitting into a PATH_MAX buffer, but these days\n we use a dynamic buffer anyway. We are probably better off to just\n feed the result to the filesystem and see if it complains (since\n either way we are aborting; I'd feel differently if we adjusted our\n truncation size)\n\n - the logic in fmt_output_subject() could probably be simpler if the\n constant was \"here's how long the subject should be\", not \"here's\n how long the whole thing must be\".\n\nBut those are both orthogonal to your patch and can be done separately.\n\n-Peff\n"},{"id":"409230","messageId":"xmqqimajijwa.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"20201105150149.GA107127@coredump.intra.peff.net","subject":"Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-05T21:16:53Z","receivedAt":"2020-11-05T21:16:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Considered the prefix patch number \"0001-\" would take 5 characters, increase\n>> the FORMAT_PATCH_NAME_MAX to 80.\n>\n> As the code is written now, the length also includes the \".patch\"\n> suffix, as well as an extra byte (maybe for a NUL? Once upon a time I\n> imagine we used static buffers, but these days it's all in a strbuf).\n>\n> A simple test with:\n>\n> git init\n> for i in $(seq 8); do printf 1234567890; done |\n> git commit --allow-empty -F -\n> git format-patch -1\n>\n> shows us generating:\n>\n> 0001-1234567890123456789012345678901234567890123456789012.patch\n>\n> So that's only 52 characters, from our constant of 64. Bumping to 80\n> gives us 66, which is reasonable though probably still involves\n> occasional truncation. But maybe keeping the total length to 80 (79,\n> really, because of the extra byte) may be worth doing.\n>\n> Which is all a long-winded way of saying that your patch seems\n> reasonable to me.\n\nA devil's advocate thinks that we should shorten it (and rename it\nto format-patch-subject-prefix-length or something) instead. That\nway, \"ls\" output can show more than one files on a single line even\non a 80-column terminal. The leading digits already guarantee the\nuniqueness anyway.\n\nI do not mind getting rid of the \"FORMAT_PATCH_NAME_MAX\" constant\nand replacing it with a variable that defaults to 64 and can be\ntweaked by a command line option and/or a configuration variable.\nIt does not feel it is worth the effort to replace one hardcoded\nconstant with another hardcoded constant.\n\n> Looking at the code which uses the constant, I suspect it could also be\n> made simpler:\n>\n> - the PATH_MAX check in open_next_file() seems pointless. Once upon a\n> time it mattered for fitting into a PATH_MAX buffer, but these days\n> we use a dynamic buffer anyway. We are probably better off to just\n> feed the result to the filesystem and see if it complains (since\n> either way we are aborting; I'd feel differently if we adjusted our\n> truncation size)\n>\n> - the logic in fmt_output_subject() could probably be simpler if the\n> constant was \"here's how long the subject should be\", not \"here's\n> how long the whole thing must be\".\n>\n> But those are both orthogonal to your patch and can be done separately.\n\nYes, these clean-ups seem worth doing.\n"},{"id":"409269","messageId":"d5338d5f83584f7caf3ff0f4309f2275@huawei.com","threadId":"54577","inReplyTo":"xmqqimajijwa.fsf@gitster.c.googlers.com","subject":"RE: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"hukeping","fromEmail":"hukeping@huawei.com","sentAt":"2020-11-06T08:51:32Z","receivedAt":"2020-11-06T08:51:41Z","isPatch":true,"sender":{"key":"hukeping@huawei.com","avatar":"https://gravatar.com/avatar/780779d4f3d832126311fb2cc42177550b7f0b21299df4a06c497eb6afdad808?d=mp&s=160"},"body":">-----Original Message-----\n>From: Junio C Hamano [mailto:gitster@pobox.com]\n>Sent: Friday, November 6, 2020 5:17 AM\n>To: Jeff King \n>Cc: hukeping ; git@vger.kernel.org; Zhengjunling (JRing,\n>Task Force) ; zhuangbiaowei\n>; git@stormcloud9.net; rafa.almas@gmail.com;\n>l.s.r@web.de\n>Subject: Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80\n>\n>Jeff King writes:\n>\n>>> Considered the prefix patch number \"0001-\" would take 5 characters,\n>>> increase the FORMAT_PATCH_NAME_MAX to 80.\n>>\n>> As the code is written now, the length also includes the \".patch\"\n>> suffix, as well as an extra byte (maybe for a NUL? Once upon a time I\n>> imagine we used static buffers, but these days it's all in a strbuf).\n>>\n>> A simple test with:\n>>\n>> git init\n>> for i in $(seq 8); do printf 1234567890; done |\n>> git commit --allow-empty -F -\n>> git format-patch -1\n>>\n>> shows us generating:\n>>\n>> 0001-1234567890123456789012345678901234567890123456789012.patch\n>>\n>> So that's only 52 characters, from our constant of 64. Bumping to 80\n>> gives us 66, which is reasonable though probably still involves\n>> occasional truncation. But maybe keeping the total length to 80 (79,\n>> really, because of the extra byte) may be worth doing.\n>>\n>> Which is all a long-winded way of saying that your patch seems\n>> reasonable to me.\n>\n>A devil's advocate thinks that we should shorten it (and rename it to format-\n>patch-subject-prefix-length or something) instead. That way, \"ls\" output can\n>show more than one files on a single line even on a 80-column terminal. The\n>leading digits already guarantee the uniqueness anyway.\n>\n>I do not mind getting rid of the \"FORMAT_PATCH_NAME_MAX\" constant and\n>replacing it with a variable that defaults to 64 and can be tweaked by a command\n>line option and/or a configuration variable.\n>It does not feel it is worth the effort to replace one hardcoded constant with\n>another hardcoded constant.\n>\n>> Looking at the code which uses the constant, I suspect it could also\n>> be made simpler:\n>>\n>> - the PATH_MAX check in open_next_file() seems pointless. Once upon a\n>> time it mattered for fitting into a PATH_MAX buffer, but these days\n>> we use a dynamic buffer anyway. We are probably better off to just\n>> feed the result to the filesystem and see if it complains (since\n>> either way we are aborting; I'd feel differently if we adjusted our\n>> truncation size)\n>>\n>> - the logic in fmt_output_subject() could probably be simpler if the\n>> constant was \"here's how long the subject should be\", not \"here's\n>> how long the whole thing must be\".\n>>\n>> But those are both orthogonal to your patch and can be done separately.\n>\n>Yes, these clean-ups seem worth doing.\n\nAgreed, and I'd like to do it with two separated commits:\n- commit-1, cleanup the open_next_file() by drop the if (filename.len>=..) statements.\n\n- commit-2, replace FORMAT_PATCH_NAME_MAX in fmt_output_subject() with a constant\n in there and make it to 80(or other value?), and drop FORMAT_PATCH_NAME_MAX\n from log-tree.h.\n\nIs this works for you?\n\nThanks.\n"},{"id":"409278","messageId":"xmqqwnyyfkgy.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"d5338d5f83584f7caf3ff0f4309f2275@huawei.com","subject":"Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-06T17:45:01Z","receivedAt":"2020-11-06T17:45:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"hukeping writes:\n\n>>I do not mind getting rid of the \"FORMAT_PATCH_NAME_MAX\" constant and\n>>replacing it with a variable that defaults to 64 and can be tweaked by a command\n>>line option and/or a configuration variable.\n>>It does not feel it is worth the effort to replace one hardcoded constant with\n>>another hardcoded constant.\n>>\n>>> Looking at the code which uses the constant, I suspect it could also\n>>> be made simpler:\n>>>\n>>> - the PATH_MAX check in open_next_file() seems pointless. Once upon a\n>>> time it mattered for fitting into a PATH_MAX buffer, but these days\n>>> we use a dynamic buffer anyway. We are probably better off to just\n>>> feed the result to the filesystem and see if it complains (since\n>>> either way we are aborting; I'd feel differently if we adjusted our\n>>> truncation size)\n>>>\n>>> - the logic in fmt_output_subject() could probably be simpler if the\n>>> constant was \"here's how long the subject should be\", not \"here's\n>>> how long the whole thing must be\".\n>>>\n>>> But those are both orthogonal to your patch and can be done separately.\n>>\n>>Yes, these clean-ups seem worth doing.\n>\n> Agreed, and I'd like to do it with two separated commits:\n> - commit-1, cleanup the open_next_file() by drop the if (filename.len>=..) statements.\n>\n> - commit-2, replace FORMAT_PATCH_NAME_MAX in fmt_output_subject() with a constant\n> in there and make it to 80(or other value?), and drop FORMAT_PATCH_NAME_MAX\n> from log-tree.h.\n>\n> Is this works for you?\n\nI am not sure what you meant by \"Agreed\". I said two things:\n\n - It is dubious that it is worth the effort to replace a hardcoded\n constant with another. Making it configurable with command line\n option and/or configuration variable may be worth doing.\n\n - Two observations Peff made for further clean-up are probably\n worth doing.\n\nIf you are agreeing to both of the above and following through, then\nyes, it seems like a good plan. If agreement is only to the former,\nit probably still is worth doing. Anything else, I don't know.\n"},{"id":"409297","messageId":"xmqqwnyydxaw.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"xmqqimajijwa.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-06T20:50:47Z","receivedAt":"2020-11-06T20:51:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> A devil's advocate thinks that we should shorten it (and rename it\n> to format-patch-subject-prefix-length or something) instead. That\n> way, \"ls\" output can show more than one files on a single line even\n> on a 80-column terminal. The leading digits already guarantee the\n> uniqueness anyway.\n>\n> I do not mind getting rid of the \"FORMAT_PATCH_NAME_MAX\" constant\n> and replacing it with a variable that defaults to 64 and can be\n> tweaked by a command line option and/or a configuration variable.\n> It does not feel it is worth the effort to replace one hardcoded\n> constant with another hardcoded constant.\n\nSo here is my lunch-time hack for the day. Totally untested beyond\n\"it comiples and links\".\n\nA new configuration variable format.patchnamemax and a new command\nline option --filename-max-length= overrides the hardcoded\nconstant 64, which is the default. A value that is unreasonably\nsmall is corrected to a hardcoded floor value of 8.\n\nNo new tests added nor documentation updated.\n\n\n\n\n builtin/log.c | 21 ++++++++++++++-------\n log-tree.c | 2 +-\n log-tree.h | 1 -\n revision.h | 1 +\n 4 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git c/builtin/log.c w/builtin/log.c\nindex 0a7ed4bef9..d938fba27e 100644\n--- c/builtin/log.c\n+++ w/builtin/log.c\n@@ -37,6 +37,8 @@\n \n #define MAIL_DEFAULT_WRAP 72\n #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n+#define FORMAT_PATCH_NAME_MIN 8\n+#define FORMAT_PATCH_NAME_MAX 64\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -50,6 +52,7 @@ static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n+static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX;\n static const char *fmt_pretty;\n \n static const char * const builtin_log_usage[] = {\n@@ -150,6 +153,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n+\trev->patch_name_max = fmt_patch_name_max;\n \trev->show_signature = default_show_signature;\n \trev->encode_email_headers = default_encode_email_headers;\n \trev->diffopt.flags.allow_textconv = 1;\n@@ -454,6 +458,12 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcmp(var, \"format.patchnamemax\")) {\n+\t\tfmt_patch_name_max = git_config_int(var, value);\n+\t\tif (fmt_patch_name_max < FORMAT_PATCH_NAME_MIN)\n+\t\t\tfmt_patch_name_max = FORMAT_PATCH_NAME_MIN;\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.encodeemailheaders\")) {\n \t\tdefault_encode_email_headers = git_config_bool(var, value);\n \t\treturn 0;\n@@ -955,15 +965,9 @@ static int open_next_file(struct commit *commit, const char *subject,\n \t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n-\tint suffix_len = strlen(rev->patch_suffix) + 1;\n \n \tif (output_directory) {\n \t\tstrbuf_addstr(&filename, output_directory);\n-\t\tif (filename.len >=\n-\t\t PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len) {\n-\t\t\tstrbuf_release(&filename);\n-\t\t\treturn error(_(\"name of output directory is too long\"));\n-\t\t}\n \t\tstrbuf_complete(&filename, '/');\n \t}\n \n@@ -1751,6 +1755,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start numbering patches at instead of 1\")),\n \t\tOPT_INTEGER('v', \"reroll-count\", &reroll_count,\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n+\t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n+\t\t\t N_(\"max length of output filename\")),\n \t\tOPT_CALLBACK_F(0, \"rfc\", &rev, NULL,\n \t\t\t N_(\"Use [RFC PATCH] instead of [PATCH]\"),\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback),\n@@ -1822,6 +1828,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tinit_display_notes(¬es_opt);\n \tgit_config(git_format_config, NULL);\n \trepo_init_revisions(the_repository, &rev, prefix);\n+\trev.subject_prefix = fmt_patch_subject_prefix;\n \trev.show_notes = show_notes;\n \tmemcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));\n \trev.commit_format = CMIT_FMT_EMAIL;\n@@ -1831,7 +1838,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diff = 1;\n \trev.max_parents = 1;\n \trev.diffopt.flags.recursive = 1;\n-\trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n@@ -1935,6 +1941,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \n \trev.zero_commit = zero_commit;\n+\trev.patch_name_max = fmt_patch_name_max;\n \n \tif (!rev.diffopt.flags.text && !no_binary_diff)\n \t\trev.diffopt.flags.binary = 1;\ndiff --git c/log-tree.c w/log-tree.c\nindex 1927f917ce..fd0dde97ec 100644\n--- c/log-tree.c\n+++ w/log-tree.c\n@@ -367,7 +367,7 @@ void fmt_output_subject(struct strbuf *filename,\n \tconst char *suffix = info->patch_suffix;\n \tint nr = info->nr;\n \tint start_len = filename->len;\n-\tint max_len = start_len + FORMAT_PATCH_NAME_MAX - (strlen(suffix) + 1);\n+\tint max_len = start_len + info->patch_name_max - (strlen(suffix) + 1);\n \n \tif (0 < info->reroll_count)\n \t\tstrbuf_addf(filename, \"v%d-\", info->reroll_count);\ndiff --git c/log-tree.h w/log-tree.h\nindex 8fa79289ec..1e8c91dbf2 100644\n--- c/log-tree.h\n+++ w/log-tree.h\n@@ -33,7 +33,6 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n-#define FORMAT_PATCH_NAME_MAX 64\n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\n void fmt_output_email_subject(struct strbuf *, struct rev_info *);\ndiff --git c/revision.h w/revision.h\nindex f6bf860d19..086ff10280 100644\n--- c/revision.h\n+++ w/revision.h\n@@ -238,6 +238,7 @@ struct rev_info {\n \tconst char\t*extra_headers;\n \tconst char\t*log_reencode;\n \tconst char\t*subject_prefix;\n+\tint\t\tpatch_name_max;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n \tstruct string_list *mailmap;\n"},{"id":"409300","messageId":"xmqqo8kadu9j.fsf_-_@gitster.c.googlers.com","threadId":"54577","inReplyTo":"xmqqwnyydxaw.fsf@gitster.c.googlers.com","subject":"[PATCH] format-patch: make output filename configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-06T21:56:24Z","receivedAt":"2020-11-06T21:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"For the past 15 years, we've used the hardcoded 64 as the length\nlimit of the filename of the output from the \"git format-patch\"\ncommand. Since the value is shorter than the 80-column terminal, it\ncould grow without line wrapping a bit. At the same time, since the\nvalue is longer than half of the 80-column terminal, we could fit\ntwo or more of them in \"ls\" output on such a terminal if we allowed\nto lower it.\n\nIntroduce a new command line option --filename-max-length= and a\nnew configuration variable format.filenameMaxLength to override the\nhardcoded default.\n\nWhile we are at it, remove a check that the name of output directory\ndoes not exceed PATH_MAX---this check is pointless in that by the\ntime control reaches the function, the caller would already have\ndone an equivalent of \"mkdir -p\", so if the system does not like an\noverly long directory name, the control wouldn't have reached here,\nand otherwise, we know that the system allowed the output directory\nto exist. In the worst case, we will get an error when we try to\nopen the output file and handle the error correctly anyway.\n\nSigned-off-by: Junio C Hamano \n---\n\n * Unlike the preview I sent out earlier, this is with doc/test,\n with consistent naming between option and config and adjust the\n minimum a bit better.\n\n Documentation/git-format-patch.txt | 8 +++++\n builtin/log.c | 22 ++++++++----\n log-tree.c | 2 +-\n log-tree.h | 1 -\n revision.h | 1 +\n t/t4014-format-patch.sh | 54 ++++++++++++++++++++++++++++++\n 6 files changed, 79 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 0f81d0437b..6c0da53d1b 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -28,6 +28,7 @@ SYNOPSIS\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n \t\t [--range-diff= [--creation-factor=]]\n+\t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n \t\t [ | ]\n@@ -200,6 +201,13 @@ populated with placeholder text.\n \tallows for useful naming of a patch series, and can be\n \tcombined with the `--numbered` option.\n \n+--filename-max-length=::\n+\tInstead of the standard 64 bytes, chomp the generated output\n+\tfilenames at around '' bytes (too short a value will be\n+\tsilently raised to a reasonable length). Defaults to the\n+\tvalue of the `format.filenamemaxlength` configuration\n+\tvariable, or 64 if unconfigured.\n+\n --rfc::\n \tAlias for `--subject-prefix=\"RFC PATCH\"`. RFC means \"Request For\n \tComments\"; use this when sending an experimental patch for\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0a7ed4bef9..140a7feedc 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -37,6 +37,7 @@\n \n #define MAIL_DEFAULT_WRAP 72\n #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n+#define FORMAT_PATCH_NAME_MAX 64\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -50,6 +51,7 @@ static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n+static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX;\n static const char *fmt_pretty;\n \n static const char * const builtin_log_usage[] = {\n@@ -150,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n+\trev->patch_name_max = fmt_patch_name_max;\n \trev->show_signature = default_show_signature;\n \trev->encode_email_headers = default_encode_email_headers;\n \trev->diffopt.flags.allow_textconv = 1;\n@@ -454,6 +457,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcmp(var, \"format.filenamemaxlength\")) {\n+\t\tfmt_patch_name_max = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.encodeemailheaders\")) {\n \t\tdefault_encode_email_headers = git_config_bool(var, value);\n \t\treturn 0;\n@@ -955,15 +962,9 @@ static int open_next_file(struct commit *commit, const char *subject,\n \t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n-\tint suffix_len = strlen(rev->patch_suffix) + 1;\n \n \tif (output_directory) {\n \t\tstrbuf_addstr(&filename, output_directory);\n-\t\tif (filename.len >=\n-\t\t PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len) {\n-\t\t\tstrbuf_release(&filename);\n-\t\t\treturn error(_(\"name of output directory is too long\"));\n-\t\t}\n \t\tstrbuf_complete(&filename, '/');\n \t}\n \n@@ -1751,6 +1752,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start numbering patches at instead of 1\")),\n \t\tOPT_INTEGER('v', \"reroll-count\", &reroll_count,\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n+\t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n+\t\t\t N_(\"max length of output filename\")),\n \t\tOPT_CALLBACK_F(0, \"rfc\", &rev, NULL,\n \t\t\t N_(\"Use [RFC PATCH] instead of [PATCH]\"),\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback),\n@@ -1822,6 +1825,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tinit_display_notes(¬es_opt);\n \tgit_config(git_format_config, NULL);\n \trepo_init_revisions(the_repository, &rev, prefix);\n+\trev.subject_prefix = fmt_patch_subject_prefix;\n \trev.show_notes = show_notes;\n \tmemcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));\n \trev.commit_format = CMIT_FMT_EMAIL;\n@@ -1831,7 +1835,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diff = 1;\n \trev.max_parents = 1;\n \trev.diffopt.flags.recursive = 1;\n-\trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n@@ -1851,6 +1854,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n+\t/* Make sure \"0000-$sub.patch\" gives non-negative length for $sub */\n+\tif (fmt_patch_name_max <= strlen(\"0000-\") + strlen(fmt_patch_suffix))\n+\t\tfmt_patch_name_max = strlen(\"0000-\") + strlen(fmt_patch_suffix);\n+\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n@@ -1935,6 +1942,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \n \trev.zero_commit = zero_commit;\n+\trev.patch_name_max = fmt_patch_name_max;\n \n \tif (!rev.diffopt.flags.text && !no_binary_diff)\n \t\trev.diffopt.flags.binary = 1;\ndiff --git a/log-tree.c b/log-tree.c\nindex 1927f917ce..fd0dde97ec 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -367,7 +367,7 @@ void fmt_output_subject(struct strbuf *filename,\n \tconst char *suffix = info->patch_suffix;\n \tint nr = info->nr;\n \tint start_len = filename->len;\n-\tint max_len = start_len + FORMAT_PATCH_NAME_MAX - (strlen(suffix) + 1);\n+\tint max_len = start_len + info->patch_name_max - (strlen(suffix) + 1);\n \n \tif (0 < info->reroll_count)\n \t\tstrbuf_addf(filename, \"v%d-\", info->reroll_count);\ndiff --git a/log-tree.h b/log-tree.h\nindex 8fa79289ec..1e8c91dbf2 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -33,7 +33,6 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n-#define FORMAT_PATCH_NAME_MAX 64\n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\n void fmt_output_email_subject(struct strbuf *, struct rev_info *);\ndiff --git a/revision.h b/revision.h\nindex f6bf860d19..086ff10280 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -238,6 +238,7 @@ struct rev_info {\n \tconst char\t*extra_headers;\n \tconst char\t*log_reencode;\n \tconst char\t*subject_prefix;\n+\tint\t\tpatch_name_max;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n \tstruct string_list *mailmap;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 294e76c860..024c0a026d 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -313,6 +313,60 @@ test_expect_success 'multiple files' '\n \tls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch\n '\n \n+test_expect_success 'filename length limit' '\n+\ttest_when_finished \"rm -f 000*\" &&\n+\trm -rf 000[1-9]-*.patch &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit format-patch --filename-max-length=$len -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in 000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"$patch\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n+test_expect_success 'filename length limit from config' '\n+\ttest_when_finished \"rm -f 000*\" &&\n+\trm -rf 000[1-9]-*.patch &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit -c format.filenameMaxLength=$len format-patch -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in 000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"$patch\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n+test_expect_success 'filename limit applies only to basename' '\n+\ttest_when_finished \"rm -rf patches/\" &&\n+\trm -rf patches/ &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit format-patch -o patches --filename-max-length=$len -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in patches/000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"${patch#patches/}\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n test_expect_success 'reroll count' '\n \trm -fr patches &&\n \tgit format-patch -o patches --cover-letter --reroll-count 4 master..side >list &&\n-- \n2.29.2-306-gddee074d3d\n\n"},{"id":"409302","messageId":"CAPig+cQAreDJn_tS_K4sR9HwsSVNoFid9sWSdxzwSqkJLRidgQ@mail.gmail.com","threadId":"54577","inReplyTo":"xmqqo8kadu9j.fsf_-_@gitster.c.googlers.com","subject":"Re: [PATCH] format-patch: make output filename configurable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-06T22:05:59Z","receivedAt":"2020-11-06T22:06:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 6, 2020 at 4:56 PM Junio C Hamano wrote:\n> [...]\n> Introduce a new command line option --filename-max-length= and a\n> new configuration variable format.filenameMaxLength to override the\n> hardcoded default.\n> [...]\n> Signed-off-by: Junio C Hamano \n> ---\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> @@ -200,6 +201,13 @@ populated with placeholder text.\n> +--filename-max-length=::\n> + Instead of the standard 64 bytes, chomp the generated output\n> + filenames at around '' bytes (too short a value will be\n> + silently raised to a reasonable length). Defaults to the\n> + value of the `format.filenamemaxlength` configuration\n> + variable, or 64 if unconfigured.\n\nIn user-facing documentation, I believe practice is to camelCase the\nconfiguration variable name, so:\ns/filenamemaxlength/filenameMaxLength/\n\nThis new configuration also ought to be mentioned in\nDocumentation/config/format.txt.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -37,6 +37,7 @@\n> +#define FORMAT_PATCH_NAME_MAX 64\n\nNit: A slightly less confusing name would be FORMAT_PATCH_NAME_MAX_DEFAULT.\n"},{"id":"409404","messageId":"xmqqwnyubagr.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"CAPig+cQAreDJn_tS_K4sR9HwsSVNoFid9sWSdxzwSqkJLRidgQ@mail.gmail.com","subject":"[PATCH v2] format-patch: make output filename configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-09T19:23:48Z","receivedAt":"2020-11-09T19:23:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"For the past 15 years, we've used the hardcoded 64 as the length\nlimit of the filename of the output from the \"git format-patch\"\ncommand. Since the value is shorter than the 80-column terminal, it\ncould grow without line wrapping a bit. At the same time, since the\nvalue is longer than half of the 80-column terminal, we could fit\ntwo or more of them in \"ls\" output on such a terminal if we allowed\nto lower it.\n\nIntroduce a new command line option --filename-max-length= and a\nnew configuration variable format.filenameMaxLength to override the\nhardcoded default.\n\nWhile we are at it, remove a check that the name of output directory\ndoes not exceed PATH_MAX---this check is pointless in that by the\ntime control reaches the function, the caller would already have\ndone an equivalent of \"mkdir -p\", so if the system does not like an\noverly long directory name, the control wouldn't have reached here,\nand otherwise, we know that the system allowed the output directory\nto exist. In the worst case, we will get an error when we try to\nopen the output file and handle the error correctly anyway.\n\nSigned-off-by: Junio C Hamano \n---\nRange-diff against v1:\n1: 1b012b4164 ! 1: d1d91e4833 format-patch: make output filename configurable\n @@ Commit message\n \n Signed-off-by: Junio C Hamano \n \n + ## Documentation/config/format.txt ##\n +@@ Documentation/config/format.txt: format.outputDirectory::\n + \tSet a custom directory to store the resulting files instead of the\n + \tcurrent working directory. All directory components will be created.\n + \n ++format.filenameMaxLength::\n ++\tThe maximum length of the output filenames generated by the\n ++\t`format-patch` command; defaults to 64. Can be overridden\n ++\tby the `--filename-max-length=` command line option.\n ++\n + format.useAutoBase::\n + \tA boolean value which lets you enable the `--base=auto` option of\n + \tformat-patch by default. Can also be set to \"whenAble\" to allow\n +\n ## Documentation/git-format-patch.txt ##\n @@ Documentation/git-format-patch.txt: SYNOPSIS\n \t\t [--no-notes | --notes[=]]\n @@ Documentation/git-format-patch.txt: populated with placeholder text.\n +\tInstead of the standard 64 bytes, chomp the generated output\n +\tfilenames at around '' bytes (too short a value will be\n +\tsilently raised to a reasonable length). Defaults to the\n -+\tvalue of the `format.filenamemaxlength` configuration\n ++\tvalue of the `format.filenameMaxLength` configuration\n +\tvariable, or 64 if unconfigured.\n +\n --rfc::\n @@ builtin/log.c\n \n #define MAIL_DEFAULT_WRAP 72\n #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n -+#define FORMAT_PATCH_NAME_MAX 64\n ++#define FORMAT_PATCH_NAME_MAX_DEFAULT 64\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n @@ builtin/log.c: static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n -+static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX;\n ++static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;\n static const char *fmt_pretty;\n \n static const char * const builtin_log_usage[] = {\n\n Documentation/config/format.txt | 5 +++\n Documentation/git-format-patch.txt | 8 +++++\n builtin/log.c | 22 ++++++++----\n log-tree.c | 2 +-\n log-tree.h | 1 -\n revision.h | 1 +\n t/t4014-format-patch.sh | 54 ++++++++++++++++++++++++++++++\n 7 files changed, 84 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex c2efd8758a..7f6d11b5d5 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -94,6 +94,11 @@ format.outputDirectory::\n \tSet a custom directory to store the resulting files instead of the\n \tcurrent working directory. All directory components will be created.\n \n+format.filenameMaxLength::\n+\tThe maximum length of the output filenames generated by the\n+\t`format-patch` command; defaults to 64. Can be overridden\n+\tby the `--filename-max-length=` command line option.\n+\n format.useAutoBase::\n \tA boolean value which lets you enable the `--base=auto` option of\n \tformat-patch by default. Can also be set to \"whenAble\" to allow\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 0f81d0437b..3347702b71 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -28,6 +28,7 @@ SYNOPSIS\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n \t\t [--range-diff= [--creation-factor=]]\n+\t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n \t\t [ | ]\n@@ -200,6 +201,13 @@ populated with placeholder text.\n \tallows for useful naming of a patch series, and can be\n \tcombined with the `--numbered` option.\n \n+--filename-max-length=::\n+\tInstead of the standard 64 bytes, chomp the generated output\n+\tfilenames at around '' bytes (too short a value will be\n+\tsilently raised to a reasonable length). Defaults to the\n+\tvalue of the `format.filenameMaxLength` configuration\n+\tvariable, or 64 if unconfigured.\n+\n --rfc::\n \tAlias for `--subject-prefix=\"RFC PATCH\"`. RFC means \"Request For\n \tComments\"; use this when sending an experimental patch for\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0a7ed4bef9..861ac17da0 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -37,6 +37,7 @@\n \n #define MAIL_DEFAULT_WRAP 72\n #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n+#define FORMAT_PATCH_NAME_MAX_DEFAULT 64\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -50,6 +51,7 @@ static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n+static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;\n static const char *fmt_pretty;\n \n static const char * const builtin_log_usage[] = {\n@@ -150,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n+\trev->patch_name_max = fmt_patch_name_max;\n \trev->show_signature = default_show_signature;\n \trev->encode_email_headers = default_encode_email_headers;\n \trev->diffopt.flags.allow_textconv = 1;\n@@ -454,6 +457,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcmp(var, \"format.filenamemaxlength\")) {\n+\t\tfmt_patch_name_max = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.encodeemailheaders\")) {\n \t\tdefault_encode_email_headers = git_config_bool(var, value);\n \t\treturn 0;\n@@ -955,15 +962,9 @@ static int open_next_file(struct commit *commit, const char *subject,\n \t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n-\tint suffix_len = strlen(rev->patch_suffix) + 1;\n \n \tif (output_directory) {\n \t\tstrbuf_addstr(&filename, output_directory);\n-\t\tif (filename.len >=\n-\t\t PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len) {\n-\t\t\tstrbuf_release(&filename);\n-\t\t\treturn error(_(\"name of output directory is too long\"));\n-\t\t}\n \t\tstrbuf_complete(&filename, '/');\n \t}\n \n@@ -1751,6 +1752,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start numbering patches at instead of 1\")),\n \t\tOPT_INTEGER('v', \"reroll-count\", &reroll_count,\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n+\t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n+\t\t\t N_(\"max length of output filename\")),\n \t\tOPT_CALLBACK_F(0, \"rfc\", &rev, NULL,\n \t\t\t N_(\"Use [RFC PATCH] instead of [PATCH]\"),\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback),\n@@ -1822,6 +1825,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tinit_display_notes(¬es_opt);\n \tgit_config(git_format_config, NULL);\n \trepo_init_revisions(the_repository, &rev, prefix);\n+\trev.subject_prefix = fmt_patch_subject_prefix;\n \trev.show_notes = show_notes;\n \tmemcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));\n \trev.commit_format = CMIT_FMT_EMAIL;\n@@ -1831,7 +1835,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diff = 1;\n \trev.max_parents = 1;\n \trev.diffopt.flags.recursive = 1;\n-\trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n@@ -1851,6 +1854,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n+\t/* Make sure \"0000-$sub.patch\" gives non-negative length for $sub */\n+\tif (fmt_patch_name_max <= strlen(\"0000-\") + strlen(fmt_patch_suffix))\n+\t\tfmt_patch_name_max = strlen(\"0000-\") + strlen(fmt_patch_suffix);\n+\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n@@ -1935,6 +1942,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \n \trev.zero_commit = zero_commit;\n+\trev.patch_name_max = fmt_patch_name_max;\n \n \tif (!rev.diffopt.flags.text && !no_binary_diff)\n \t\trev.diffopt.flags.binary = 1;\ndiff --git a/log-tree.c b/log-tree.c\nindex 1927f917ce..fd0dde97ec 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -367,7 +367,7 @@ void fmt_output_subject(struct strbuf *filename,\n \tconst char *suffix = info->patch_suffix;\n \tint nr = info->nr;\n \tint start_len = filename->len;\n-\tint max_len = start_len + FORMAT_PATCH_NAME_MAX - (strlen(suffix) + 1);\n+\tint max_len = start_len + info->patch_name_max - (strlen(suffix) + 1);\n \n \tif (0 < info->reroll_count)\n \t\tstrbuf_addf(filename, \"v%d-\", info->reroll_count);\ndiff --git a/log-tree.h b/log-tree.h\nindex 8fa79289ec..1e8c91dbf2 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -33,7 +33,6 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n-#define FORMAT_PATCH_NAME_MAX 64\n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\n void fmt_output_email_subject(struct strbuf *, struct rev_info *);\ndiff --git a/revision.h b/revision.h\nindex f6bf860d19..086ff10280 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -238,6 +238,7 @@ struct rev_info {\n \tconst char\t*extra_headers;\n \tconst char\t*log_reencode;\n \tconst char\t*subject_prefix;\n+\tint\t\tpatch_name_max;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n \tstruct string_list *mailmap;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 294e76c860..024c0a026d 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -313,6 +313,60 @@ test_expect_success 'multiple files' '\n \tls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch\n '\n \n+test_expect_success 'filename length limit' '\n+\ttest_when_finished \"rm -f 000*\" &&\n+\trm -rf 000[1-9]-*.patch &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit format-patch --filename-max-length=$len -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in 000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"$patch\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n+test_expect_success 'filename length limit from config' '\n+\ttest_when_finished \"rm -f 000*\" &&\n+\trm -rf 000[1-9]-*.patch &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit -c format.filenameMaxLength=$len format-patch -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in 000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"$patch\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n+test_expect_success 'filename limit applies only to basename' '\n+\ttest_when_finished \"rm -rf patches/\" &&\n+\trm -rf patches/ &&\n+\tfor len in 15 25 35\n+\tdo\n+\t\tgit format-patch -o patches --filename-max-length=$len -3 side &&\n+\t\tmax=$(\n+\t\t\tfor patch in patches/000[1-9]-*.patch\n+\t\t\tdo\n+\t\t\t\techo \"${patch#patches/}\" | wc -c\n+\t\t\tdone |\n+\t\t\tsort -nr |\n+\t\t\thead -n 1\n+\t\t) &&\n+\t\ttest $max -le $len || return 1\n+\tdone\n+'\n+\n test_expect_success 'reroll count' '\n \trm -fr patches &&\n \tgit format-patch -o patches --cover-letter --reroll-count 4 master..side >list &&\n-- \n2.29.2-306-gddee074d3d\n\n"},{"id":"409441","messageId":"20201110002312.GA1267848@coredump.intra.peff.net","threadId":"54577","inReplyTo":"xmqqwnyubagr.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] format-patch: make output filename configurable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-10T00:23:12Z","receivedAt":"2020-11-10T00:23:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 09, 2020 at 11:23:48AM -0800, Junio C Hamano wrote:\n\n> @@ -1822,6 +1825,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tinit_display_notes(¬es_opt);\n> \tgit_config(git_format_config, NULL);\n> \trepo_init_revisions(the_repository, &rev, prefix);\n> +\trev.subject_prefix = fmt_patch_subject_prefix;\n> \trev.show_notes = show_notes;\n> \tmemcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));\n> \trev.commit_format = CMIT_FMT_EMAIL;\n> @@ -1831,7 +1835,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \trev.diff = 1;\n> \trev.max_parents = 1;\n> \trev.diffopt.flags.recursive = 1;\n> -\trev.subject_prefix = fmt_patch_subject_prefix;\n> \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n> \ts_r_opt.def = \"HEAD\";\n> \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n\nIt's not clear to me what these hunks are doing. I'm trying really hard\nto find some subtle reason that we need to init this field sooner, but I\ncan't. It really looks like it might be leftover noise.\n\n-Peff\n"},{"id":"409454","messageId":"xmqqzh3q7zqu.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"20201110002312.GA1267848@coredump.intra.peff.net","subject":"Re: [PATCH v2] format-patch: make output filename configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T01:43:37Z","receivedAt":"2020-11-10T01:43:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Nov 09, 2020 at 11:23:48AM -0800, Junio C Hamano wrote:\n>\n>> @@ -1822,6 +1825,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>> \tinit_display_notes(¬es_opt);\n>> \tgit_config(git_format_config, NULL);\n>> \trepo_init_revisions(the_repository, &rev, prefix);\n>> +\trev.subject_prefix = fmt_patch_subject_prefix;\n>> \trev.show_notes = show_notes;\n>> \tmemcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));\n>> \trev.commit_format = CMIT_FMT_EMAIL;\n>> @@ -1831,7 +1835,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>> \trev.diff = 1;\n>> \trev.max_parents = 1;\n>> \trev.diffopt.flags.recursive = 1;\n>> -\trev.subject_prefix = fmt_patch_subject_prefix;\n>> \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n>> \ts_r_opt.def = \"HEAD\";\n>> \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n>\n> It's not clear to me what these hunks are doing. I'm trying really hard\n> to find some subtle reason that we need to init this field sooner, but I\n> can't. It really looks like it might be leftover noise.\n\nIt indeed was leftover noise and has nothing to do with the output\nfilename length configurablility (I suspect it is a true no-op).\n\nThanks.\n\n\n"},{"id":"409460","messageId":"9d4b387655024b24a20ce2384740b03a@huawei.com","threadId":"54577","inReplyTo":"xmqqwnyubagr.fsf@gitster.c.googlers.com","subject":"RE: [PATCH v2] format-patch: make output filename configurable","fromName":"hukeping","fromEmail":"hukeping@huawei.com","sentAt":"2020-11-10T02:31:38Z","receivedAt":"2020-11-10T02:31:44Z","isPatch":true,"sender":{"key":"hukeping@huawei.com","avatar":"https://gravatar.com/avatar/780779d4f3d832126311fb2cc42177550b7f0b21299df4a06c497eb6afdad808?d=mp&s=160"},"body":">For the past 15 years, we've used the hardcoded 64 as the length limit of the\n>filename of the output from the \"git format-patch\"\n>command. Since the value is shorter than the 80-column terminal, it could grow\n>without line wrapping a bit. At the same time, since the value is longer than half\n>of the 80-column terminal, we could fit two or more of them in \"ls\" output on\n>such a terminal if we allowed to lower it.\n>\n>Introduce a new command line option --filename-max-length= and a new\n>configuration variable format.filenameMaxLength to override the hardcoded\n>default.\n>\nIt would be very hard to remove a config knob rather than add a new one and we already\nhave too many.\n\nDoes it worth to add a new configuration variable for this or just a hard-coded value is enough?\n\n>While we are at it, remove a check that the name of output directory does not\n>exceed PATH_MAX---this check is pointless in that by the time control reaches the\n>function, the caller would already have done an equivalent of \"mkdir -p\", so if the\n>system does not like an overly long directory name, the control wouldn't have\n>reached here, and otherwise, we know that the system allowed the output\n>directory to exist. In the worst case, we will get an error when we try to open the\n>output file and handle the error correctly anyway.\n>\n>Signed-off-by: Junio C Hamano \n\n"},{"id":"409462","messageId":"xmqqr1p27x8d.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"9d4b387655024b24a20ce2384740b03a@huawei.com","subject":"Re: [PATCH v2] format-patch: make output filename configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T02:37:54Z","receivedAt":"2020-11-10T02:38:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"hukeping writes:\n\n> It would be very hard to remove a config knob rather than add a\n> new one and we already have too many.\n>\n> Does it worth to add a new configuration variable for this or just\n> a hard-coded value is enough?\n\nI personally would say \"yes, the current code that limits to 64 is\nenough\", but you, as the person who said that you do not like the\ncurrent hard-coded value, are not in the position to ask that\nquestion, I would have to say---if it were enough for you, you\nwouldn't have complained about 64 in the first place ;-)\n\n\n\n"},{"id":"409464","messageId":"8b97c61e983c432aab1063912d6266b9@huawei.com","threadId":"54577","inReplyTo":"xmqqr1p27x8d.fsf@gitster.c.googlers.com","subject":"RE: [PATCH v2] format-patch: make output filename configurable","fromName":"hukeping","fromEmail":"hukeping@huawei.com","sentAt":"2020-11-10T04:44:21Z","receivedAt":"2020-11-10T04:44:27Z","isPatch":true,"sender":{"key":"hukeping@huawei.com","avatar":"https://gravatar.com/avatar/780779d4f3d832126311fb2cc42177550b7f0b21299df4a06c497eb6afdad808?d=mp&s=160"},"body":">> It would be very hard to remove a config knob rather than add a new\n>> one and we already have too many.\n>>\n>> Does it worth to add a new configuration variable for this or just a\n>> hard-coded value is enough?\n>\n>I personally would say \"yes, the current code that limits to 64 is enough\", but you,\n>as the person who said that you do not like the current hard-coded value, are not\n>in the position to ask that question, I would have to say---if it were enough for\n>you, you wouldn't have complained about 64 in the first place ;-)\n\nThe original motivation is to lengthening the limit because of file name truncated problem,\nso update the value to a larger one seems like the simplest way for me.\n\nThe v2 patch can fundamentally solve this problem, just a little worry about the more and\nmore git-config knobs.\n"},{"id":"409465","messageId":"xmqqk0ut93bs.fsf@gitster.c.googlers.com","threadId":"54577","inReplyTo":"8b97c61e983c432aab1063912d6266b9@huawei.com","subject":"Re: [PATCH v2] format-patch: make output filename configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T05:40:55Z","receivedAt":"2020-11-10T05:41:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"hukeping writes:\n\n>>> Does it worth to add a new configuration variable for this or just a\n>>> hard-coded value is enough?\n>>\n>>I personally would say \"yes, the current code that limits to 64 is enough\", but you,\n>>as the person who said that you do not like the current hard-coded value, are not\n>>in the position to ask that question, I would have to say---if it were enough for\n>>you, you wouldn't have complained about 64 in the first place ;-)\n>\n> The original motivation is to lengthening the limit because of\n> file name truncated problem, so update the value to a larger one\n> seems like the simplest way for me.\n\nIt actually would be an improvement to me if we shorten the current\nhardcoded limit. In fact, 64 is still a bit too long to fit in\ndired buffer and raising the limit will make it even worse.\n\nIn other words, you are forgetting that a larger limit is not always\na better limit.\n\nI am not saying that between my wish to keep 64 as near optimal (and\npossibly make it shorter) and your wish to make it longer to avoid\ntruncation, the former wish is more important than the latter. The\nformer however is at least as important as the latter [*1*].\n\nAnd once you realize that one hardcoded limit never is ideal for\neverybody, you wouldn't even dream to suggest that raising the limit\nto another hardcoded value is better. Adding a configuration knob is\na way to treat people with respect and give them choice to suit the\nsystem to their taste.\n\nThe names of output files from format-patch is a very local matter,\nbecause once it is fed to \"git send-email\", the receiving end does\nnot even care how many letters you used for your filename---after\nall, you may not have used an intermediate file at all. It is a\ngood place to give personal choice, as opposed to parts of the\nsystem where interaction among multiple people happens (e.g. we\nwouldn't dream of making the characters allowed in refnames\nconfigurable---that would cause chaos between hosting sites and\nfetchers), where we give more careful thought before making things\nconfigurable in order to avoid fracturing the ecosystem.\n\nThanks.\n\n\n[Footnote]\n\n*1* ... exactly because we know that most users who used Git in the\npast 15 years were happy with the current limit as we haven't seen\nmuch complaint until your patch. We cannot tell if they wanted the\nlimit to be shorter, or longer, though.\n\n"}]}