{"thread":{"id":"52019","subject":"[PATCH v4 1/2] format-patch: create leading components of output directory","startedAt":"2019-10-11T08:36:47Z","lastAt":"2019-10-11T16:17:14Z","messageCount":6,"participants":["Bert Wesarg","SZEDER Gábor"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"383884","messageId":"b172eba0b748c3f0f638786a5cfba905aca385cc.1570782773.git.bert.wesarg@googlemail.com","threadId":"52019","inReplyTo":null,"subject":"[PATCH v4 1/2] format-patch: create leading components of output directory","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-10-11T08:36:41Z","receivedAt":"2019-10-11T08:36:47Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"'git format-patch -o <outdir>' did an equivalent of 'mkdir <outdir>'\nnot 'mkdir -p <outdir>', which is being corrected.\n\nAvoid the usage of 'adjust_shared_perm' on the leading directories which\nmay have security implications. Achieved by temporarily disabling of\n'config.sharedRepository' like 'git init' does.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\n\nChanges in v2:\n * squashed and base new tests on 'dl/format-patch-doc-test-cleanup'\n\nChanges in v3:\n * avoid applying adjust_shared_perm\n\nChanges in v4:\n * based on dl/format-patch-doc-test-cleanup and adopt it\n\nCc: Denton Liu <liu.denton@gmail.com>\nCc: Junio C Hamano <gitster@pobox.com>\nCc: SZEDER Gábor <szeder.dev@gmail.com>\n---\n Documentation/config/format.txt    |  2 +-\n Documentation/git-format-patch.txt |  3 ++-\n builtin/log.c                      | 16 ++++++++++++++++\n t/t4014-format-patch.sh            | 23 +++++++++++++++++++++++\n 4 files changed, 42 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex cb629fa769..40cad9278f 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -81,7 +81,7 @@ format.coverLetter::\n \n format.outputDirectory::\n \tSet a custom directory to store the resulting files instead of the\n-\tcurrent working directory.\n+\tcurrent working directory. All directory components will be created.\n \n format.useAutoBase::\n \tA boolean value which lets you enable the `--base=auto` option of\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 0ac56f4b70..2035d4d5d5 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -66,7 +66,8 @@ they are created in the current working directory. The default path\n can be set with the `format.outputDirectory` configuration option.\n The `-o` option takes precedence over `format.outputDirectory`.\n To store patches in the current working directory even when\n-`format.outputDirectory` points elsewhere, use `-o .`.\n+`format.outputDirectory` points elsewhere, use `-o .`. All directory\n+components will be created.\n \n By default, the subject of a single patch is \"[PATCH] \" followed by\n the concatenation of lines from the commit message up to the first blank\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 44b10b3415..8d08632858 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1765,10 +1765,26 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tsetup_pager();\n \n \tif (output_directory) {\n+\t\tint saved;\n \t\tif (rev.diffopt.use_color != GIT_COLOR_ALWAYS)\n \t\t\trev.diffopt.use_color = GIT_COLOR_NEVER;\n \t\tif (use_stdout)\n \t\t\tdie(_(\"standard output, or directory, which one?\"));\n+\t\t/*\n+\t\t * We consider <outdir> as 'outside of gitdir', therefore avoid\n+\t\t * applying adjust_shared_perm in s-c-l-d.\n+\t\t */\n+\t\tsaved = get_shared_repository();\n+\t\tset_shared_repository(0);\n+\t\tswitch (safe_create_leading_directories_const(output_directory)) {\n+\t\tcase SCLD_OK:\n+\t\tcase SCLD_EXISTS:\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tdie(_(\"could not create leading directories \"\n+\t\t\t      \"of '%s'\"), output_directory);\n+\t\t}\n+\t\tset_shared_repository(saved);\n \t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n \t\t\tdie_errno(_(\"could not create directory '%s'\"),\n \t\t\t\t  output_directory);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 72b09896cf..9facc3a79e 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' '\n \ttest_cmp from filtered\n '\n \n+test_expect_success 'format-patch -o with no leading directories' '\n+\trm -fr patches &&\n+\tgit format-patch -o patches master..side &&\n+\tcount=$(git rev-list --count master..side) &&\n+\tls patches >list &&\n+\ttest_line_count = $count list\n+'\n+\n+test_expect_success 'format-patch -o with leading existing directories' '\n+\tgit format-patch -o patches/side master..side &&\n+\tcount=$(git rev-list --count master..side) &&\n+\tls patches/side >list &&\n+\ttest_line_count = $count list\n+'\n+\n+test_expect_success 'format-patch -o with leading non-existing directories' '\n+\trm -fr patches &&\n+\tgit format-patch -o patches/side master..side &&\n+\tcount=$(git rev-list --count master..side) &&\n+\tls patches/side >list &&\n+\ttest_line_count = $count list\n+'\n+\n test_expect_success 'format-patch format.outputDirectory option' '\n \ttest_config format.outputDirectory patches &&\n \trm -fr patches &&\n-- \n2.23.0.13.g28bc381d7c\n\n"},{"id":"383885","messageId":"bdca9086256c6723f13712b1e80b0f1e29dafe9b.1570782773.git.bert.wesarg@googlemail.com","threadId":"52019","inReplyTo":"b172eba0b748c3f0f638786a5cfba905aca385cc.1570782773.git.bert.wesarg@googlemail.com","subject":"[PATCH v4 2/2] format-patch: configure a command to generate the output directory name","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-10-11T08:36:42Z","receivedAt":"2019-10-11T08:36:48Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"The 'format.outputDirectory' configuration is only able to store constant\ndirectory names. Though some may use\n\n   $ git format-patch -o $(createdir) …\n\nto name the directory dynamically. Provide a new configuration to be able\nto store such a command too.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nChanges in v2:\n * rephrase motivation\n\nChanges in v3:\n * remove RFC\n\nChanges in v4:\n * based on dl/format-patch-doc-test-cleanup and adopt it\n\nCc: Alexander Kuleshov <kuleshovmail@gmail.com>\nCc: Eric Sunshine <sunshine@sunshineco.com>\nCc: Denton Liu <liu.denton@gmail.com>\nCc: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config/format.txt    |  5 +++++\n Documentation/git-format-patch.txt |  6 +++++-\n builtin/log.c                      | 24 +++++++++++++++++++++++-\n t/t4014-format-patch.sh            | 26 ++++++++++++++++++++++++++\n 4 files changed, 59 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex 40cad9278f..420188a1c6 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -83,6 +83,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.outputDirectoryCmd::\n+\tThe command which is used to name a custom directory to store the\n+\tresulting files instead of the current working directory. All directory\n+\tcomponents will be created.\n+\n format.useAutoBase::\n \tA boolean value which lets you enable the `--base=auto` option of\n \tformat-patch by default.\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 2035d4d5d5..4936b9f91d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -67,7 +67,11 @@ can be set with the `format.outputDirectory` configuration option.\n The `-o` option takes precedence over `format.outputDirectory`.\n To store patches in the current working directory even when\n `format.outputDirectory` points elsewhere, use `-o .`. All directory\n-components will be created.\n+components will be created. The 'format.outputDirectoryCmd' configuration can\n+be used to name a command to produce the directory name programmatically. The\n+command should produce the name to its standard output. The\n+`format.outputDirectory` configuration takes precedence over\n+`format.outputDirectoryCmd`.\n \n By default, the subject of a single patch is \"[PATCH] \" followed by\n the concatenation of lines from the commit message up to the first blank\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8d08632858..3eb507c02f 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -774,6 +774,7 @@ static const char *signature = git_version_string;\n static const char *signature_file;\n static int config_cover_letter;\n static const char *config_output_directory;\n+static const char *config_output_directory_cmd;\n \n enum {\n \tCOVER_UNSET,\n@@ -856,6 +857,8 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(var, \"format.outputdirectory\"))\n \t\treturn git_config_string(&config_output_directory, var, value);\n+\tif (!strcmp(var, \"format.outputdirectorycmd\"))\n+\t\treturn git_config_string(&config_output_directory_cmd, var, value);\n \tif (!strcmp(var, \"format.useautobase\")) {\n \t\tbase_auto = git_config_bool(var, value);\n \t\treturn 0;\n@@ -1756,8 +1759,27 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (rev.show_notes)\n \t\tinit_display_notes(&rev.notes_opt);\n \n-\tif (!output_directory && !use_stdout)\n+\tif (!output_directory && !use_stdout) {\n+\t\t// outputDirectoryCmd can be preceeded by outputDirectory\n+\t\tif (!config_output_directory && config_output_directory_cmd) {\n+\t\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\t\tconst char *argv[1];\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tint rc;\n+\n+\t\t\targv[0] = config_output_directory_cmd;\n+\t\t\tcp.argv = argv;\n+\t\t\tcp.use_shell = 1;\n+\t\t\trc = capture_command(&cp, &buf, PATH_MAX);\n+\t\t\tif (rc)\n+\t\t\t\tdie(_(\"outputDirectoryCmd command failed: \"\n+\t\t\t\t      \"'%s'\"), config_output_directory_cmd);\n+\t\t\tstrbuf_setlen(&buf, strcspn(buf.buf, \"\\r\\n\"));\n+\t\t\tconfig_output_directory = strbuf_detach(&buf, NULL);\n+\t\t}\n+\n \t\toutput_directory = config_output_directory;\n+\t}\n \n \tif (!use_stdout)\n \t\toutput_directory = set_outdir(prefix, output_directory);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 9facc3a79e..14c8f5e854 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1646,6 +1646,32 @@ test_expect_success 'format-patch -o overrides format.outputDirectory' '\n \ttest_path_is_dir patchset\n '\n \n+test_expect_success 'format-patch format.outputDirectoryCmd option' '\n+\ttest_config format.outputDirectoryCmd \"echo patches\" &&\n+\trm -fr patches &&\n+\tgit format-patch master..side &&\n+\tcount=$(git rev-list --count master..side) &&\n+\tls patches >list &&\n+\ttest_line_count = $count list\n+'\n+\n+test_expect_success 'format-patch format.outputDirectory overrides format.outputDirectoryCmd' '\n+\ttest_config format.outputDirectoryCmd \"echo patches\" &&\n+\ttest_config format.outputDirectory patchset &&\n+\trm -fr patches patchset &&\n+\tgit format-patch master..side &&\n+\ttest_path_is_missing patches &&\n+\ttest_path_is_dir patchset\n+'\n+\n+test_expect_success 'format-patch -o overrides format.outputDirectoryCmd' '\n+\ttest_config format.outputDirectoryCmd \"echo patches\" &&\n+\trm -fr patches patchset &&\n+\tgit format-patch -o patchset master..side &&\n+\ttest_path_is_missing patches &&\n+\ttest_path_is_dir patchset\n+'\n+\n test_expect_success 'format-patch --base' '\n \tgit checkout patchid &&\n \n-- \n2.23.0.13.g28bc381d7c\n\n"},{"id":"383891","messageId":"20191011144650.GM29845@szeder.dev","threadId":"52019","inReplyTo":"b172eba0b748c3f0f638786a5cfba905aca385cc.1570782773.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH v4 1/2] format-patch: create leading components of output directory","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-11T14:46:50Z","receivedAt":"2019-10-11T14:46:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 11, 2019 at 10:36:41AM +0200, Bert Wesarg wrote:\n> Changes in v4:\n>  * based on dl/format-patch-doc-test-cleanup and adopt it\n\nThanks...  but here I am nitpicking again, sorry :)\n\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 72b09896cf..9facc3a79e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' '\n>  \ttest_cmp from filtered\n>  '\n>  \n> +test_expect_success 'format-patch -o with no leading directories' '\n> +\trm -fr patches &&\n> +\tgit format-patch -o patches master..side &&\n> +\tcount=$(git rev-list --count master..side) &&\n> +\tls patches >list &&\n> +\ttest_line_count = $count list\n> +'\n> +\n> +test_expect_success 'format-patch -o with leading existing directories' '\n> +\tgit format-patch -o patches/side master..side &&\n\nThe previous test case creates the 'patches' directory and leaves it\nbehind, and this test implicitly relies on that directory to check\nthat 'format-patch -o' can deal with already existing directories.  So\nif the previous test case were to fail early or were not run at all\n(e.g. './t4014-format-patch.sh -r 1,137'), then that directory\nwouldn't exist, and, consequently, this test case would not check what\nit was supposed to.\n\nI think it would be better to be more explicit and self-contained\nabout it, and create a leading directory in this test case:\n\n   mkdir existing-dir &&\n   git format-patch -o existing-dir/side master..size &&\n   ls existing-dir/side >list &&\n\n> +\tcount=$(git rev-list --count master..side) &&\n> +\tls patches/side >list &&\n> +\ttest_line_count = $count list\n> +'\n> +\n> +test_expect_success 'format-patch -o with leading non-existing directories' '\n> +\trm -fr patches &&\n> +\tgit format-patch -o patches/side master..side &&\n> +\tcount=$(git rev-list --count master..side) &&\n> +\tls patches/side >list &&\n> +\ttest_line_count = $count list\n> +'\n> +\n>  test_expect_success 'format-patch format.outputDirectory option' '\n>  \ttest_config format.outputDirectory patches &&\n>  \trm -fr patches &&\n> -- \n> 2.23.0.13.g28bc381d7c\n> \n"},{"id":"383895","messageId":"CAKPyHN0s8Rc8Sbp7ubyJ5cTDC5oh4Re+wqyTUWUo+3HgpKfECg@mail.gmail.com","threadId":"52019","inReplyTo":"20191011144650.GM29845@szeder.dev","subject":"Re: [PATCH v4 1/2] format-patch: create leading components of output directory","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-10-11T15:45:18Z","receivedAt":"2019-10-11T15:45:33Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Fri, Oct 11, 2019 at 4:46 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n>\n> On Fri, Oct 11, 2019 at 10:36:41AM +0200, Bert Wesarg wrote:\n> > Changes in v4:\n> >  * based on dl/format-patch-doc-test-cleanup and adopt it\n>\n> Thanks...  but here I am nitpicking again, sorry :)\n>\n> > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> > index 72b09896cf..9facc3a79e 100755\n> > --- a/t/t4014-format-patch.sh\n> > +++ b/t/t4014-format-patch.sh\n> > @@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' '\n> >       test_cmp from filtered\n> >  '\n> >\n> > +test_expect_success 'format-patch -o with no leading directories' '\n> > +     rm -fr patches &&\n> > +     git format-patch -o patches master..side &&\n> > +     count=$(git rev-list --count master..side) &&\n> > +     ls patches >list &&\n> > +     test_line_count = $count list\n> > +'\n> > +\n> > +test_expect_success 'format-patch -o with leading existing directories' '\n> > +     git format-patch -o patches/side master..side &&\n>\n> The previous test case creates the 'patches' directory and leaves it\n> behind, and this test implicitly relies on that directory to check\n> that 'format-patch -o' can deal with already existing directories.  So\n> if the previous test case were to fail early or were not run at all\n> (e.g. './t4014-format-patch.sh -r 1,137'), then that directory\n> wouldn't exist, and, consequently, this test case would not check what\n> it was supposed to.\n>\n> I think it would be better to be more explicit and self-contained\n> about it, and create a leading directory in this test case:\n>\n>    mkdir existing-dir &&\n>    git format-patch -o existing-dir/side master..size &&\n>    ls existing-dir/side >list &&\n>\n\nthanks. Your nitpicking is always appreciated.\n\nBert\n\n> > +     count=$(git rev-list --count master..side) &&\n> > +     ls patches/side >list &&\n> > +     test_line_count = $count list\n> > +'\n> > +\n> > +test_expect_success 'format-patch -o with leading non-existing directories' '\n> > +     rm -fr patches &&\n> > +     git format-patch -o patches/side master..side &&\n> > +     count=$(git rev-list --count master..side) &&\n> > +     ls patches/side >list &&\n> > +     test_line_count = $count list\n> > +'\n> > +\n> >  test_expect_success 'format-patch format.outputDirectory option' '\n> >       test_config format.outputDirectory patches &&\n> >       rm -fr patches &&\n> > --\n> > 2.23.0.13.g28bc381d7c\n> >\n"},{"id":"383896","messageId":"CAKPyHN22-SnZ2_o1MO6EXuWk7S2AAWGQSjxu0OrhDq4pw=ap=g@mail.gmail.com","threadId":"52019","inReplyTo":"CAKPyHN0s8Rc8Sbp7ubyJ5cTDC5oh4Re+wqyTUWUo+3HgpKfECg@mail.gmail.com","subject":"Re: [PATCH v4 1/2] format-patch: create leading components of output directory","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-10-11T15:47:44Z","receivedAt":"2019-10-11T15:47:58Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Fri, Oct 11, 2019 at 5:45 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n>\n> On Fri, Oct 11, 2019 at 4:46 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n> >\n> > On Fri, Oct 11, 2019 at 10:36:41AM +0200, Bert Wesarg wrote:\n> > > Changes in v4:\n> > >  * based on dl/format-patch-doc-test-cleanup and adopt it\n> >\n> > Thanks...  but here I am nitpicking again, sorry :)\n> >\n> > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> > > index 72b09896cf..9facc3a79e 100755\n> > > --- a/t/t4014-format-patch.sh\n> > > +++ b/t/t4014-format-patch.sh\n> > > @@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' '\n> > >       test_cmp from filtered\n> > >  '\n> > >\n> > > +test_expect_success 'format-patch -o with no leading directories' '\n> > > +     rm -fr patches &&\n> > > +     git format-patch -o patches master..side &&\n> > > +     count=$(git rev-list --count master..side) &&\n> > > +     ls patches >list &&\n> > > +     test_line_count = $count list\n> > > +'\n> > > +\n> > > +test_expect_success 'format-patch -o with leading existing directories' '\n> > > +     git format-patch -o patches/side master..side &&\n> >\n> > The previous test case creates the 'patches' directory and leaves it\n> > behind, and this test implicitly relies on that directory to check\n> > that 'format-patch -o' can deal with already existing directories.  So\n> > if the previous test case were to fail early or were not run at all\n> > (e.g. './t4014-format-patch.sh -r 1,137'), then that directory\n> > wouldn't exist, and, consequently, this test case would not check what\n> > it was supposed to.\n> >\n> > I think it would be better to be more explicit and self-contained\n> > about it, and create a leading directory in this test case:\n> >\n> >    mkdir existing-dir &&\n> >    git format-patch -o existing-dir/side master..size &&\n> >    ls existing-dir/side >list &&\n\none question: How about removing this directory first, just to be\nsure, that mkdir does create a directory?\n\nBert\n\n> >\n>\n> thanks. Your nitpicking is always appreciated.\n>\n> Bert\n>\n> > > +     count=$(git rev-list --count master..side) &&\n> > > +     ls patches/side >list &&\n> > > +     test_line_count = $count list\n> > > +'\n> > > +\n> > > +test_expect_success 'format-patch -o with leading non-existing directories' '\n> > > +     rm -fr patches &&\n> > > +     git format-patch -o patches/side master..side &&\n> > > +     count=$(git rev-list --count master..side) &&\n> > > +     ls patches/side >list &&\n> > > +     test_line_count = $count list\n> > > +'\n> > > +\n> > >  test_expect_success 'format-patch format.outputDirectory option' '\n> > >       test_config format.outputDirectory patches &&\n> > >       rm -fr patches &&\n> > > --\n> > > 2.23.0.13.g28bc381d7c\n> > >\n"},{"id":"383902","messageId":"20191011161707.GN29845@szeder.dev","threadId":"52019","inReplyTo":"CAKPyHN22-SnZ2_o1MO6EXuWk7S2AAWGQSjxu0OrhDq4pw=ap=g@mail.gmail.com","subject":"Re: [PATCH v4 1/2] format-patch: create leading components of output directory","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-11T16:17:07Z","receivedAt":"2019-10-11T16:17:14Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 11, 2019 at 05:47:44PM +0200, Bert Wesarg wrote:\n> > > > +test_expect_success 'format-patch -o with no leading directories' '\n> > > > +     rm -fr patches &&\n> > > > +     git format-patch -o patches master..side &&\n> > > > +     count=$(git rev-list --count master..side) &&\n> > > > +     ls patches >list &&\n> > > > +     test_line_count = $count list\n> > > > +'\n> > > > +\n> > > > +test_expect_success 'format-patch -o with leading existing directories' '\n> > > > +     git format-patch -o patches/side master..side &&\n> > >\n> > > The previous test case creates the 'patches' directory and leaves it\n> > > behind, and this test implicitly relies on that directory to check\n> > > that 'format-patch -o' can deal with already existing directories.  So\n> > > if the previous test case were to fail early or were not run at all\n> > > (e.g. './t4014-format-patch.sh -r 1,137'), then that directory\n> > > wouldn't exist, and, consequently, this test case would not check what\n> > > it was supposed to.\n> > >\n> > > I think it would be better to be more explicit and self-contained\n> > > about it, and create a leading directory in this test case:\n> > >\n> > >    mkdir existing-dir &&\n> > >    git format-patch -o existing-dir/side master..size &&\n> > >    ls existing-dir/side >list &&\n> \n> one question: How about removing this directory first, just to be\n> sure, that mkdir does create a directory?\n\nI'm not sure I understand...\n\nDo you mean that a previous test might have already created and left a\ndirectory with the same name behind, and then this 'mkdir' would error\nout and thus fail the test?  If yes, then you're right with your\nnitpicking on my nitpicking ;)  Though instead of 'rm -rf'ing that\ndirectory I would suggest 'mkdir -p' to simply ignore it if it were to\nexist.\n\n"}]}