{"thread":{"id":"52025","subject":"[PATCH 1/2] merge-recursive: clean up get_renamed_dir_portion()","startedAt":"2019-10-11T20:42:29Z","lastAt":"2019-10-31T08:29:13Z","messageCount":21,"participants":["Elijah Newren via GitGitGadget","Johannes Schindelin","Elijah Newren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"383931","messageId":"8ae78679c9066a206610465e1da91a5f575fced2.1570826543.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] merge-recursive: clean up get_renamed_dir_portion()","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-11T20:42:24Z","receivedAt":"2019-10-11T20:42:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nDscho noted a few things making this function hard to follow.\nRestructure it a bit and add comments to make it easier to follow. The\nrestructurings include:\n\n * There was a special case if-check at the end of the function\n checking whether someone just renamed a file within its original\n directory, meaning that there could be no directory rename involved.\n That check was slightly convoluted; it could be done in a more\n straightforward fashion earlier in the function, and can be done\n more cheaply too (no call to strncmp).\n\n * The conditions for advancing end_of_old and end_of_new before\n calling strchr were both confusing and unnecessary. If either\n points at a '/', then they need to be advanced in order to find the\n next '/'. If either doesn't point at a '/', then advancing them one\n char before calling strchr() doesn't hurt. So, just rip out the\n if conditions and advance both before calling strchr().\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 60 ++++++++++++++++++++++++++++-------------------\n 1 file changed, 36 insertions(+), 24 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 22a12cfeba..f80e48f623 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1943,8 +1943,8 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t\t\t\t char **old_dir, char **new_dir)\n {\n \tchar *end_of_old, *end_of_new;\n-\tint old_len, new_len;\n \n+\t/* Default return values: NULL, meaning no rename */\n \t*old_dir = NULL;\n \t*new_dir = NULL;\n \n@@ -1955,43 +1955,55 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t * \"a/b/c/d\" was renamed to \"a/b/some/thing/else\"\n \t * so, for this example, this function returns \"a/b/c/d\" in\n \t * *old_dir and \"a/b/some/thing/else\" in *new_dir.\n-\t *\n-\t * Also, if the basename of the file changed, we don't care. We\n-\t * want to know which portion of the directory, if any, changed.\n+\t */\n+\n+\t/*\n+\t * If the basename of the file changed, we don't care. We want\n+\t * to know which portion of the directory, if any, changed.\n \t */\n \tend_of_old = strrchr(old_path, '/');\n \tend_of_new = strrchr(new_path, '/');\n-\n \tif (end_of_old == NULL || end_of_new == NULL)\n-\t\treturn;\n+\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n+\n+\t/* Find the first non-matching character traversing backwards */\n \twhile (*--end_of_new == *--end_of_old &&\n \t end_of_old != old_path &&\n \t end_of_new != new_path)\n \t\t; /* Do nothing; all in the while loop */\n+\n \t/*\n-\t * We've found the first non-matching character in the directory\n-\t * paths. That means the current directory we were comparing\n-\t * represents the rename. Move end_of_old and end_of_new back\n-\t * to the full directory name.\n+\t * If both got back to the beginning of their strings, then the\n+\t * directory didn't change at all, only the basename did.\n \t */\n-\tif (*end_of_old == '/')\n-\t\tend_of_old++;\n-\tif (*end_of_old != '/')\n-\t\tend_of_new++;\n-\tend_of_old = strchr(end_of_old, '/');\n-\tend_of_new = strchr(end_of_new, '/');\n+\tif (end_of_old == old_path && end_of_new == new_path &&\n+\t *end_of_old == *end_of_new)\n+\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n \n \t/*\n-\t * It may have been the case that old_path and new_path were the same\n-\t * directory all along. Don't claim a rename if they're the same.\n+\t * We've found the first non-matching character in the directory\n+\t * paths. That means the current characters we were looking at\n+\t * were part of the first non-matching subdir name going back from\n+\t * the end of the strings. Get the whole name by advancing both\n+\t * end_of_old and end_of_new to the NEXT '/' character. That will\n+\t * represent the entire directory rename.\n+\t *\n+\t * The reason for the increment is cases like\n+\t * a/b/star/foo/whatever.c -> a/b/tar/foo/random.c\n+\t * After dropping the basename and going back to the first\n+\t * non-matching character, we're now comparing:\n+\t * a/b/s and a/b/\n+\t * and we want to be comparing:\n+\t * a/b/star/ and a/b/tar/\n+\t * but without the pre-increment, the one on the right would stay\n+\t * a/b/.\n \t */\n-\told_len = end_of_old - old_path;\n-\tnew_len = end_of_new - new_path;\n+\tend_of_old = strchr(++end_of_old, '/');\n+\tend_of_new = strchr(++end_of_new, '/');\n \n-\tif (old_len != new_len || strncmp(old_path, new_path, old_len)) {\n-\t\t*old_dir = xstrndup(old_path, old_len);\n-\t\t*new_dir = xstrndup(new_path, new_len);\n-\t}\n+\t/* Copy the old and new directories into *old_dir and *new_dir. */\n+\t*old_dir = xstrndup(old_path, end_of_old - old_path);\n+\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n }\n \n static void remove_hashmap_entries(struct hashmap *dir_renames,\n-- \ngitgitgadget\n\n"},{"id":"383932","messageId":"pull.390.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":null,"subject":"[PATCH 0/2] Dir rename fixes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-11T20:42:23Z","receivedAt":"2019-10-11T20:42:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series improves a couple things found after looking into things Dscho\nflagged:\n\n * clarify and slightly restructure code in the get_renamed_dir_portion()\n function\n * extend support of detecting renaming/merging of one directory into\n another to support the root directory as a target directory\n\nFirst patch best viewed with a --histogram diff, which I sadly don't know\nhow to make gitgitgadget generate.\n\nElijah Newren (2):\n merge-recursive: clean up get_renamed_dir_portion()\n merge-recursive: fix merging a subdirectory into the root directory\n\n merge-recursive.c | 89 +++++++++++++++++++++--------\n t/t6043-merge-rename-directories.sh | 56 ++++++++++++++++++\n 2 files changed, 121 insertions(+), 24 deletions(-)\n\n\nbase-commit: 08da6496b61341ec45eac36afcc8f94242763468\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-390%2Fnewren%2Fdir-rename-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-390/newren/dir-rename-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/390\n-- \ngitgitgadget\n"},{"id":"383933","messageId":"37aee862e14b1352eb08485f15ea06bab33679df.1570826543.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-11T20:42:25Z","receivedAt":"2019-10-11T20:42:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWe allow renaming all entries in e.g. a directory named z/ into a\ndirectory named y/ to be detected as a z/ -> y/ rename, so that if the\nother side of history adds any files to the directory z/ in the mean\ntime, we can provide the hint that they should be moved to y/.\n\nThere is no reason to not allow 'y/' to be the root directory, but the\ncode did not handle that case correctly. Add a testcase and the\nnecessary special checks to support this case.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 29 +++++++++++++++\n t/t6043-merge-rename-directories.sh | 56 +++++++++++++++++++++++++++++\n 2 files changed, 85 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex f80e48f623..7bd4a7cf10 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1931,6 +1931,16 @@ static char *apply_dir_rename(struct dir_rename_entry *entry,\n \t\treturn NULL;\n \n \toldlen = strlen(entry->dir);\n+\tif (entry->new_dir.len == 0)\n+\t\t/*\n+\t\t * If someone renamed/merged a subdirectory into the root\n+\t\t * directory (e.g. 'some/subdir' -> ''), then we want to\n+\t\t * avoid returning\n+\t\t * '' + '/filename'\n+\t\t * as the rename; we need to make old_path + oldlen advance\n+\t\t * past the '/' character.\n+\t\t */\n+\t\toldlen++;\n \tnewlen = entry->new_dir.len + (strlen(old_path) - oldlen) + 1;\n \tstrbuf_grow(&new_path, newlen);\n \tstrbuf_addbuf(&new_path, &entry->new_dir);\n@@ -1980,6 +1990,25 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t *end_of_old == *end_of_new)\n \t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n \n+\t/*\n+\t * If end_of_new got back to the beginning of its string, and\n+\t * end_of_old got back to the beginning of some subdirectory, then\n+\t * we have a rename/merge of a subdirectory into the root, which\n+\t * needs slightly special handling.\n+\t *\n+\t * Note: There is no need to consider the opposite case, with a\n+\t * rename/merge of the root directory into some subdirectory.\n+\t * Our rule elsewhere that a directory which still exists is not\n+\t * considered to have been renamed means the root directory can\n+\t * never be renamed (because the root directory always exists).\n+\t */\n+\tif (end_of_new == new_path &&\n+\t end_of_old != old_path && end_of_old[-1] == '/') {\n+\t\t*old_dir = xstrndup(old_path, end_of_old-1 - old_path);\n+\t\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n+\t\treturn;\n+\t}\n+\n \t/*\n \t * We've found the first non-matching character in the directory\n \t * paths. That means the current characters we were looking at\ndiff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\nindex c966147d5d..b920bb0850 100755\n--- a/t/t6043-merge-rename-directories.sh\n+++ b/t/t6043-merge-rename-directories.sh\n@@ -4051,6 +4051,62 @@ test_expect_success '12c-check: Moving one directory hierarchy into another w/ c\n \t)\n '\n \n+# Testcase 12d, Rename/merge of subdirectory into the root\n+# Commit O: a/b/{foo.c}\n+# Commit A: foo.c\n+# Commit B: a/b/{foo.c,bar.c}\n+# Expected: a/b/{foo.c,bar.c}\n+\n+test_expect_success '12d-setup: Rename (merge) of subdirectory into the root' '\n+\ttest_create_repo 12d &&\n+\t(\n+\t\tcd 12d &&\n+\n+\t\tmkdir -p a/b/subdir &&\n+\t\ttest_commit a/b/subdir/foo.c &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\t\tgit branch B &&\n+\n+\t\tgit checkout A &&\n+\t\tmkdir subdir &&\n+\t\tgit mv a/b/subdir/foo.c.t subdir/foo.c.t &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"A\" &&\n+\n+\t\tgit checkout B &&\n+\t\ttest_commit a/b/bar.c\n+\t)\n+'\n+\n+test_expect_success '12d-check: Rename (merge) of subdirectory into the root' '\n+\t(\n+\t\tcd 12d &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\tgit -c merge.directoryRenames=true merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -s >out &&\n+\t\ttest_line_count = 2 out &&\n+\n+\t\tgit rev-parse >actual \\\n+\t\t\tHEAD:subdir/foo.c.t HEAD:bar.c.t &&\n+\t\tgit rev-parse >expect \\\n+\t\t\tO:a/b/subdir/foo.c.t B:a/b/bar.c.t &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tgit hash-object bar.c.t >actual &&\n+\t\tgit rev-parse B:a/b/bar.c.t >expect &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\ttest_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&\n+\t\ttest_must_fail git rev-parse HEAD:a/b/bar.c.t &&\n+\t\ttest_path_is_missing a/\n+\t)\n+'\n+\n ###########################################################################\n # SECTION 13: Checking informational and conflict messages\n #\n-- \ngitgitgadget\n"},{"id":"383971","messageId":"nycvar.QRO.7.76.6.1910122035350.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"pull.390.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Dir rename fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-12T18:41:11Z","receivedAt":"2019-10-12T18:41:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 11 Oct 2019, Elijah Newren via GitGitGadget wrote:\n\n> This series improves a couple things found after looking into things Dscho\n> flagged:\n>\n> * clarify and slightly restructure code in the get_renamed_dir_portion()\n> function\n>\n> * extend support of detecting renaming/merging of one directory into\n> another to support the root directory as a target directory\n\nWill have a look in a moment,\n\n> First patch best viewed with a --histogram diff, which I sadly don't know\n> how to make gitgitgadget generate.\n\nCurrently, there is no way to do that yet. It should not be too\ndifficult to implement code to pick up a footer in the PR description to\ndo that, I just don't have the time right now to do so.\n\nEssentially, it would have to imitate the already-existing code path\nthat picks up the `Cc:` footer in the PR description.\n\nIf you're interested, have a look at the `parsePullRequestDescription()`\nfunction in `lib/patch-series.ts` in\nhttps://github.com/gitgitgadget/gitgitgadget/.\n\nCiao,\nDscho\n\n> Elijah Newren (2):\n> merge-recursive: clean up get_renamed_dir_portion()\n> merge-recursive: fix merging a subdirectory into the root directory\n>\n> merge-recursive.c | 89 +++++++++++++++++++++--------\n> t/t6043-merge-rename-directories.sh | 56 ++++++++++++++++++\n> 2 files changed, 121 insertions(+), 24 deletions(-)\n>\n>\n> base-commit: 08da6496b61341ec45eac36afcc8f94242763468\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-390%2Fnewren%2Fdir-rename-fixes-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-390/newren/dir-rename-fixes-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/390\n> --\n> gitgitgadget\n>\n"},{"id":"383973","messageId":"nycvar.QRO.7.76.6.1910122147070.3272@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"8ae78679c9066a206610465e1da91a5f575fced2.1570826543.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] merge-recursive: clean up get_renamed_dir_portion()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-12T19:47:47Z","receivedAt":"2019-10-12T19:48:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 11 Oct 2019, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren \n>\n> Dscho noted a few things making this function hard to follow.\n> Restructure it a bit and add comments to make it easier to follow. The\n> restructurings include:\n>\n> * There was a special case if-check at the end of the function\n> checking whether someone just renamed a file within its original\n> directory, meaning that there could be no directory rename involved.\n> That check was slightly convoluted; it could be done in a more\n> straightforward fashion earlier in the function, and can be done\n> more cheaply too (no call to strncmp).\n>\n> * The conditions for advancing end_of_old and end_of_new before\n> calling strchr were both confusing and unnecessary. If either\n> points at a '/', then they need to be advanced in order to find the\n> next '/'. If either doesn't point at a '/', then advancing them one\n> char before calling strchr() doesn't hurt. So, just rip out the\n> if conditions and advance both before calling strchr().\n>\n> Signed-off-by: Elijah Newren \n\nThis commit message, as well as the patch, make a lot of sense to me.\nThank you for doing this!\n\nCiao,\nDscho\n\n> ---\n> merge-recursive.c | 60 ++++++++++++++++++++++++++++-------------------\n> 1 file changed, 36 insertions(+), 24 deletions(-)\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 22a12cfeba..f80e48f623 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1943,8 +1943,8 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n> \t\t\t\t char **old_dir, char **new_dir)\n> {\n> \tchar *end_of_old, *end_of_new;\n> -\tint old_len, new_len;\n>\n> +\t/* Default return values: NULL, meaning no rename */\n> \t*old_dir = NULL;\n> \t*new_dir = NULL;\n>\n> @@ -1955,43 +1955,55 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n> \t * \"a/b/c/d\" was renamed to \"a/b/some/thing/else\"\n> \t * so, for this example, this function returns \"a/b/c/d\" in\n> \t * *old_dir and \"a/b/some/thing/else\" in *new_dir.\n> -\t *\n> -\t * Also, if the basename of the file changed, we don't care. We\n> -\t * want to know which portion of the directory, if any, changed.\n> +\t */\n> +\n> +\t/*\n> +\t * If the basename of the file changed, we don't care. We want\n> +\t * to know which portion of the directory, if any, changed.\n> \t */\n> \tend_of_old = strrchr(old_path, '/');\n> \tend_of_new = strrchr(new_path, '/');\n> -\n> \tif (end_of_old == NULL || end_of_new == NULL)\n> -\t\treturn;\n> +\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n> +\n> +\t/* Find the first non-matching character traversing backwards */\n> \twhile (*--end_of_new == *--end_of_old &&\n> \t end_of_old != old_path &&\n> \t end_of_new != new_path)\n> \t\t; /* Do nothing; all in the while loop */\n> +\n> \t/*\n> -\t * We've found the first non-matching character in the directory\n> -\t * paths. That means the current directory we were comparing\n> -\t * represents the rename. Move end_of_old and end_of_new back\n> -\t * to the full directory name.\n> +\t * If both got back to the beginning of their strings, then the\n> +\t * directory didn't change at all, only the basename did.\n> \t */\n> -\tif (*end_of_old == '/')\n> -\t\tend_of_old++;\n> -\tif (*end_of_old != '/')\n> -\t\tend_of_new++;\n> -\tend_of_old = strchr(end_of_old, '/');\n> -\tend_of_new = strchr(end_of_new, '/');\n> +\tif (end_of_old == old_path && end_of_new == new_path &&\n> +\t *end_of_old == *end_of_new)\n> +\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n>\n> \t/*\n> -\t * It may have been the case that old_path and new_path were the same\n> -\t * directory all along. Don't claim a rename if they're the same.\n> +\t * We've found the first non-matching character in the directory\n> +\t * paths. That means the current characters we were looking at\n> +\t * were part of the first non-matching subdir name going back from\n> +\t * the end of the strings. Get the whole name by advancing both\n> +\t * end_of_old and end_of_new to the NEXT '/' character. That will\n> +\t * represent the entire directory rename.\n> +\t *\n> +\t * The reason for the increment is cases like\n> +\t * a/b/star/foo/whatever.c -> a/b/tar/foo/random.c\n> +\t * After dropping the basename and going back to the first\n> +\t * non-matching character, we're now comparing:\n> +\t * a/b/s and a/b/\n> +\t * and we want to be comparing:\n> +\t * a/b/star/ and a/b/tar/\n> +\t * but without the pre-increment, the one on the right would stay\n> +\t * a/b/.\n> \t */\n> -\told_len = end_of_old - old_path;\n> -\tnew_len = end_of_new - new_path;\n> +\tend_of_old = strchr(++end_of_old, '/');\n> +\tend_of_new = strchr(++end_of_new, '/');\n>\n> -\tif (old_len != new_len || strncmp(old_path, new_path, old_len)) {\n> -\t\t*old_dir = xstrndup(old_path, old_len);\n> -\t\t*new_dir = xstrndup(new_path, new_len);\n> -\t}\n> +\t/* Copy the old and new directories into *old_dir and *new_dir. */\n> +\t*old_dir = xstrndup(old_path, end_of_old - old_path);\n> +\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n> }\n>\n> static void remove_hashmap_entries(struct hashmap *dir_renames,\n> --\n> gitgitgadget\n>\n>\n"},{"id":"383975","messageId":"nycvar.QRO.7.76.6.1910122152210.3272@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"37aee862e14b1352eb08485f15ea06bab33679df.1570826543.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-12T20:37:26Z","receivedAt":"2019-10-12T20:37:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 11 Oct 2019, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren \n>\n> We allow renaming all entries in e.g. a directory named z/ into a\n> directory named y/ to be detected as a z/ -> y/ rename, so that if the\n> other side of history adds any files to the directory z/ in the mean\n> time, we can provide the hint that they should be moved to y/.\n>\n> There is no reason to not allow 'y/' to be the root directory, but the\n> code did not handle that case correctly. Add a testcase and the\n> necessary special checks to support this case.\n>\n> Signed-off-by: Elijah Newren \n\nThis makes sense.\n\n> ---\n> merge-recursive.c | 29 +++++++++++++++\n> t/t6043-merge-rename-directories.sh | 56 +++++++++++++++++++++++++++++\n\nIt is good to have a test case verifying this!\n\nFWIW I frequently run into those same issues because I have to use --\nquite often! -- `contrib/fast-import/import-tars.perl` in the Git for\nWindows project (in the MSYS2 part thereof, to be precise), and the\n`pax_global_header` and I will probably never become friends, so I often\nhave to move files into the top-level directory...\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index f80e48f623..7bd4a7cf10 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1931,6 +1931,16 @@ static char *apply_dir_rename(struct dir_rename_entry *entry,\n> \t\treturn NULL;\n>\n> \toldlen = strlen(entry->dir);\n> +\tif (entry->new_dir.len == 0)\n> +\t\t/*\n> +\t\t * If someone renamed/merged a subdirectory into the root\n> +\t\t * directory (e.g. 'some/subdir' -> ''), then we want to\n> +\t\t * avoid returning\n> +\t\t * '' + '/filename'\n> +\t\t * as the rename; we need to make old_path + oldlen advance\n> +\t\t * past the '/' character.\n> +\t\t */\n> +\t\toldlen++;\n\nThis makes sense to me.\n\n> \tnewlen = entry->new_dir.len + (strlen(old_path) - oldlen) + 1;\n> \tstrbuf_grow(&new_path, newlen);\n> \tstrbuf_addbuf(&new_path, &entry->new_dir);\n> @@ -1980,6 +1990,25 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n> \t *end_of_old == *end_of_new)\n> \t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n>\n> +\t/*\n> +\t * If end_of_new got back to the beginning of its string, and\n> +\t * end_of_old got back to the beginning of some subdirectory, then\n> +\t * we have a rename/merge of a subdirectory into the root, which\n> +\t * needs slightly special handling.\n> +\t *\n> +\t * Note: There is no need to consider the opposite case, with a\n> +\t * rename/merge of the root directory into some subdirectory.\n> +\t * Our rule elsewhere that a directory which still exists is not\n> +\t * considered to have been renamed means the root directory can\n> +\t * never be renamed (because the root directory always exists).\n> +\t */\n> +\tif (end_of_new == new_path &&\n> +\t end_of_old != old_path && end_of_old[-1] == '/') {\n> +\t\t*old_dir = xstrndup(old_path, end_of_old-1 - old_path);\n> +\t\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n\nHowever, here we write something convoluted that essentially amounts to\n`xstrdup(\"\")`. I would rather have that simple call than the convoluted\none that would puzzle me every time I have to look at this part of the\ncode.\n\nWhile at it, would you mind either surrounding the `-` and the `1` by\nspaces, or even write `--end_of_old - old_path`?\n\n> +\t\treturn;\n> +\t}\n> +\n> \t/*\n> \t * We've found the first non-matching character in the directory\n> \t * paths. That means the current characters we were looking at\n> diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\n> index c966147d5d..b920bb0850 100755\n> --- a/t/t6043-merge-rename-directories.sh\n> +++ b/t/t6043-merge-rename-directories.sh\n> @@ -4051,6 +4051,62 @@ test_expect_success '12c-check: Moving one directory hierarchy into another w/ c\n> \t)\n> '\n>\n> +# Testcase 12d, Rename/merge of subdirectory into the root\n> +# Commit O: a/b/{foo.c}\n> +# Commit A: foo.c\n> +# Commit B: a/b/{foo.c,bar.c}\n> +# Expected: a/b/{foo.c,bar.c}\n> +\n> +test_expect_success '12d-setup: Rename (merge) of subdirectory into the root' '\n> +\ttest_create_repo 12d &&\n> +\t(\n> +\t\tcd 12d &&\n> +\n> +\t\tmkdir -p a/b/subdir &&\n> +\t\ttest_commit a/b/subdir/foo.c &&\n\nWhy `.c`? That's a little distracting.\n\n> +\n> +\t\tgit branch O &&\n\nMight be simpler just to use `master` subsequently and not \"waste\" a new\nref on that.\n\n> +\t\tgit branch A &&\n\nMight make more sense to create it below, via the `-b` option of `git\ncheckout`.\n\nOr, for extra brownie points, via the `-c` option of `git switch`.\n\n> +\t\tgit branch B &&\n\nLikewise, this might want to be created below, via replacing `git\ncheckout B` with `git switch -c B master`.\n\n> +\n> +\t\tgit checkout A &&\n> +\t\tmkdir subdir &&\n> +\t\tgit mv a/b/subdir/foo.c.t subdir/foo.c.t &&\n> +\t\ttest_tick &&\n> +\t\tgit commit -m \"A\" &&\n> +\n> +\t\tgit checkout B &&\n> +\t\ttest_commit a/b/bar.c\n> +\t)\n> +'\n> +\n> +test_expect_success '12d-check: Rename (merge) of subdirectory into the root' '\n\nFor the record: I am still a huge anti-fan of splitting `setup` test\ncases from the test cases that do actual things, _unless_ it is _one_,\nand _only one_, big, honking `setup` test case that is the very first\none in the test script.\n\nSplitting things into two inevitably leads to unnecessary churn when\ninvestigating test failures.\n\nAnd that's really what test cases need to be optimized for: when they\nreport breakages. They need to help as much as they can to investigate\nwhy things break. Nobody cares when test cases succeed. The ~150k test\ncases that pass on every CI build: nobody is interested. When a test\ncase reports failure, that's when people pay attention. At least some,\nincluding me.\n\nThe most common case for me (and every other lazy person who relies on\nCI/PR builds) is when a build breaks, and then I usually get to the\ntrace of the actually failing test case very quickly. The previous test\ncase's trace, not so easy. Clicks involved. Now I lose context. Not\ngood.\n\nA less common case for me is when I run test scripts locally, with `-i\n-v -x`. Still, I need to scroll back to get context. And then, really, I\nalready lost context.\n\n> +\t(\n> +\t\tcd 12d &&\n> +\n> +\t\tgit checkout A^0 &&\n> +\n> +\t\tgit -c merge.directoryRenames=true merge -s recursive B^0 &&\n> +\n> +\t\tgit ls-files -s >out &&\n> +\t\ttest_line_count = 2 out &&\n\nIsn't this a bit overzealous?\n\n> +\n> +\t\tgit rev-parse >actual \\\n> +\t\t\tHEAD:subdir/foo.c.t HEAD:bar.c.t &&\n> +\t\tgit rev-parse >expect \\\n> +\t\t\tO:a/b/subdir/foo.c.t B:a/b/bar.c.t &&\n> +\t\ttest_cmp expect actual &&\n\nLikewise?\n\n> +\n> +\t\tgit hash-object bar.c.t >actual &&\n> +\t\tgit rev-parse B:a/b/bar.c.t >expect &&\n> +\t\ttest_cmp expect actual &&\n\nLikewise?\n\n> +\n> +\t\ttest_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&\n> +\t\ttest_must_fail git rev-parse HEAD:a/b/bar.c.t &&\n> +\t\ttest_path_is_missing a/\n\nMakes sense, but the part that I am missing is\n\n\t\ttest_path_is_file bar.c.t\n\nI.e. the _most_ important outcome of this test is: the rename was\ndetected, and the added file was correctly placed into the target\ndirectory of the rename.\n\nThanks,\nDscho\n\n> +\t)\n> +'\n> +\n> ###########################################################################\n> # SECTION 13: Checking informational and conflict messages\n> #\n> --\n> gitgitgadget\n>\n"},{"id":"383984","messageId":"CABPp-BFNCLJnt4NgFKVxURBGD1Z00gastc5q4ZPjcHmwS=kuFw@mail.gmail.com","threadId":"52025","inReplyTo":"nycvar.QRO.7.76.6.1910122152210.3272@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-10-13T00:40:25Z","receivedAt":"2019-10-13T00:40:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nThanks for the reviews!\n\nOn Sat, Oct 12, 2019 at 1:37 PM Johannes Schindelin\n wrote:\n> On Fri, 11 Oct 2019, Elijah Newren via GitGitGadget wrote:\n>\n[...]\n> > @@ -1980,6 +1990,25 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n> > *end_of_old == *end_of_new)\n> > return; /* We haven't modified *old_dir or *new_dir yet. */\n> >\n> > + /*\n> > + * If end_of_new got back to the beginning of its string, and\n> > + * end_of_old got back to the beginning of some subdirectory, then\n> > + * we have a rename/merge of a subdirectory into the root, which\n> > + * needs slightly special handling.\n> > + *\n> > + * Note: There is no need to consider the opposite case, with a\n> > + * rename/merge of the root directory into some subdirectory.\n> > + * Our rule elsewhere that a directory which still exists is not\n> > + * considered to have been renamed means the root directory can\n> > + * never be renamed (because the root directory always exists).\n> > + */\n> > + if (end_of_new == new_path &&\n> > + end_of_old != old_path && end_of_old[-1] == '/') {\n> > + *old_dir = xstrndup(old_path, end_of_old-1 - old_path);\n> > + *new_dir = xstrndup(new_path, end_of_new - new_path);\n>\n> However, here we write something convoluted that essentially amounts to\n> `xstrdup(\"\")`. I would rather have that simple call than the convoluted\n> one that would puzzle me every time I have to look at this part of the\n> code.\n\nMakes sense; I can switch it over.\n\n>\n> While at it, would you mind either surrounding the `-` and the `1` by\n> spaces, or even write `--end_of_old - old_path`?\n\nSounds good to me; I'll make the change.\n\n\n> > diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\n> > index c966147d5d..b920bb0850 100755\n> > --- a/t/t6043-merge-rename-directories.sh\n> > +++ b/t/t6043-merge-rename-directories.sh\n> > @@ -4051,6 +4051,62 @@ test_expect_success '12c-check: Moving one directory hierarchy into another w/ c\n> > )\n> > '\n> >\n> > +# Testcase 12d, Rename/merge of subdirectory into the root\n> > +# Commit O: a/b/{foo.c}\n> > +# Commit A: foo.c\n> > +# Commit B: a/b/{foo.c,bar.c}\n> > +# Expected: a/b/{foo.c,bar.c}\n\nNote the nice explanation of the testcase setup at the beginning of\nevery test within this file...\n\n> > +\n> > +test_expect_success '12d-setup: Rename (merge) of subdirectory into the root' '\n> > + test_create_repo 12d &&\n> > + (\n> > + cd 12d &&\n> > +\n> > + mkdir -p a/b/subdir &&\n> > + test_commit a/b/subdir/foo.c &&\n>\n> Why `.c`? That's a little distracting.\n\nI can toss it.\n\n> > +\n> > + git branch O &&\n>\n> Might be simpler just to use `master` subsequently and not \"waste\" a new\n> ref on that.\n\nI could do so, but then this makes the testcase description comment\nearlier harder to read comparing \"master\", \"A\", and \"B\". Having the\nsame length simplifies it a bit, and the triple of O, A, and B are\nalso used quite a bit in merge-recursive.c (e.g. in process_entry()\nand merge_threeway() and other places).\n\nAlso, behaving differently for this test than the other 50+ tests in\nthe testfile would break the comment at the beginning of t6043 which\nexplains how *every* test in the file is of a certain form, using O,\nA, and B.\n\n>\n> > + git branch A &&\n>\n> Might make more sense to create it below, via the `-b` option of `git\n> checkout`.\n>\n> Or, for extra brownie points, via the `-c` option of `git switch`.\n>\n> > + git branch B &&\n>\n> Likewise, this might want to be created below, via replacing `git\n> checkout B` with `git switch -c B master`.\n\nI'm not sure I see why it'd be beneficial to switch this, though in\nisolation I also don't see any drawbacks with your suggestion either.\nIt looks entirely reasonable, so I'd probably just do it if it weren't\nfor the fact that there are four dozen or so other tests in the same\nfile that already do it this way. I'd rather keep the file internally\nconsistent, and there's a bit too much inertia for me to want to\nswitch all the tests over...unless you can provide a reason to\nstrongly prefer one style over the other?\n\n> > +\n> > + git checkout A &&\n> > + mkdir subdir &&\n> > + git mv a/b/subdir/foo.c.t subdir/foo.c.t &&\n> > + test_tick &&\n> > + git commit -m \"A\" &&\n> > +\n> > + git checkout B &&\n> > + test_commit a/b/bar.c\n> > + )\n> > +'\n> > +\n> > +test_expect_success '12d-check: Rename (merge) of subdirectory into the root' '\n>\n> For the record: I am still a huge anti-fan of splitting `setup` test\n> cases from the test cases that do actual things, _unless_ it is _one_,\n> and _only one_, big, honking `setup` test case that is the very first\n> one in the test script.\n>\n> Splitting things into two inevitably leads to unnecessary churn when\n> investigating test failures.\n>\n> And that's really what test cases need to be optimized for: when they\n> report breakages. They need to help as much as they can to investigate\n> why things break. Nobody cares when test cases succeed. The ~150k test\n> cases that pass on every CI build: nobody is interested. When a test\n> case reports failure, that's when people pay attention. At least some,\n> including me.\n>\n> The most common case for me (and every other lazy person who relies on\n> CI/PR builds) is when a build breaks, and then I usually get to the\n> trace of the actually failing test case very quickly. The previous test\n> case's trace, not so easy. Clicks involved. Now I lose context. Not\n> good.\n>\n> A less common case for me is when I run test scripts locally, with `-i\n> -v -x`. Still, I need to scroll back to get context. And then, really, I\n> already lost context.\n\nI guess we have some strongly differing opinions here. The one thing\nI do agree with you on is test cases need to be optimized for when\nthey report breakages, but that is precisely what led me to splitting\nsetup and testing. Way too many tests in the testsuite intersperse\nseveral setup and test cases together making it really hard to\ndisentangle, understand what is going on, or even reverse engineer\nwhat is relevant. The absolute worst tests are the ones which just\nkeep making additional changes to some existing repo to provide extra\nsetup, causing all sorts of problems for skipping and resuming and\nunderstanding (to say nothing of test prerequisites that aren't always\nmet). But the ones with one big honking `setup` test case that is the\nvery first one in the test script can often be pretty bad too when\nyou've found a bug in testcase 82 and want to have a simple way to\nreproduce. It's awesome when someone can just run ./testcase.sh --ver\n--imm -x --run=86,87 and reproduce the problem. It feels sadly rare\nto be able to do that in much of git.git's testsuite. Not accreting\never more changes into a repo to setup subsequent problems, using\nentirely separate repos for different cases where testing makes any\nchanges, making it clear what is setup, making it clear what's the\ncommand being tested, and making it clear what all the commands are\nthat are testing that the original test command produced the expected\nbehavior all go a long way to making things way easier to investigate.\n\nNow, I will re-use a setup case for multiple tests and even did so in\nt6043, but only when the setup really is identical (e.g. not only are\nthe tests expecting an identical setup, but the commands being tested\nare read-only or any changes they make are unconditionally reverted as\nthe last step of the testcase). Naturally, when I re-use a setup\nblock multiple times, I really don't want to copy the setup into each\nof those tests either.\n\nIt would be really nice, though, if there were some way for me to\nspecify that a given \"testcase\" was just setup (so that they don't\neven get a number in the prove output and don't count as a \"test\"\nexcept maybe when they fail), and if there were some way to specify\nwhich testcases depend on which setup blocks (so that if I want to run\njust a given test, the setup test gets automatically included).\n\n\nYour example of CI/PR builds makes sense to me, and I really would\nlike to make that nicer and improve other workflows too, especially if\nit can be done without breaking my ability to investigate test\nfailures. If someone can think of a clever way to do that (e.g. some\nway of implementing my \"It would be really nice\" in the last\nparagraph, and in a way that could also benefit CI/PR builds), I'd be\nhappy to go through and help switch things over.\n\n> > + (\n> > + cd 12d &&\n> > +\n> > + git checkout A^0 &&\n> > +\n> > + git -c merge.directoryRenames=true merge -s recursive B^0 &&\n> > +\n> > + git ls-files -s >out &&\n> > + test_line_count = 2 out &&\n>\n> Isn't this a bit overzealous?\n>\n> > +\n> > + git rev-parse >actual \\\n> > + HEAD:subdir/foo.c.t HEAD:bar.c.t &&\n> > + git rev-parse >expect \\\n> > + O:a/b/subdir/foo.c.t B:a/b/bar.c.t &&\n> > + test_cmp expect actual &&\n>\n> Likewise?\n>\n> > +\n> > + git hash-object bar.c.t >actual &&\n> > + git rev-parse B:a/b/bar.c.t >expect &&\n> > + test_cmp expect actual &&\n>\n> Likewise?\n>\n> > +\n> > + test_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&\n> > + test_must_fail git rev-parse HEAD:a/b/bar.c.t &&\n> > + test_path_is_missing a/\n>\n> Makes sense, but the part that I am missing is\n>\n> test_path_is_file bar.c.t\n>\n> I.e. the _most_ important outcome of this test is: the rename was\n> detected, and the added file was correctly placed into the target\n> directory of the rename.\n\nThat's a useful thing to add to the test, I'll add it. (It's kind of\nincluded in the 'git hash-object bar.c.t' half a dozen or so lines\nabove, but this line helps document the expectation a bit better.)\n\nI'd be very reticent to include only this test_path_is_file check, as\nit makes no guarantee that it has the right contents or that we didn't\nalso keep around another copy in a/b/bar.c.t, etc. I understand my\nchecks above look overzealous, but merge-recursive has been a bit of a\npain to understand and deal with at times, and I've been bitten one\ntoo many times with tests (of merge-recursive and elsewhere in\ngit.git) that passed but still did the wrong thing because they only\ntested one way in which the test problem failed in the past rather\nthan specifying what correct behavior is. And my \"overzealousness\" in\nthe merge-recursive tests really have caught and prevented bugs would\nhave been missed otherwise.\n\n\nOh, and I think there's another place in the code that needs to be\ntweaked to make sure we handle renaming subdirectories into the root\ndirectory that I missed (and just wasn't tested by this testcase), so\nI'll check into it and if so fix the code and add another testcase,\nand include the fixups I agreed to above and send out a v2. Probably\nwon't get to it until the middle of next week, though.\n\n\nThanks again for the review and food for thought. The CI/PR builds\nsound particularly interesting; I'd really like to make those better\nfor you if it can be done in a way that doesn't break command line\nbuilds with clear documentation of setup and expectations while\nsupporting test skipping and post-facto investigation (even without\n--immediate or re-running), etc. Hmm...\n"},{"id":"384007","messageId":"nycvar.QRO.7.76.6.1910141211130.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"CABPp-BFNCLJnt4NgFKVxURBGD1Z00gastc5q4ZPjcHmwS=kuFw@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-14T10:41:46Z","receivedAt":"2019-10-14T10:42:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Sat, 12 Oct 2019, Elijah Newren wrote:\n\n> On Sat, Oct 12, 2019 at 1:37 PM Johannes Schindelin\n> wrote:\n> >\n> [...]\n> > For the record: I am still a huge anti-fan of splitting `setup` test\n> > cases from the test cases that do actual things, _unless_ it is\n> > _one_, and _only one_, big, honking `setup` test case that is the\n> > very first one in the test script.\n> >\n> > Splitting things into two inevitably leads to unnecessary churn when\n> > investigating test failures.\n> >\n> > And that's really what test cases need to be optimized for: when\n> > they report breakages. They need to help as much as they can to\n> > investigate why things break. Nobody cares when test cases succeed.\n> > The ~150k test cases that pass on every CI build: nobody is\n> > interested. When a test case reports failure, that's when people pay\n> > attention. At least some, including me.\n> >\n> > The most common case for me (and every other lazy person who relies\n> > on CI/PR builds) is when a build breaks, and then I usually get to\n> > the trace of the actually failing test case very quickly. The\n> > previous test case's trace, not so easy. Clicks involved. Now I lose\n> > context. Not good.\n> >\n> > A less common case for me is when I run test scripts locally, with\n> > `-i -v -x`. Still, I need to scroll back to get context. And then,\n> > really, I already lost context.\n>\n> I guess we have some strongly differing opinions here.\n\nAnd probably experiences, too.\n\nI literally debug something like a dozen per week test failures that are\nreported via Azure Pipelines. And yes, I ran into some snags even with\nyour two-parter test cases in the past, and they did not help me. They\nincreased my burden of figuring out what is going wrong.\n\n> The one thing I do agree with you on is test cases need to be\n> optimized for when they report breakages, but that is precisely what\n> led me to splitting setup and testing.\n\nTo me, it is so not helpful _not_ to see the output of a `setup` that\nsucceeded, and only the output of the actual test that actually failed.\n\nIt removes context.\n\nI need to understand the scenario where the breakage happens, and the\nonly way I can understand is when I understand the context.\n\nSo the context needs to be as close as possible.\n\n> Way too many tests in the testsuite intersperse several setup and test\n> cases together making it really hard to disentangle, understand what\n> is going on, or even reverse engineer what is relevant. The absolute\n> worst tests are the ones which just keep making additional changes to\n> some existing repo to provide extra setup, causing all sorts of\n> problems for skipping and resuming and understanding (to say nothing\n> of test prerequisites that aren't always met).\n\nI agree with this sentiment, and have to point out that this is yet\nanother fallout of the way our test suite is implemented. If you look\ne.g. at JUnit, there are no \"setup test cases\". There are specific setup\nsteps that you can define, there is even a teardown step you can define,\nand those individual test cases? They can run in parallel, or\nrandomized, and they run in their own sandbox, to make sure that they\ndon't rely on side effects of unrelated test cases.\n\nWe don't do that. We don't enforce the absence of side effects, and\ntherefore we have a megaton of them.\n\nBut note how this actually speaks _against_ separating the setup from\nthe test? Because then you _explicitly_ want those test cases to rely on\none another. Which flies in the _face_ of trying to disentangling them.\n\n> But the ones with one big honking `setup` test case that is the very\n> first one in the test script can often be pretty bad too when you've\n> found a bug in testcase 82 and want to have a simple way to reproduce.\n\nIndeed. One of the first things I try when `--run=82` fails is\n`--run=1,82`. That's the best we can do in the absence of a clean design\nlike JUnit and its `@Before` method.\n\n> It's awesome when someone can just run ./testcase.sh --ver --imm -x\n> --run=86,87 and reproduce the problem.\n\nBut of course, often you would need `--run=1,86,87`. And it is totally\nnot obvious to _anybody_ who wants to contribute a new feature and whose\nCI/PR build fails in one of your test cases.\n\n> It feels sadly rare to be able to do that in much of git.git's\n> testsuite. Not accreting ever more changes into a repo to setup\n> subsequent problems, using entirely separate repos for different cases\n> where testing makes any changes, making it clear what is setup, making\n> it clear what's the command being tested, and making it clear what all\n> the commands are that are testing that the original test command\n> produced the expected behavior all go a long way to making things way\n> easier to investigate.\n\nSorry, I disagree. By squirreling away your setup phase into what looks\nlike an independent test case, the code is not more obvious, but less\nso.\n\n> Now, I will re-use a setup case for multiple tests and even did so in\n> t6043, but only when the setup really is identical (e.g. not only are\n> the tests expecting an identical setup, but the commands being tested\n> are read-only or any changes they make are unconditionally reverted as\n> the last step of the testcase). Naturally, when I re-use a setup\n> block multiple times, I really don't want to copy the setup into each\n> of those tests either.\n>\n> It would be really nice, though, if there were some way for me to\n> specify that a given \"testcase\" was just setup (so that they don't\n> even get a number in the prove output and don't count as a \"test\"\n> except maybe when they fail), and if there were some way to specify\n> which testcases depend on which setup blocks (so that if I want to run\n> just a given test, the setup test gets automatically included).\n>\n>\n> Your example of CI/PR builds makes sense to me, and I really would\n> like to make that nicer and improve other workflows too, especially if\n> it can be done without breaking my ability to investigate test\n> failures. If someone can think of a clever way to do that (e.g. some\n> way of implementing my \"It would be really nice\" in the last\n> paragraph, and in a way that could also benefit CI/PR builds), I'd be\n> happy to go through and help switch things over.\n\nMy suggestion: do not rip apart commands that belong together. The setup\nphase is often more important to understand than the actually failing\ncommand. _Especially_ when the setup test case reports success, but\nactually failed to set things up as intended (which I encountered more\ntimes than I care to count).\n\n> > > + (\n> > > + cd 12d &&\n> > > +\n> > > + git checkout A^0 &&\n> > > +\n> > > + git -c merge.directoryRenames=true merge -s recursive B^0 &&\n> > > +\n> > > + git ls-files -s >out &&\n> > > + test_line_count = 2 out &&\n> >\n> > Isn't this a bit overzealous?\n> >\n> > > +\n> > > + git rev-parse >actual \\\n> > > + HEAD:subdir/foo.c.t HEAD:bar.c.t &&\n> > > + git rev-parse >expect \\\n> > > + O:a/b/subdir/foo.c.t B:a/b/bar.c.t &&\n> > > + test_cmp expect actual &&\n> >\n> > Likewise?\n> >\n> > > +\n> > > + git hash-object bar.c.t >actual &&\n> > > + git rev-parse B:a/b/bar.c.t >expect &&\n> > > + test_cmp expect actual &&\n> >\n> > Likewise?\n> >\n> > > +\n> > > + test_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&\n> > > + test_must_fail git rev-parse HEAD:a/b/bar.c.t &&\n> > > + test_path_is_missing a/\n> >\n> > Makes sense, but the part that I am missing is\n> >\n> > test_path_is_file bar.c.t\n> >\n> > I.e. the _most_ important outcome of this test is: the rename was\n> > detected, and the added file was correctly placed into the target\n> > directory of the rename.\n>\n> That's a useful thing to add to the test, I'll add it. (It's kind of\n> included in the 'git hash-object bar.c.t' half a dozen or so lines\n> above, but this line helps document the expectation a bit better.)\n>\n> I'd be very reticent to include only this test_path_is_file check, as\n> it makes no guarantee that it has the right contents or that we didn't\n> also keep around another copy in a/b/bar.c.t, etc.r\n\nI agree that it has to strike a balance. There are multiple aspects you\nneed to consider:\n\n- It needs to be easy to understand what the test case tries to ensure.\n\n- While it is important to verify that Git works correctly, you do not\n want to make the test suite so slow as to be useless. I vividly\n remember how Duy mentioned that he does not have the time to run the\n test suite before contributing. That's how bad things are _already_.\n\nAs a rule of thumb, I would like to submit that a test case should fail\nwhen the claimed goal (i.e. the test case's title) is, and a breakage in\nthat should both be sufficient and necessary for it to fail.\n\nIn this instance, the title is:\n\n\t12d-setup: Rename (merge) of subdirectory into the root\n\nNow, this does not even tell me clearly what it tries to ensure. But\nfrom the code, I guess that it wants to ensure that the directory rename\ndetection can detect when a directory's contents were moved into the\ntop-level directory.\n\nNow, let's apply the rule of the \"sufficient\". I would expect the\noutcome to be that the file that was added to that directory in one\nbranch is moved together with the directory's contents that the other\nbranch moved into the top-level directory.\n\nI was missing that test, hence I pointed it out.\n\nBut testing the absence of the file `foo.c.t` in its original location\nwhich was moved in one branch, and not touched in the other, i.e. with a\n_trivial_ three-way merge resolution, that seems to be extra. It would\nnot break if the directory rename detection is broken, so it is an extra\ntest that does not do what the test case's title claims to test.\n\nNow for the \"necessary\" part. It is totally unclear to me how all those\nother things you test for would be failing _only_ if the directory\nrename detection failed. I would actually expect that there are _many_\nways that this test case could fail in ways that have not the faintest\nthing to do with directory rename detection.\n\nAs somebody who, as I stated above, investigates dozens of test case\nfailures per week (although, granted, most of them are relatively\ntrivial to resolve, it is invariably test cases like the one I am\ncommenting on that require the most time to get resolved), I have to\nadmit that there are few more frustrating things in debugging than\nhaving to chase issues that have _nothing_ to do with the stated goal of\nthe test case.\n\n> I understand my checks above look overzealous, but merge-recursive has\n> been a bit of a pain to understand and deal with at times, and I've\n> been bitten one too many times with tests (of merge-recursive and\n> elsewhere in git.git) that passed but still did the wrong thing\n> because they only tested one way in which the test problem failed in\n> the past rather than specifying what correct behavior is. And my\n> \"overzealousness\" in the merge-recursive tests really have caught and\n> prevented bugs would have been missed otherwise.\n\nIf you must have those tests, then please structure them in a way where\nthey are much more actionable. Ideally, it should be clear from a glance\nat the trace of a failed test case (and _only_ from that test case) for\nless than five seconds _what_ is being tested, and it should be obvious\nwhat is going wrong.\n\nI cannot say that about the test case in this mail.\n\n> Oh, and I think there's another place in the code that needs to be\n> tweaked to make sure we handle renaming subdirectories into the root\n> directory that I missed (and just wasn't tested by this testcase), so\n> I'll check into it and if so fix the code and add another testcase,\n> and include the fixups I agreed to above and send out a v2. Probably\n> won't get to it until the middle of next week, though.\n>\n>\n> Thanks again for the review and food for thought. The CI/PR builds\n> sound particularly interesting; I'd really like to make those better\n> for you if it can be done in a way that doesn't break command line\n> builds with clear documentation of setup and expectations while\n> supporting test skipping and post-facto investigation (even without\n> --immediate or re-running), etc. Hmm...\n\nMy hope was that CI/PR builds could help improve the code quality in\n`pu`, and to a large part it did.\n\nThe time I need to spend on understanding test cases' code (t0021 is a\nparticularly nasty one, not your fault, of course) before I can even get\nto fire up a debugger, this amount of time is insane, though.\n\nCiao,\nDscho\n"},{"id":"384619","messageId":"CABPp-BFYWhyLqUktEnk6A7v6135k4TQHO20Wiy32mRQekt-3cg@mail.gmail.com","threadId":"52025","inReplyTo":"nycvar.QRO.7.76.6.1910141211130.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-10-22T19:15:41Z","receivedAt":"2019-10-22T19:15:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Sorry for the long delay before getting back to this; the other stuff\nI was working on took longer than expected.\n\nOn Mon, Oct 14, 2019 at 3:42 AM Johannes Schindelin\n wrote:\n> On Sat, 12 Oct 2019, Elijah Newren wrote:\n> > On Sat, Oct 12, 2019 at 1:37 PM Johannes Schindelin\n> > wrote:\n> > >\n> > > For the record: I am still a huge anti-fan of splitting `setup` test\n> > > cases from the test cases that do actual things, _unless_ it is\n> > > _one_, and _only one_, big, honking `setup` test case that is the\n> > > very first one in the test script.\n[...]\n> > The one thing I do agree with you on is test cases need to be\n> > optimized for when they report breakages, but that is precisely what\n> > led me to splitting setup and testing.\n>\n> To me, it is so not helpful _not_ to see the output of a `setup` that\n> succeeded, and only the output of the actual test that actually failed.\n>\n> It removes context.\n>\n> I need to understand the scenario where the breakage happens, and the\n> only way I can understand is when I understand the context.\n>\n> So the context needs to be as close as possible.\n\nI've updated the patch series with a change that I hope helps while\nstill allowing the setup \"steps\" to be visibly differentiated from the\ntesting steps.\n\n> > Way too many tests in the testsuite intersperse several setup and test\n> > cases together making it really hard to disentangle, understand what\n> > is going on, or even reverse engineer what is relevant. The absolute\n> > worst tests are the ones which just keep making additional changes to\n> > some existing repo to provide extra setup, causing all sorts of\n> > problems for skipping and resuming and understanding (to say nothing\n> > of test prerequisites that aren't always met).\n>\n> I agree with this sentiment, and have to point out that this is yet\n> another fallout of the way our test suite is implemented. If you look\n> e.g. at JUnit, there are no \"setup test cases\". There are specific setup\n> steps that you can define, there is even a teardown step you can define,\n> and those individual test cases? They can run in parallel, or\n> randomized, and they run in their own sandbox, to make sure that they\n> don't rely on side effects of unrelated test cases.\n>\n> We don't do that. We don't enforce the absence of side effects, and\n> therefore we have a megaton of them.\n>\n> But note how this actually speaks _against_ separating the setup from\n> the test? Because then you _explicitly_ want those test cases to rely on\n> one another. Which flies in the _face_ of trying to disentangling them.\n\nI agree that it is desirable to avoid side effects in the tests, but\nI'd like to point out that I'm not at all sure that your conclusion\nhere is the only logical one to draw here in comparing to JUnit. As\nyou point out, JUnit has clearly delineated setup steps for a test (as\nwell as teardown steps), providing a place to keep them separate. Our\ntestsuite lacks that, so how do folks try to get it? One logical way\nwould be just inlining the setup steps in the test outside a\ntest_expect_* block (which has been done in the past), but that has\neven worse problems. Another way, even if suboptimal, is placing\nthose steps in their own test_expect_* block. You say just throw the\nsetup and test together, but that breaks the separation.\n\nI think it's a case of the testsuite not providing the right\nabstractions and enough capability, leaving us to argue over which\naspects of a more featureful test harness are most important to\nemulate. You clearly picked one, while I was focusing on another.\nAnyway, all that said, I think I have a nice compromise that I'll send\nout with V2.\n\n[...]\n> > > Makes sense, but the part that I am missing is\n> > >\n> > > test_path_is_file bar.c.t\n> > >\n> > > I.e. the _most_ important outcome of this test is: the rename was\n> > > detected, and the added file was correctly placed into the target\n> > > directory of the rename.\n> >\n> > That's a useful thing to add to the test, I'll add it. (It's kind of\n> > included in the 'git hash-object bar.c.t' half a dozen or so lines\n> > above, but this line helps document the expectation a bit better.)\n> >\n> > I'd be very reticent to include only this test_path_is_file check, as\n> > it makes no guarantee that it has the right contents or that we didn't\n> > also keep around another copy in a/b/bar.c.t, etc.\n>\n> I agree that it has to strike a balance. There are multiple aspects you\n> need to consider:\n>\n> - It needs to be easy to understand what the test case tries to ensure.\n>\n> - While it is important to verify that Git works correctly, you do not\n> want to make the test suite so slow as to be useless. I vividly\n> remember how Duy mentioned that he does not have the time to run the\n> test suite before contributing. That's how bad things are _already_.\n\nOn this issue I'm having a harder time finding common ground. Perhaps\nthere's something clever to be done, but I'm not seeing it. I can at\nleast try to explain my perspective...\n\nI understand you probably meant these two items as a non-exhaustive\nlist of things that need to be balanced, but you did omit what I feel\nis the most important thing in the balance:\n\n- The combination of tests should provide adequate confidence of\ncoverage that someone can refactor the code as necessary.\n\nThe tests in t6036, t6042, t6043, and t6046 are more defensive than\nnormal (defining \"correct\" behavior more finely-grained than usual),\nbut that came about precisely because it was so easy to make\nrefactoring mistakes that broke things in unexpected ways. There have\nbeen a handful of times already when I have gone in to refactor or fix\na reported bug, thought I had the right solution, and then only one or\nmaybe two tests failed and it was one of these tests and in particular\nthe \"overzealous\" portions of these tests that caught the bug (sorry I\ndon't still have details, I just remember being surprised and happy\nabout the extra defensiveness on multiple occasions). Without this\nextra defensiveness, there would have been extra bugs introduced that\nI'm sure would have made it into real git releases. So, there would\nneed to be a large pile of evidence and problems before I'd be willing\nto take these out.\n\nAll that said...\n\nI do very much agree with you that the overall testsuite needs to be\nrelatively speedy too, and that is an important counterbalance in\ngeneral. But testsuite performance is somewhat of a red-herring in\nthis context: when running all testcases with 'merge' in their name\nwhich I have contributed anything at all to (apparently 17 t[0-9]*.sh\nfiles meet both these criteria), and running them in serial (no -j8 or\nanything of the sort), they all combined complete in less than 40% of\nthe time that t9001-send-email.sh alone takes on my box. It'd reduce\nto barely over a quarter of the time that the send-email test takes if\nit weren't for t3425-rebase-topology-merges.sh (which is the slowest\ntest with 'merge' in its name that I've touched, despite the fact that\nit has none of this extra defensiveness you are complaining about and\nlabelling as unrelated). Yes, t6043 is pretty long code-wise, and\nstill has over five dozen tests after ditching the separate \"setup\"\ntests -- but all of those tests still run in 3.6s on my box. I think\nthat a few extra calls to ls-files, rev-parse, hash-object and\ntest_path_is_missing are the wrong place to look if you're worried\nabout testsuite performance.\n\nFinally, you also brought up \"easy to understand\" the test case. You\nare absolutely right that the extra calls to ls-files, rev-parse,\nhash-object, test_path_is_missing do slow down an external reader and\nalmost certainly doesn't fit in your \"within five seconds\" goal you\nmentioned below. That's unfortunate. But optimizing this at the\nexpense of preventing actual bugs that would have hit git.git releases\nis a bad tradeoff and one I'm not willing to make. If you want me to\nrestrict the extra defensiveness to just the merge-recursive tests,\nI'm happy to do so. I do agree that it's unusual, and probably should\nbe, but I think merge-recursive merits the special handling.\n\n\nHope that helps; thanks for all the feedback.\n\nElijah\n"},{"id":"384632","messageId":"8ae78679c9066a206610465e1da91a5f575fced2.1571779371.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.v2.git.1571779371.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] merge-recursive: clean up get_renamed_dir_portion()","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-22T21:22:49Z","receivedAt":"2019-10-22T21:22:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nDscho noted a few things making this function hard to follow.\nRestructure it a bit and add comments to make it easier to follow. The\nrestructurings include:\n\n * There was a special case if-check at the end of the function\n checking whether someone just renamed a file within its original\n directory, meaning that there could be no directory rename involved.\n That check was slightly convoluted; it could be done in a more\n straightforward fashion earlier in the function, and can be done\n more cheaply too (no call to strncmp).\n\n * The conditions for advancing end_of_old and end_of_new before\n calling strchr were both confusing and unnecessary. If either\n points at a '/', then they need to be advanced in order to find the\n next '/'. If either doesn't point at a '/', then advancing them one\n char before calling strchr() doesn't hurt. So, just rip out the\n if conditions and advance both before calling strchr().\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 60 ++++++++++++++++++++++++++++-------------------\n 1 file changed, 36 insertions(+), 24 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 22a12cfeba..f80e48f623 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1943,8 +1943,8 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t\t\t\t char **old_dir, char **new_dir)\n {\n \tchar *end_of_old, *end_of_new;\n-\tint old_len, new_len;\n \n+\t/* Default return values: NULL, meaning no rename */\n \t*old_dir = NULL;\n \t*new_dir = NULL;\n \n@@ -1955,43 +1955,55 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t * \"a/b/c/d\" was renamed to \"a/b/some/thing/else\"\n \t * so, for this example, this function returns \"a/b/c/d\" in\n \t * *old_dir and \"a/b/some/thing/else\" in *new_dir.\n-\t *\n-\t * Also, if the basename of the file changed, we don't care. We\n-\t * want to know which portion of the directory, if any, changed.\n+\t */\n+\n+\t/*\n+\t * If the basename of the file changed, we don't care. We want\n+\t * to know which portion of the directory, if any, changed.\n \t */\n \tend_of_old = strrchr(old_path, '/');\n \tend_of_new = strrchr(new_path, '/');\n-\n \tif (end_of_old == NULL || end_of_new == NULL)\n-\t\treturn;\n+\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n+\n+\t/* Find the first non-matching character traversing backwards */\n \twhile (*--end_of_new == *--end_of_old &&\n \t end_of_old != old_path &&\n \t end_of_new != new_path)\n \t\t; /* Do nothing; all in the while loop */\n+\n \t/*\n-\t * We've found the first non-matching character in the directory\n-\t * paths. That means the current directory we were comparing\n-\t * represents the rename. Move end_of_old and end_of_new back\n-\t * to the full directory name.\n+\t * If both got back to the beginning of their strings, then the\n+\t * directory didn't change at all, only the basename did.\n \t */\n-\tif (*end_of_old == '/')\n-\t\tend_of_old++;\n-\tif (*end_of_old != '/')\n-\t\tend_of_new++;\n-\tend_of_old = strchr(end_of_old, '/');\n-\tend_of_new = strchr(end_of_new, '/');\n+\tif (end_of_old == old_path && end_of_new == new_path &&\n+\t *end_of_old == *end_of_new)\n+\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n \n \t/*\n-\t * It may have been the case that old_path and new_path were the same\n-\t * directory all along. Don't claim a rename if they're the same.\n+\t * We've found the first non-matching character in the directory\n+\t * paths. That means the current characters we were looking at\n+\t * were part of the first non-matching subdir name going back from\n+\t * the end of the strings. Get the whole name by advancing both\n+\t * end_of_old and end_of_new to the NEXT '/' character. That will\n+\t * represent the entire directory rename.\n+\t *\n+\t * The reason for the increment is cases like\n+\t * a/b/star/foo/whatever.c -> a/b/tar/foo/random.c\n+\t * After dropping the basename and going back to the first\n+\t * non-matching character, we're now comparing:\n+\t * a/b/s and a/b/\n+\t * and we want to be comparing:\n+\t * a/b/star/ and a/b/tar/\n+\t * but without the pre-increment, the one on the right would stay\n+\t * a/b/.\n \t */\n-\told_len = end_of_old - old_path;\n-\tnew_len = end_of_new - new_path;\n+\tend_of_old = strchr(++end_of_old, '/');\n+\tend_of_new = strchr(++end_of_new, '/');\n \n-\tif (old_len != new_len || strncmp(old_path, new_path, old_len)) {\n-\t\t*old_dir = xstrndup(old_path, old_len);\n-\t\t*new_dir = xstrndup(new_path, new_len);\n-\t}\n+\t/* Copy the old and new directories into *old_dir and *new_dir. */\n+\t*old_dir = xstrndup(old_path, end_of_old - old_path);\n+\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n }\n \n static void remove_hashmap_entries(struct hashmap *dir_renames,\n-- \ngitgitgadget\n\n"},{"id":"384633","messageId":"pull.390.v2.git.1571779371.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Dir rename fixes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-22T21:22:48Z","receivedAt":"2019-10-22T21:22:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series improves a couple things found after looking into things Dscho\nflagged:\n\n * clarify and slightly restructure code in the get_renamed_dir_portion()\n function\n * extend support of detecting renaming/merging of one directory into\n another to support the root directory as a target directory\n\nFirst patch best viewed with a --histogram diff (sorry, gitgitgadget does\nnot yet know how to generate those).\n\nChanges since v1:\n\n * Incorporated code cleanups suggested by Dscho\n * Fixed to work with an alternate rename-to-root-directory case (end_of_new\n == NULL), with new testcase\n * Added a new patch to the end of the series to stop making setup tests be\n part of a separate test_expect_success block.\n\nElijah Newren (3):\n merge-recursive: clean up get_renamed_dir_portion()\n merge-recursive: fix merging a subdirectory into the root directory\n t604[236]: do not run setup in separate tests\n\n merge-recursive.c | 104 ++++-\n t/t6042-merge-rename-corner-cases.sh | 111 +++--\n t/t6043-merge-rename-directories.sh | 568 ++++++++++++++++---------\n t/t6046-merge-skip-unneeded-updates.sh | 135 +++---\n 4 files changed, 582 insertions(+), 336 deletions(-)\n\n\nbase-commit: 08da6496b61341ec45eac36afcc8f94242763468\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-390%2Fnewren%2Fdir-rename-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-390/newren/dir-rename-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/390\n\nRange-diff vs v1:\n\n 1: 8ae78679c9 = 1: 8ae78679c9 merge-recursive: clean up get_renamed_dir_portion()\n 2: 37aee862e1 ! 2: a1e80e8fbb merge-recursive: fix merging a subdirectory into the root directory\n @@ -34,9 +34,41 @@\n \tstrbuf_grow(&new_path, newlen);\n \tstrbuf_addbuf(&new_path, &entry->new_dir);\n @@\n - \t *end_of_old == *end_of_new)\n - \t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n + \t */\n + \tend_of_old = strrchr(old_path, '/');\n + \tend_of_new = strrchr(new_path, '/');\n +-\tif (end_of_old == NULL || end_of_new == NULL)\n +-\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n ++\n ++\t/*\n ++\t * If end_of_old is NULL, old_path wasn't in a directory, so there\n ++\t * could not be a directory rename (our rule elsewhere that a\n ++\t * directory which still exists is not considered to have been\n ++\t * renamed means the root directory can never be renamed -- because\n ++\t * the root directory always exists).\n ++\t */\n ++\tif (end_of_old == NULL)\n ++\t\treturn; /* Note: *old_dir and *new_dir are still NULL */\n ++\n ++\t/*\n ++\t * If new_path contains no directory (end_of_new is NULL), then we\n ++\t * have a rename of old_path's directory to the root directory.\n ++\t */\n ++\tif (end_of_new == NULL) {\n ++\t\t*old_dir = xstrndup(old_path, end_of_old - old_path);\n ++\t\t*new_dir = xstrdup(\"\");\n ++\t\treturn;\n ++\t}\n \n + \t/* Find the first non-matching character traversing backwards */\n + \twhile (*--end_of_new == *--end_of_old &&\n +@@\n + \t */\n + \tif (end_of_old == old_path && end_of_new == new_path &&\n + \t *end_of_old == *end_of_new)\n +-\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n ++\t\treturn; /* Note: *old_dir and *new_dir are still NULL */\n ++\n +\t/*\n +\t * If end_of_new got back to the beginning of its string, and\n +\t * end_of_old got back to the beginning of some subdirectory, then\n @@ -44,21 +76,19 @@\n +\t * needs slightly special handling.\n +\t *\n +\t * Note: There is no need to consider the opposite case, with a\n -+\t * rename/merge of the root directory into some subdirectory.\n -+\t * Our rule elsewhere that a directory which still exists is not\n -+\t * considered to have been renamed means the root directory can\n -+\t * never be renamed (because the root directory always exists).\n ++\t * rename/merge of the root directory into some subdirectory\n ++\t * because as noted above the root directory always exists so it\n ++\t * cannot be considered to be renamed.\n +\t */\n +\tif (end_of_new == new_path &&\n +\t end_of_old != old_path && end_of_old[-1] == '/') {\n -+\t\t*old_dir = xstrndup(old_path, end_of_old-1 - old_path);\n -+\t\t*new_dir = xstrndup(new_path, end_of_new - new_path);\n ++\t\t*old_dir = xstrndup(old_path, --end_of_old - old_path);\n ++\t\t*new_dir = xstrdup(\"\");\n +\t\treturn;\n +\t}\n -+\n + \n \t/*\n \t * We've found the first non-matching character in the directory\n - \t * paths. That means the current characters we were looking at\n \n diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\n --- a/t/t6043-merge-rename-directories.sh\n @@ -68,18 +98,18 @@\n '\n \n +# Testcase 12d, Rename/merge of subdirectory into the root\n -+# Commit O: a/b/{foo.c}\n -+# Commit A: foo.c\n -+# Commit B: a/b/{foo.c,bar.c}\n -+# Expected: a/b/{foo.c,bar.c}\n ++# Commit O: a/b/subdir/foo\n ++# Commit A: subdir/foo\n ++# Commit B: a/b/subdir/foo, a/b/bar\n ++# Expected: subdir/foo, bar\n +\n -+test_expect_success '12d-setup: Rename (merge) of subdirectory into the root' '\n ++test_expect_success '12d-setup: Rename/merge subdir into the root, variant 1' '\n +\ttest_create_repo 12d &&\n +\t(\n +\t\tcd 12d &&\n +\n +\t\tmkdir -p a/b/subdir &&\n -+\t\ttest_commit a/b/subdir/foo.c &&\n ++\t\ttest_commit a/b/subdir/foo &&\n +\n +\t\tgit branch O &&\n +\t\tgit branch A &&\n @@ -87,16 +117,16 @@\n +\n +\t\tgit checkout A &&\n +\t\tmkdir subdir &&\n -+\t\tgit mv a/b/subdir/foo.c.t subdir/foo.c.t &&\n ++\t\tgit mv a/b/subdir/foo.t subdir/foo.t &&\n +\t\ttest_tick &&\n +\t\tgit commit -m \"A\" &&\n +\n +\t\tgit checkout B &&\n -+\t\ttest_commit a/b/bar.c\n ++\t\ttest_commit a/b/bar\n +\t)\n +'\n +\n -+test_expect_success '12d-check: Rename (merge) of subdirectory into the root' '\n ++test_expect_success '12d-check: Rename/merge subdir into the root, variant 1' '\n +\t(\n +\t\tcd 12d &&\n +\n @@ -108,18 +138,76 @@\n +\t\ttest_line_count = 2 out &&\n +\n +\t\tgit rev-parse >actual \\\n -+\t\t\tHEAD:subdir/foo.c.t HEAD:bar.c.t &&\n ++\t\t\tHEAD:subdir/foo.t HEAD:bar.t &&\n ++\t\tgit rev-parse >expect \\\n ++\t\t\tO:a/b/subdir/foo.t B:a/b/bar.t &&\n ++\t\ttest_cmp expect actual &&\n ++\n ++\t\tgit hash-object bar.t >actual &&\n ++\t\tgit rev-parse B:a/b/bar.t >expect &&\n ++\t\ttest_cmp expect actual &&\n ++\n ++\t\ttest_must_fail git rev-parse HEAD:a/b/subdir/foo.t &&\n ++\t\ttest_must_fail git rev-parse HEAD:a/b/bar.t &&\n ++\t\ttest_path_is_missing a/ &&\n ++\t\ttest_path_is_file bar.t\n ++\t)\n ++'\n ++\n ++# Testcase 12e, Rename/merge of subdirectory into the root\n ++# Commit O: a/b/foo\n ++# Commit A: foo\n ++# Commit B: a/b/foo, a/b/bar\n ++# Expected: foo, bar\n ++\n ++test_expect_success '12e-setup: Rename/merge subdir into the root, variant 2' '\n ++\ttest_create_repo 12e &&\n ++\t(\n ++\t\tcd 12e &&\n ++\n ++\t\tmkdir -p a/b &&\n ++\t\ttest_commit a/b/foo &&\n ++\n ++\t\tgit branch O &&\n ++\t\tgit branch A &&\n ++\t\tgit branch B &&\n ++\n ++\t\tgit checkout A &&\n ++\t\tmkdir subdir &&\n ++\t\tgit mv a/b/foo.t foo.t &&\n ++\t\ttest_tick &&\n ++\t\tgit commit -m \"A\" &&\n ++\n ++\t\tgit checkout B &&\n ++\t\ttest_commit a/b/bar\n ++\t)\n ++'\n ++\n ++test_expect_success '12e-check: Rename/merge subdir into the root, variant 2' '\n ++\t(\n ++\t\tcd 12e &&\n ++\n ++\t\tgit checkout A^0 &&\n ++\n ++\t\tgit -c merge.directoryRenames=true merge -s recursive B^0 &&\n ++\n ++\t\tgit ls-files -s >out &&\n ++\t\ttest_line_count = 2 out &&\n ++\n ++\t\tgit rev-parse >actual \\\n ++\t\t\tHEAD:foo.t HEAD:bar.t &&\n +\t\tgit rev-parse >expect \\\n -+\t\t\tO:a/b/subdir/foo.c.t B:a/b/bar.c.t &&\n ++\t\t\tO:a/b/foo.t B:a/b/bar.t &&\n +\t\ttest_cmp expect actual &&\n +\n -+\t\tgit hash-object bar.c.t >actual &&\n -+\t\tgit rev-parse B:a/b/bar.c.t >expect &&\n ++\t\tgit hash-object bar.t >actual &&\n ++\t\tgit rev-parse B:a/b/bar.t >expect &&\n +\t\ttest_cmp expect actual &&\n +\n -+\t\ttest_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&\n -+\t\ttest_must_fail git rev-parse HEAD:a/b/bar.c.t &&\n -+\t\ttest_path_is_missing a/\n ++\t\ttest_must_fail git rev-parse HEAD:a/b/foo.t &&\n ++\t\ttest_must_fail git rev-parse HEAD:a/b/bar.t &&\n ++\t\ttest_path_is_missing a/ &&\n ++\t\ttest_path_is_file bar.t\n +\t)\n +'\n +\n -: ---------- > 3: 0dd61a3580 t604[236]: do not run setup in separate tests\n\n-- \ngitgitgadget\n"},{"id":"384634","messageId":"a1e80e8fbbf4b62c83a4a082c1be03f7ab92541c.1571779371.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.v2.git.1571779371.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-22T21:22:50Z","receivedAt":"2019-10-22T21:22:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWe allow renaming all entries in e.g. a directory named z/ into a\ndirectory named y/ to be detected as a z/ -> y/ rename, so that if the\nother side of history adds any files to the directory z/ in the mean\ntime, we can provide the hint that they should be moved to y/.\n\nThere is no reason to not allow 'y/' to be the root directory, but the\ncode did not handle that case correctly. Add a testcase and the\nnecessary special checks to support this case.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 52 ++++++++++++-\n t/t6043-merge-rename-directories.sh | 114 ++++++++++++++++++++++++++++\n 2 files changed, 163 insertions(+), 3 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex f80e48f623..ec60715368 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1931,6 +1931,16 @@ static char *apply_dir_rename(struct dir_rename_entry *entry,\n \t\treturn NULL;\n \n \toldlen = strlen(entry->dir);\n+\tif (entry->new_dir.len == 0)\n+\t\t/*\n+\t\t * If someone renamed/merged a subdirectory into the root\n+\t\t * directory (e.g. 'some/subdir' -> ''), then we want to\n+\t\t * avoid returning\n+\t\t * '' + '/filename'\n+\t\t * as the rename; we need to make old_path + oldlen advance\n+\t\t * past the '/' character.\n+\t\t */\n+\t\toldlen++;\n \tnewlen = entry->new_dir.len + (strlen(old_path) - oldlen) + 1;\n \tstrbuf_grow(&new_path, newlen);\n \tstrbuf_addbuf(&new_path, &entry->new_dir);\n@@ -1963,8 +1973,26 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t */\n \tend_of_old = strrchr(old_path, '/');\n \tend_of_new = strrchr(new_path, '/');\n-\tif (end_of_old == NULL || end_of_new == NULL)\n-\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n+\n+\t/*\n+\t * If end_of_old is NULL, old_path wasn't in a directory, so there\n+\t * could not be a directory rename (our rule elsewhere that a\n+\t * directory which still exists is not considered to have been\n+\t * renamed means the root directory can never be renamed -- because\n+\t * the root directory always exists).\n+\t */\n+\tif (end_of_old == NULL)\n+\t\treturn; /* Note: *old_dir and *new_dir are still NULL */\n+\n+\t/*\n+\t * If new_path contains no directory (end_of_new is NULL), then we\n+\t * have a rename of old_path's directory to the root directory.\n+\t */\n+\tif (end_of_new == NULL) {\n+\t\t*old_dir = xstrndup(old_path, end_of_old - old_path);\n+\t\t*new_dir = xstrdup(\"\");\n+\t\treturn;\n+\t}\n \n \t/* Find the first non-matching character traversing backwards */\n \twhile (*--end_of_new == *--end_of_old &&\n@@ -1978,7 +2006,25 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,\n \t */\n \tif (end_of_old == old_path && end_of_new == new_path &&\n \t *end_of_old == *end_of_new)\n-\t\treturn; /* We haven't modified *old_dir or *new_dir yet. */\n+\t\treturn; /* Note: *old_dir and *new_dir are still NULL */\n+\n+\t/*\n+\t * If end_of_new got back to the beginning of its string, and\n+\t * end_of_old got back to the beginning of some subdirectory, then\n+\t * we have a rename/merge of a subdirectory into the root, which\n+\t * needs slightly special handling.\n+\t *\n+\t * Note: There is no need to consider the opposite case, with a\n+\t * rename/merge of the root directory into some subdirectory\n+\t * because as noted above the root directory always exists so it\n+\t * cannot be considered to be renamed.\n+\t */\n+\tif (end_of_new == new_path &&\n+\t end_of_old != old_path && end_of_old[-1] == '/') {\n+\t\t*old_dir = xstrndup(old_path, --end_of_old - old_path);\n+\t\t*new_dir = xstrdup(\"\");\n+\t\treturn;\n+\t}\n \n \t/*\n \t * We've found the first non-matching character in the directory\ndiff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\nindex c966147d5d..32cdd1f493 100755\n--- a/t/t6043-merge-rename-directories.sh\n+++ b/t/t6043-merge-rename-directories.sh\n@@ -4051,6 +4051,120 @@ test_expect_success '12c-check: Moving one directory hierarchy into another w/ c\n \t)\n '\n \n+# Testcase 12d, Rename/merge of subdirectory into the root\n+# Commit O: a/b/subdir/foo\n+# Commit A: subdir/foo\n+# Commit B: a/b/subdir/foo, a/b/bar\n+# Expected: subdir/foo, bar\n+\n+test_expect_success '12d-setup: Rename/merge subdir into the root, variant 1' '\n+\ttest_create_repo 12d &&\n+\t(\n+\t\tcd 12d &&\n+\n+\t\tmkdir -p a/b/subdir &&\n+\t\ttest_commit a/b/subdir/foo &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\t\tgit branch B &&\n+\n+\t\tgit checkout A &&\n+\t\tmkdir subdir &&\n+\t\tgit mv a/b/subdir/foo.t subdir/foo.t &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"A\" &&\n+\n+\t\tgit checkout B &&\n+\t\ttest_commit a/b/bar\n+\t)\n+'\n+\n+test_expect_success '12d-check: Rename/merge subdir into the root, variant 1' '\n+\t(\n+\t\tcd 12d &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\tgit -c merge.directoryRenames=true merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -s >out &&\n+\t\ttest_line_count = 2 out &&\n+\n+\t\tgit rev-parse >actual \\\n+\t\t\tHEAD:subdir/foo.t HEAD:bar.t &&\n+\t\tgit rev-parse >expect \\\n+\t\t\tO:a/b/subdir/foo.t B:a/b/bar.t &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tgit hash-object bar.t >actual &&\n+\t\tgit rev-parse B:a/b/bar.t >expect &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\ttest_must_fail git rev-parse HEAD:a/b/subdir/foo.t &&\n+\t\ttest_must_fail git rev-parse HEAD:a/b/bar.t &&\n+\t\ttest_path_is_missing a/ &&\n+\t\ttest_path_is_file bar.t\n+\t)\n+'\n+\n+# Testcase 12e, Rename/merge of subdirectory into the root\n+# Commit O: a/b/foo\n+# Commit A: foo\n+# Commit B: a/b/foo, a/b/bar\n+# Expected: foo, bar\n+\n+test_expect_success '12e-setup: Rename/merge subdir into the root, variant 2' '\n+\ttest_create_repo 12e &&\n+\t(\n+\t\tcd 12e &&\n+\n+\t\tmkdir -p a/b &&\n+\t\ttest_commit a/b/foo &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\t\tgit branch B &&\n+\n+\t\tgit checkout A &&\n+\t\tmkdir subdir &&\n+\t\tgit mv a/b/foo.t foo.t &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"A\" &&\n+\n+\t\tgit checkout B &&\n+\t\ttest_commit a/b/bar\n+\t)\n+'\n+\n+test_expect_success '12e-check: Rename/merge subdir into the root, variant 2' '\n+\t(\n+\t\tcd 12e &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\tgit -c merge.directoryRenames=true merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -s >out &&\n+\t\ttest_line_count = 2 out &&\n+\n+\t\tgit rev-parse >actual \\\n+\t\t\tHEAD:foo.t HEAD:bar.t &&\n+\t\tgit rev-parse >expect \\\n+\t\t\tO:a/b/foo.t B:a/b/bar.t &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tgit hash-object bar.t >actual &&\n+\t\tgit rev-parse B:a/b/bar.t >expect &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\ttest_must_fail git rev-parse HEAD:a/b/foo.t &&\n+\t\ttest_must_fail git rev-parse HEAD:a/b/bar.t &&\n+\t\ttest_path_is_missing a/ &&\n+\t\ttest_path_is_file bar.t\n+\t)\n+'\n+\n ###########################################################################\n # SECTION 13: Checking informational and conflict messages\n #\n-- \ngitgitgadget\n\n"},{"id":"384635","messageId":"0dd61a35806628a65c46f61f77fce2d4b60438bf.1571779371.git.gitgitgadget@gmail.com","threadId":"52025","inReplyTo":"pull.390.v2.git.1571779371.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] t604[236]: do not run setup in separate tests","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-22T21:22:51Z","receivedAt":"2019-10-22T21:23:02Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nTransform the setup \"tests\" to setup functions, and have the actual\ntests call the setup functions. Advantages:\n\n * Should make life easier for people working with webby CI/PR builds\n who have to abuse mice (and their own index finger as well) in\n order to switch from viewing one testcase to another. Sounds\n awful; hopefully this will improve things for them.\n\n * Improves re-runnability: any failed test in any of these three\n files can now be re-run in isolation, e.g.\n ./t6042* --ver --imm -x --run=21\n whereas before it would require two tests to be specified to the\n --run argument, the other needing to be picked out as the relevant\n setup test from one or two tests before.\n\n * Importantly, this still keeps the \"setup\" and \"test\" sections\n somewhat separate to make it easier for readers to discern what is\n just ancillary setup and what the intent of the test is.\n\nSigned-off-by: Elijah Newren \n---\n t/t6042-merge-rename-corner-cases.sh | 111 +++---\n t/t6043-merge-rename-directories.sh | 466 ++++++++++++++-----------\n t/t6046-merge-skip-unneeded-updates.sh | 135 ++++---\n 3 files changed, 393 insertions(+), 319 deletions(-)\n\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex c5b57f40c3..b047cf1c1c 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -5,7 +5,7 @@ test_description=\"recursive merge corner cases w/ renames but not criss-crosses\"\n \n . ./test-lib.sh\n \n-test_expect_success 'setup rename/delete + untracked file' '\n+test_setup_rename_delete_untracked () {\n \ttest_create_repo rename-delete-untracked &&\n \t(\n \t\tcd rename-delete-untracked &&\n@@ -29,9 +29,10 @@ test_expect_success 'setup rename/delete + untracked file' '\n \t\tgit commit -m track-people-instead-of-objects &&\n \t\techo \"Myyy PRECIOUSSS\" >ring\n \t)\n-'\n+}\n \n test_expect_success \"Does git preserve Gollum's precious artifact?\" '\n+\ttest_setup_rename_delete_untracked &&\n \t(\n \t\tcd rename-delete-untracked &&\n \n@@ -49,7 +50,7 @@ test_expect_success \"Does git preserve Gollum's precious artifact?\" '\n #\n # We should be able to merge B & C cleanly\n \n-test_expect_success 'setup rename/modify/add-source conflict' '\n+test_setup_rename_modify_add_source () {\n \ttest_create_repo rename-modify-add-source &&\n \t(\n \t\tcd rename-modify-add-source &&\n@@ -70,9 +71,10 @@ test_expect_success 'setup rename/modify/add-source conflict' '\n \t\tgit add a &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_failure 'rename/modify/add-source conflict resolvable' '\n+\ttest_setup_rename_modify_add_source &&\n \t(\n \t\tcd rename-modify-add-source &&\n \n@@ -88,7 +90,7 @@ test_expect_failure 'rename/modify/add-source conflict resolvable' '\n \t)\n '\n \n-test_expect_success 'setup resolvable conflict missed if rename missed' '\n+test_setup_break_detection_1 () {\n \ttest_create_repo break-detection-1 &&\n \t(\n \t\tcd break-detection-1 &&\n@@ -110,9 +112,10 @@ test_expect_success 'setup resolvable conflict missed if rename missed' '\n \t\tgit add a &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_failure 'conflict caused if rename not detected' '\n+\ttest_setup_break_detection_1 &&\n \t(\n \t\tcd break-detection-1 &&\n \n@@ -135,7 +138,7 @@ test_expect_failure 'conflict caused if rename not detected' '\n \t)\n '\n \n-test_expect_success 'setup conflict resolved wrong if rename missed' '\n+test_setup_break_detection_2 () {\n \ttest_create_repo break-detection-2 &&\n \t(\n \t\tcd break-detection-2 &&\n@@ -160,9 +163,10 @@ test_expect_success 'setup conflict resolved wrong if rename missed' '\n \t\tgit add a &&\n \t\tgit commit -m E\n \t)\n-'\n+}\n \n test_expect_failure 'missed conflict if rename not detected' '\n+\ttest_setup_break_detection_2 &&\n \t(\n \t\tcd break-detection-2 &&\n \n@@ -182,7 +186,7 @@ test_expect_failure 'missed conflict if rename not detected' '\n # Commit B: rename a->b\n # Commit C: rename a->b, add unrelated a\n \n-test_expect_success 'setup undetected rename/add-source causes data loss' '\n+test_setup_break_detection_3 () {\n \ttest_create_repo break-detection-3 &&\n \t(\n \t\tcd break-detection-3 &&\n@@ -202,9 +206,10 @@ test_expect_success 'setup undetected rename/add-source causes data loss' '\n \t\tgit add a &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_failure 'detect rename/add-source and preserve all data' '\n+\ttest_setup_break_detection_3 &&\n \t(\n \t\tcd break-detection-3 &&\n \n@@ -231,6 +236,7 @@ test_expect_failure 'detect rename/add-source and preserve all data' '\n '\n \n test_expect_failure 'detect rename/add-source and preserve all data, merge other way' '\n+\ttest_setup_break_detection_3 &&\n \t(\n \t\tcd break-detection-3 &&\n \n@@ -256,10 +262,10 @@ test_expect_failure 'detect rename/add-source and preserve all data, merge other\n \t)\n '\n \n-test_expect_success 'setup content merge + rename/directory conflict' '\n-\ttest_create_repo rename-directory-1 &&\n+test_setup_rename_directory () {\n+\ttest_create_repo rename-directory-$1 &&\n \t(\n-\t\tcd rename-directory-1 &&\n+\t\tcd rename-directory-$1 &&\n \n \t\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n\" >file &&\n \t\tgit add file &&\n@@ -290,11 +296,12 @@ test_expect_success 'setup content merge + rename/directory conflict' '\n \t\ttest_tick &&\n \t\tgit commit -m left\n \t)\n-'\n+}\n \n test_expect_success 'rename/directory conflict + clean content merge' '\n+\ttest_setup_rename_directory 1a &&\n \t(\n-\t\tcd rename-directory-1 &&\n+\t\tcd rename-directory-1a &&\n \n \t\tgit checkout left-clean^0 &&\n \n@@ -320,8 +327,9 @@ test_expect_success 'rename/directory conflict + clean content merge' '\n '\n \n test_expect_success 'rename/directory conflict + content merge conflict' '\n+\ttest_setup_rename_directory 1b &&\n \t(\n-\t\tcd rename-directory-1 &&\n+\t\tcd rename-directory-1b &&\n \n \t\tgit reset --hard &&\n \t\tgit clean -fdqx &&\n@@ -358,7 +366,7 @@ test_expect_success 'rename/directory conflict + content merge conflict' '\n \t)\n '\n \n-test_expect_success 'setup content merge + rename/directory conflict w/ disappearing dir' '\n+test_setup_rename_directory_2 () {\n \ttest_create_repo rename-directory-2 &&\n \t(\n \t\tcd rename-directory-2 &&\n@@ -385,9 +393,10 @@ test_expect_success 'setup content merge + rename/directory conflict w/ disappea\n \t\ttest_tick &&\n \t\tgit commit -m left\n \t)\n-'\n+}\n \n test_expect_success 'disappearing dir in rename/directory conflict handled' '\n+\ttest_setup_rename_directory_2 &&\n \t(\n \t\tcd rename-directory-2 &&\n \n@@ -416,10 +425,10 @@ test_expect_success 'disappearing dir in rename/directory conflict handled' '\n # Commit A: rename a->b, modifying b too\n # Commit B: modify a, add different b\n \n-test_expect_success 'setup rename-with-content-merge vs. add' '\n-\ttest_create_repo rename-with-content-merge-and-add &&\n+test_setup_rename_with_content_merge_and_add () {\n+\ttest_create_repo rename-with-content-merge-and-add-$1 &&\n \t(\n-\t\tcd rename-with-content-merge-and-add &&\n+\t\tcd rename-with-content-merge-and-add-$1 &&\n \n \t\ttest_seq 1 5 >a &&\n \t\tgit add a &&\n@@ -438,11 +447,12 @@ test_expect_success 'setup rename-with-content-merge vs. add' '\n \t\tgit add a b &&\n \t\tgit commit -m B\n \t)\n-'\n+}\n \n test_expect_success 'handle rename-with-content-merge vs. add' '\n+\ttest_setup_rename_with_content_merge_and_add AB &&\n \t(\n-\t\tcd rename-with-content-merge-and-add &&\n+\t\tcd rename-with-content-merge-and-add-AB &&\n \n \t\tgit checkout A^0 &&\n \n@@ -483,8 +493,9 @@ test_expect_success 'handle rename-with-content-merge vs. add' '\n '\n \n test_expect_success 'handle rename-with-content-merge vs. add, merge other way' '\n+\ttest_setup_rename_with_content_merge_and_add BA &&\n \t(\n-\t\tcd rename-with-content-merge-and-add &&\n+\t\tcd rename-with-content-merge-and-add-BA &&\n \n \t\tgit reset --hard &&\n \t\tgit clean -fdx &&\n@@ -539,7 +550,7 @@ test_expect_success 'handle rename-with-content-merge vs. add, merge other way'\n # * The working copy should have two files, both of form c~; does it?\n # * Nothing else should be present. Is anything?\n \n-test_expect_success 'setup rename/rename (2to1) + modify/modify' '\n+test_setup_rename_rename_2to1 () {\n \ttest_create_repo rename-rename-2to1 &&\n \t(\n \t\tcd rename-rename-2to1 &&\n@@ -562,9 +573,10 @@ test_expect_success 'setup rename/rename (2to1) + modify/modify' '\n \t\tgit add a &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_success 'handle rename/rename (2to1) conflict correctly' '\n+\ttest_setup_rename_rename_2to1 &&\n \t(\n \t\tcd rename-rename-2to1 &&\n \n@@ -610,7 +622,7 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '\n # Commit A: new file: a\n # Commit B: rename a->b\n # Commit C: rename a->c\n-test_expect_success 'setup simple rename/rename (1to2) conflict' '\n+test_setup_rename_rename_1to2 () {\n \ttest_create_repo rename-rename-1to2 &&\n \t(\n \t\tcd rename-rename-1to2 &&\n@@ -631,9 +643,10 @@ test_expect_success 'setup simple rename/rename (1to2) conflict' '\n \t\ttest_tick &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_success 'merge has correct working tree contents' '\n+\ttest_setup_rename_rename_1to2 &&\n \t(\n \t\tcd rename-rename-1to2 &&\n \n@@ -667,7 +680,7 @@ test_expect_success 'merge has correct working tree contents' '\n #\n # Merging of B & C should NOT be clean; there's a rename/rename conflict\n \n-test_expect_success 'setup rename/rename(1to2)/add-source conflict' '\n+test_setup_rename_rename_1to2_add_source_1 () {\n \ttest_create_repo rename-rename-1to2-add-source-1 &&\n \t(\n \t\tcd rename-rename-1to2-add-source-1 &&\n@@ -687,9 +700,10 @@ test_expect_success 'setup rename/rename(1to2)/add-source conflict' '\n \t\tgit add a &&\n \t\tgit commit -m C\n \t)\n-'\n+}\n \n test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge' '\n+\ttest_setup_rename_rename_1to2_add_source_1 &&\n \t(\n \t\tcd rename-rename-1to2-add-source-1 &&\n \n@@ -714,7 +728,7 @@ test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge'\n \t)\n '\n \n-test_expect_success 'setup rename/rename(1to2)/add-source resolvable conflict' '\n+test_setup_rename_rename_1to2_add_source_2 () {\n \ttest_create_repo rename-rename-1to2-add-source-2 &&\n \t(\n \t\tcd rename-rename-1to2-add-source-2 &&\n@@ -737,9 +751,10 @@ test_expect_success 'setup rename/rename(1to2)/add-source resolvable conflict' '\n \t\ttest_tick &&\n \t\tgit commit -m two\n \t)\n-'\n+}\n \n test_expect_failure 'rename/rename/add-source still tracks new a file' '\n+\ttest_setup_rename_rename_1to2_add_source_2 &&\n \t(\n \t\tcd rename-rename-1to2-add-source-2 &&\n \n@@ -759,7 +774,7 @@ test_expect_failure 'rename/rename/add-source still tracks new a file' '\n \t)\n '\n \n-test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '\n+test_setup_rename_rename_1to2_add_dest () {\n \ttest_create_repo rename-rename-1to2-add-dest &&\n \t(\n \t\tcd rename-rename-1to2-add-dest &&\n@@ -784,9 +799,10 @@ test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '\n \t\ttest_tick &&\n \t\tgit commit -m two\n \t)\n-'\n+}\n \n test_expect_success 'rename/rename/add-dest merge still knows about conflicting file versions' '\n+\ttest_setup_rename_rename_1to2_add_dest &&\n \t(\n \t\tcd rename-rename-1to2-add-dest &&\n \n@@ -838,7 +854,7 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting\n # Commit B: rename foo->bar\n # Expected: CONFLICT (rename/add/delete), two-way merged bar\n \n-test_expect_success 'rad-setup: rename/add/delete conflict' '\n+test_setup_rad () {\n \ttest_create_repo rad &&\n \t(\n \t\tcd rad &&\n@@ -860,9 +876,10 @@ test_expect_success 'rad-setup: rename/add/delete conflict' '\n \t\tgit mv foo bar &&\n \t\tgit commit -m \"rename foo to bar\"\n \t)\n-'\n+}\n \n test_expect_failure 'rad-check: rename/add/delete conflict' '\n+\ttest_setup_rad &&\n \t(\n \t\tcd rad &&\n \n@@ -904,7 +921,7 @@ test_expect_failure 'rad-check: rename/add/delete conflict' '\n # Commit B: rename bar->baz, rm foo\n # Expected: CONFLICT (rename/rename/delete/delete), two-way merged baz\n \n-test_expect_success 'rrdd-setup: rename/rename(2to1)/delete/delete conflict' '\n+test_setup_rrdd () {\n \ttest_create_repo rrdd &&\n \t(\n \t\tcd rrdd &&\n@@ -927,9 +944,10 @@ test_expect_success 'rrdd-setup: rename/rename(2to1)/delete/delete conflict' '\n \t\tgit rm foo &&\n \t\tgit commit -m \"Rename bar, remove foo\"\n \t)\n-'\n+}\n \n test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '\n+\ttest_setup_rrdd &&\n \t(\n \t\tcd rrdd &&\n \n@@ -973,7 +991,7 @@ test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '\n # Expected: six CONFLICT(rename/rename) messages, each path in two of the\n # multi-way merged contents found in two, four, six\n \n-test_expect_success 'mod6-setup: chains of rename/rename(1to2) and rename/rename(2to1)' '\n+test_setup_mod6 () {\n \ttest_create_repo mod6 &&\n \t(\n \t\tcd mod6 &&\n@@ -1009,9 +1027,10 @@ test_expect_success 'mod6-setup: chains of rename/rename(1to2) and rename/rename\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '\n+\ttest_setup_mod6 &&\n \t(\n \t\tcd mod6 &&\n \n@@ -1108,7 +1127,8 @@ test_conflicts_with_adds_and_renames() {\n \t# files. Is it present?\n \t# 4) There should not be any three~* files in the working\n \t# tree\n-\ttest_expect_success \"setup simple $sideL/$sideR conflict\" '\n+\ttest_setup_collision_conflict () {\n+\t#test_expect_success \"setup simple $sideL/$sideR conflict\" '\n \t\ttest_create_repo simple_${sideL}_${sideR} &&\n \t\t(\n \t\t\tcd simple_${sideL}_${sideR} &&\n@@ -1185,9 +1205,11 @@ test_conflicts_with_adds_and_renames() {\n \t\t\tfi &&\n \t\t\ttest_tick && git commit -m R\n \t\t)\n-\t'\n+\t#'\n+\t}\n \n \ttest_expect_success \"check simple $sideL/$sideR conflict\" '\n+\t\ttest_setup_collision_conflict &&\n \t\t(\n \t\t\tcd simple_${sideL}_${sideR} &&\n \n@@ -1254,7 +1276,7 @@ test_conflicts_with_adds_and_renames add add\n #\n # So, we have four different conflicting files that all end up at path\n # 'three'.\n-test_expect_success 'setup nested conflicts from rename/rename(2to1)' '\n+test_setup_nested_conflicts_from_rename_rename () {\n \ttest_create_repo nested_conflicts_from_rename_rename &&\n \t(\n \t\tcd nested_conflicts_from_rename_rename &&\n@@ -1305,9 +1327,10 @@ test_expect_success 'setup nested conflicts from rename/rename(2to1)' '\n \t\tgit add one three &&\n \t\ttest_tick && git commit -m german\n \t)\n-'\n+}\n \n test_expect_success 'check nested conflicts from rename/rename(2to1)' '\n+\ttest_setup_nested_conflicts_from_rename_rename &&\n \t(\n \t\tcd nested_conflicts_from_rename_rename &&\n \ndiff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\nindex 32cdd1f493..bd2f97ba95 100755\n--- a/t/t6043-merge-rename-directories.sh\n+++ b/t/t6043-merge-rename-directories.sh\n@@ -38,7 +38,7 @@ test_description=\"recursive merge with directory renames\"\n # Commit B: z/{b,c,d,e/f}\n # Expected: y/{b,c,d,e/f}\n \n-test_expect_success '1a-setup: Simple directory rename detection' '\n+test_setup_1a () {\n \ttest_create_repo 1a &&\n \t(\n \t\tcd 1a &&\n@@ -67,9 +67,10 @@ test_expect_success '1a-setup: Simple directory rename detection' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1a-check: Simple directory rename detection' '\n+test_expect_success '1a: Simple directory rename detection' '\n+\ttest_setup_1a &&\n \t(\n \t\tcd 1a &&\n \n@@ -103,7 +104,7 @@ test_expect_success '1a-check: Simple directory rename detection' '\n # Commit B: y/{b,c,d}\n # Expected: y/{b,c,d,e}\n \n-test_expect_success '1b-setup: Merge a directory with another' '\n+test_setup_1b () {\n \ttest_create_repo 1b &&\n \t(\n \t\tcd 1b &&\n@@ -134,9 +135,10 @@ test_expect_success '1b-setup: Merge a directory with another' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1b-check: Merge a directory with another' '\n+test_expect_success '1b: Merge a directory with another' '\n+\ttest_setup_1b &&\n \t(\n \t\tcd 1b &&\n \n@@ -165,7 +167,7 @@ test_expect_success '1b-check: Merge a directory with another' '\n # Commit B: z/{b,c,d}\n # Expected: y/{b,c,d} (because x/d -> z/d -> y/d)\n \n-test_expect_success '1c-setup: Transitive renaming' '\n+test_setup_1c () {\n \ttest_create_repo 1c &&\n \t(\n \t\tcd 1c &&\n@@ -193,9 +195,10 @@ test_expect_success '1c-setup: Transitive renaming' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1c-check: Transitive renaming' '\n+test_expect_success '1c: Transitive renaming' '\n+\ttest_setup_1c &&\n \t(\n \t\tcd 1c &&\n \n@@ -227,7 +230,7 @@ test_expect_success '1c-check: Transitive renaming' '\n # Note: y/m & z/n should definitely move into x. By the same token, both\n # y/wham_1 & z/wham_2 should too...giving us a conflict.\n \n-test_expect_success '1d-setup: Directory renames cause a rename/rename(2to1) conflict' '\n+test_setup_1d () {\n \ttest_create_repo 1d &&\n \t(\n \t\tcd 1d &&\n@@ -262,9 +265,10 @@ test_expect_success '1d-setup: Directory renames cause a rename/rename(2to1) con\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) conflict' '\n+test_expect_success '1d: Directory renames cause a rename/rename(2to1) conflict' '\n+\ttest_setup_1d &&\n \t(\n \t\tcd 1d &&\n \n@@ -313,7 +317,7 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con\n # Commit B: z/{oldb,oldc,d}\n # Expected: y/{newb,newc,d}\n \n-test_expect_success '1e-setup: Renamed directory, with all files being renamed too' '\n+test_setup_1e () {\n \ttest_create_repo 1e &&\n \t(\n \t\tcd 1e &&\n@@ -342,9 +346,10 @@ test_expect_success '1e-setup: Renamed directory, with all files being renamed t\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1e-check: Renamed directory, with all files being renamed too' '\n+test_expect_success '1e: Renamed directory, with all files being renamed too' '\n+\ttest_setup_1e &&\n \t(\n \t\tcd 1e &&\n \n@@ -371,7 +376,7 @@ test_expect_success '1e-check: Renamed directory, with all files being renamed t\n # Commit B: y/{b,c}, x/{d,e,f}\n # Expected: y/{b,c}, x/{d,e,f,g}\n \n-test_expect_success '1f-setup: Split a directory into two other directories' '\n+test_setup_1f () {\n \ttest_create_repo 1f &&\n \t(\n \t\tcd 1f &&\n@@ -408,9 +413,10 @@ test_expect_success '1f-setup: Split a directory into two other directories' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1f-check: Split a directory into two other directories' '\n+test_expect_success '1f: Split a directory into two other directories' '\n+\ttest_setup_1f &&\n \t(\n \t\tcd 1f &&\n \n@@ -459,7 +465,7 @@ test_expect_success '1f-check: Split a directory into two other directories' '\n # Commit A: y/b, w/c\n # Commit B: z/{b,c,d}\n # Expected: y/b, w/c, z/d, with warning about z/ -> (y/ vs. w/) conflict\n-test_expect_success '2a-setup: Directory split into two on one side, with equal numbers of paths' '\n+test_setup_2a () {\n \ttest_create_repo 2a &&\n \t(\n \t\tcd 2a &&\n@@ -489,9 +495,10 @@ test_expect_success '2a-setup: Directory split into two on one side, with equal\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '2a-check: Directory split into two on one side, with equal numbers of paths' '\n+test_expect_success '2a: Directory split into two on one side, with equal numbers of paths' '\n+\ttest_setup_2a &&\n \t(\n \t\tcd 2a &&\n \n@@ -520,7 +527,7 @@ test_expect_success '2a-check: Directory split into two on one side, with equal\n # Commit A: y/b, w/c\n # Commit B: z/{b,c}, x/d\n # Expected: y/b, w/c, x/d; No warning about z/ -> (y/ vs. w/) conflict\n-test_expect_success '2b-setup: Directory split into two on one side, with equal numbers of paths' '\n+test_setup_2b () {\n \ttest_create_repo 2b &&\n \t(\n \t\tcd 2b &&\n@@ -551,9 +558,10 @@ test_expect_success '2b-setup: Directory split into two on one side, with equal\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '2b-check: Directory split into two on one side, with equal numbers of paths' '\n+test_expect_success '2b: Directory split into two on one side, with equal numbers of paths' '\n+\ttest_setup_2b &&\n \t(\n \t\tcd 2b &&\n \n@@ -601,7 +609,7 @@ test_expect_success '2b-check: Directory split into two on one side, with equal\n # Commit A: z/{b,c,d} (no change)\n # Commit B: y/{b,c}, x/d\n # Expected: y/{b,c}, x/d\n-test_expect_success '3a-setup: Avoid implicit rename if involved as source on other side' '\n+test_setup_3a () {\n \ttest_create_repo 3a &&\n \t(\n \t\tcd 3a &&\n@@ -632,9 +640,10 @@ test_expect_success '3a-setup: Avoid implicit rename if involved as source on ot\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '3a-check: Avoid implicit rename if involved as source on other side' '\n+test_expect_success '3a: Avoid implicit rename if involved as source on other side' '\n+\ttest_setup_3a &&\n \t(\n \t\tcd 3a &&\n \n@@ -664,7 +673,7 @@ test_expect_success '3a-check: Avoid implicit rename if involved as source on ot\n # get it involved in directory rename detection. If it were, we might\n # end up with CONFLICT:(z/d -> y/d vs. x/d vs. w/d), i.e. a\n # rename/rename/rename(1to3) conflict, which is just weird.\n-test_expect_success '3b-setup: Avoid implicit rename if involved as source on current side' '\n+test_setup_3b () {\n \ttest_create_repo 3b &&\n \t(\n \t\tcd 3b &&\n@@ -697,9 +706,10 @@ test_expect_success '3b-setup: Avoid implicit rename if involved as source on cu\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '3b-check: Avoid implicit rename if involved as source on current side' '\n+test_expect_success '3b: Avoid implicit rename if involved as source on current side' '\n+\ttest_setup_3b &&\n \t(\n \t\tcd 3b &&\n \n@@ -786,7 +796,7 @@ test_expect_success '3b-check: Avoid implicit rename if involved as source on cu\n # Expected: y/{b,c,d}, z/{e,f}\n # NOTE: Even though most files from z moved to y, we don't want f to follow.\n \n-test_expect_success '4a-setup: Directory split, with original directory still present' '\n+test_setup_4a () {\n \ttest_create_repo 4a &&\n \t(\n \t\tcd 4a &&\n@@ -818,9 +828,10 @@ test_expect_success '4a-setup: Directory split, with original directory still pr\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '4a-check: Directory split, with original directory still present' '\n+test_expect_success '4a: Directory split, with original directory still present' '\n+\ttest_setup_4a &&\n \t(\n \t\tcd 4a &&\n \n@@ -874,7 +885,7 @@ test_expect_success '4a-check: Directory split, with original directory still pr\n # of history, giving us no way to represent this conflict in the\n # index.\n \n-test_expect_success '5a-setup: Merge directories, other side adds files to original and target' '\n+test_setup_5a () {\n \ttest_create_repo 5a &&\n \t(\n \t\tcd 5a &&\n@@ -907,9 +918,10 @@ test_expect_success '5a-setup: Merge directories, other side adds files to origi\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '5a-check: Merge directories, other side adds files to original and target' '\n+test_expect_success '5a: Merge directories, other side adds files to original and target' '\n+\ttest_setup_5a &&\n \t(\n \t\tcd 5a &&\n \n@@ -948,7 +960,7 @@ test_expect_success '5a-check: Merge directories, other side adds files to origi\n # cause us to bail on directory rename detection for that path, falling\n # back to git behavior without the directory rename detection.\n \n-test_expect_success '5b-setup: Rename/delete in order to get add/add/add conflict' '\n+test_setup_5b () {\n \ttest_create_repo 5b &&\n \t(\n \t\tcd 5b &&\n@@ -981,9 +993,10 @@ test_expect_success '5b-setup: Rename/delete in order to get add/add/add conflic\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '5b-check: Rename/delete in order to get add/add/add conflict' '\n+test_expect_success '5b: Rename/delete in order to get add/add/add conflict' '\n+\ttest_setup_5b &&\n \t(\n \t\tcd 5b &&\n \n@@ -1024,7 +1037,7 @@ test_expect_success '5b-check: Rename/delete in order to get add/add/add conflic\n # y/d are y/d_2 and y/d_4. We still do the move from z/e to y/e,\n # though, because it doesn't have anything in the way.\n \n-test_expect_success '5c-setup: Transitive rename would cause rename/rename/rename/add/add/add' '\n+test_setup_5c () {\n \ttest_create_repo 5c &&\n \t(\n \t\tcd 5c &&\n@@ -1061,9 +1074,10 @@ test_expect_success '5c-setup: Transitive rename would cause rename/rename/renam\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '5c-check: Transitive rename would cause rename/rename/rename/add/add/add' '\n+test_expect_success '5c: Transitive rename would cause rename/rename/rename/add/add/add' '\n+\ttest_setup_5c &&\n \t(\n \t\tcd 5c &&\n \n@@ -1113,7 +1127,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam\n # detection for z/d_2, but that doesn't prevent us from applying the\n # directory rename detection for z/f -> y/f.\n \n-test_expect_success '5d-setup: Directory/file/file conflict due to directory rename' '\n+test_setup_5d () {\n \ttest_create_repo 5d &&\n \t(\n \t\tcd 5d &&\n@@ -1145,9 +1159,10 @@ test_expect_success '5d-setup: Directory/file/file conflict due to directory ren\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '5d-check: Directory/file/file conflict due to directory rename' '\n+test_expect_success '5d: Directory/file/file conflict due to directory rename' '\n+\ttest_setup_5d &&\n \t(\n \t\tcd 5d &&\n \n@@ -1205,7 +1220,7 @@ test_expect_success '5d-check: Directory/file/file conflict due to directory ren\n # them under y/ doesn't accidentally catch z/d and make it look like\n # it is also involved in a rename/delete conflict.\n \n-test_expect_success '6a-setup: Tricky rename/delete' '\n+test_setup_6a () {\n \ttest_create_repo 6a &&\n \t(\n \t\tcd 6a &&\n@@ -1235,9 +1250,10 @@ test_expect_success '6a-setup: Tricky rename/delete' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '6a-check: Tricky rename/delete' '\n+test_expect_success '6a: Tricky rename/delete' '\n+\ttest_setup_6a &&\n \t(\n \t\tcd 6a &&\n \n@@ -1271,7 +1287,7 @@ test_expect_success '6a-check: Tricky rename/delete' '\n # but B did that rename and still decided to put the file into z/,\n # so we probably shouldn't apply directory rename detection for it.\n \n-test_expect_success '6b-setup: Same rename done on both sides' '\n+test_setup_6b () {\n \ttest_create_repo 6b &&\n \t(\n \t\tcd 6b &&\n@@ -1300,9 +1316,10 @@ test_expect_success '6b-setup: Same rename done on both sides' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '6b-check: Same rename done on both sides' '\n+test_expect_success '6b: Same rename done on both sides' '\n+\ttest_setup_6b &&\n \t(\n \t\tcd 6b &&\n \n@@ -1334,7 +1351,7 @@ test_expect_success '6b-check: Same rename done on both sides' '\n # NOTE: Seems obvious, but just checking that the implementation doesn't\n # \"accidentally detect a rename\" and give us y/{b,c,d}.\n \n-test_expect_success '6c-setup: Rename only done on same side' '\n+test_setup_6c () {\n \ttest_create_repo 6c &&\n \t(\n \t\tcd 6c &&\n@@ -1362,9 +1379,10 @@ test_expect_success '6c-setup: Rename only done on same side' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '6c-check: Rename only done on same side' '\n+test_expect_success '6c: Rename only done on same side' '\n+\ttest_setup_6c &&\n \t(\n \t\tcd 6c &&\n \n@@ -1396,7 +1414,7 @@ test_expect_success '6c-check: Rename only done on same side' '\n # NOTE: Again, this seems obvious but just checking that the implementation\n # doesn't \"accidentally detect a rename\" and give us y/{b,c,d}.\n \n-test_expect_success '6d-setup: We do not always want transitive renaming' '\n+test_setup_6d () {\n \ttest_create_repo 6d &&\n \t(\n \t\tcd 6d &&\n@@ -1424,9 +1442,10 @@ test_expect_success '6d-setup: We do not always want transitive renaming' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '6d-check: We do not always want transitive renaming' '\n+test_expect_success '6d: We do not always want transitive renaming' '\n+\ttest_setup_6d &&\n \t(\n \t\tcd 6d &&\n \n@@ -1458,7 +1477,7 @@ test_expect_success '6d-check: We do not always want transitive renaming' '\n # doesn't \"accidentally detect a rename\" and give us y/{b,c} +\n # add/add conflict on y/d_1 vs y/d_2.\n \n-test_expect_success '6e-setup: Add/add from one side' '\n+test_setup_6e () {\n \ttest_create_repo 6e &&\n \t(\n \t\tcd 6e &&\n@@ -1487,9 +1506,10 @@ test_expect_success '6e-setup: Add/add from one side' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '6e-check: Add/add from one side' '\n+test_expect_success '6e: Add/add from one side' '\n+\ttest_setup_6e &&\n \t(\n \t\tcd 6e &&\n \n@@ -1552,7 +1572,7 @@ test_expect_success '6e-check: Add/add from one side' '\n # Expected: y/d, CONFLICT(rename/rename for both z/b and z/c)\n # NOTE: There's a rename of z/ here, y/ has more renames, so z/d -> y/d.\n \n-test_expect_success '7a-setup: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '\n+test_setup_7a () {\n \ttest_create_repo 7a &&\n \t(\n \t\tcd 7a &&\n@@ -1583,9 +1603,10 @@ test_expect_success '7a-setup: rename-dir vs. rename-dir (NOT split evenly) PLUS\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '\n+test_expect_success '7a: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '\n+\ttest_setup_7a &&\n \t(\n \t\tcd 7a &&\n \n@@ -1623,7 +1644,7 @@ test_expect_success '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS\n # Commit B: z/{b,c,d_1}, w/d_2\n # Expected: y/{b,c}, CONFLICT(rename/rename(2to1): x/d_1, w/d_2 -> y_d)\n \n-test_expect_success '7b-setup: rename/rename(2to1), but only due to transitive rename' '\n+test_setup_7b () {\n \ttest_create_repo 7b &&\n \t(\n \t\tcd 7b &&\n@@ -1655,9 +1676,10 @@ test_expect_success '7b-setup: rename/rename(2to1), but only due to transitive r\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '7b-check: rename/rename(2to1), but only due to transitive rename' '\n+test_expect_success '7b: rename/rename(2to1), but only due to transitive rename' '\n+\ttest_setup_7b &&\n \t(\n \t\tcd 7b &&\n \n@@ -1702,7 +1724,7 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r\n # neither CONFLICT(x/d -> w/d vs. z/d)\n # nor CONFLiCT x/d -> w/d vs. y/d vs. z/d)\n \n-test_expect_success '7c-setup: rename/rename(1to...2or3); transitive rename may add complexity' '\n+test_setup_7c () {\n \ttest_create_repo 7c &&\n \t(\n \t\tcd 7c &&\n@@ -1732,9 +1754,10 @@ test_expect_success '7c-setup: rename/rename(1to...2or3); transitive rename may\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '7c-check: rename/rename(1to...2or3); transitive rename may add complexity' '\n+test_expect_success '7c: rename/rename(1to...2or3); transitive rename may add complexity' '\n+\ttest_setup_7c &&\n \t(\n \t\tcd 7c &&\n \n@@ -1766,7 +1789,7 @@ test_expect_success '7c-check: rename/rename(1to...2or3); transitive rename may\n # Expected: y/{b,c}, CONFLICT(delete x/d vs rename to y/d)\n # NOTE: z->y so NOT CONFLICT(delete x/d vs rename to z/d)\n \n-test_expect_success '7d-setup: transitive rename involved in rename/delete; how is it reported?' '\n+test_setup_7d () {\n \ttest_create_repo 7d &&\n \t(\n \t\tcd 7d &&\n@@ -1796,9 +1819,10 @@ test_expect_success '7d-setup: transitive rename involved in rename/delete; how\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '7d-check: transitive rename involved in rename/delete; how is it reported?' '\n+test_expect_success '7d: transitive rename involved in rename/delete; how is it reported?' '\n+\ttest_setup_7d &&\n \t(\n \t\tcd 7d &&\n \n@@ -1851,7 +1875,7 @@ test_expect_success '7d-check: transitive rename involved in rename/delete; how\n # see testcases 9c and 9d for further discussion of this issue and\n # how it's resolved.\n \n-test_expect_success '7e-setup: transitive rename in rename/delete AND dirs in the way' '\n+test_setup_7e () {\n \ttest_create_repo 7e &&\n \t(\n \t\tcd 7e &&\n@@ -1886,9 +1910,10 @@ test_expect_success '7e-setup: transitive rename in rename/delete AND dirs in th\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '7e-check: transitive rename in rename/delete AND dirs in the way' '\n+test_expect_success '7e: transitive rename in rename/delete AND dirs in the way' '\n+\ttest_setup_7e &&\n \t(\n \t\tcd 7e &&\n \n@@ -1945,7 +1970,7 @@ test_expect_success '7e-check: transitive rename in rename/delete AND dirs in th\n # simple rule from section 5 prevents me from handling this as optimally as\n # we potentially could.\n \n-test_expect_success '8a-setup: Dual-directory rename, one into the others way' '\n+test_setup_8a () {\n \ttest_create_repo 8a &&\n \t(\n \t\tcd 8a &&\n@@ -1977,9 +2002,10 @@ test_expect_success '8a-setup: Dual-directory rename, one into the others way' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '8a-check: Dual-directory rename, one into the others way' '\n+test_expect_success '8a: Dual-directory rename, one into the others way' '\n+\ttest_setup_8a &&\n \t(\n \t\tcd 8a &&\n \n@@ -2023,7 +2049,7 @@ test_expect_success '8a-check: Dual-directory rename, one into the others way' '\n # making us fall back to pre-directory-rename-detection behavior for both\n # e_1 and e_2.\n \n-test_expect_success '8b-setup: Dual-directory rename, one into the others way, with conflicting filenames' '\n+test_setup_8b () {\n \ttest_create_repo 8b &&\n \t(\n \t\tcd 8b &&\n@@ -2055,9 +2081,10 @@ test_expect_success '8b-setup: Dual-directory rename, one into the others way, w\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '8b-check: Dual-directory rename, one into the others way, with conflicting filenames' '\n+test_expect_success '8b: Dual-directory rename, one into the others way, with conflicting filenames' '\n+\ttest_setup_8b &&\n \t(\n \t\tcd 8b &&\n \n@@ -2096,7 +2123,7 @@ test_expect_success '8b-check: Dual-directory rename, one into the others way, w\n # rename/rename(1to2) conflicts -- see testcase 9h. See also\n # notes in 8d.\n \n-test_expect_success '8c-setup: modify/delete or rename+modify/delete?' '\n+test_setup_8c () {\n \ttest_create_repo 8c &&\n \t(\n \t\tcd 8c &&\n@@ -2127,9 +2154,10 @@ test_expect_success '8c-setup: modify/delete or rename+modify/delete?' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '8c-check: modify/delete or rename+modify/delete' '\n+test_expect_success '8c: modify/delete or rename+modify/delete' '\n+\ttest_setup_8c &&\n \t(\n \t\tcd 8c &&\n \n@@ -2175,7 +2203,7 @@ test_expect_success '8c-check: modify/delete or rename+modify/delete' '\n # during merging are supposed to be about opposite sides doing things\n # differently.\n \n-test_expect_success '8d-setup: rename/delete...or not?' '\n+test_setup_8d () {\n \ttest_create_repo 8d &&\n \t(\n \t\tcd 8d &&\n@@ -2204,9 +2232,10 @@ test_expect_success '8d-setup: rename/delete...or not?' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '8d-check: rename/delete...or not?' '\n+test_expect_success '8d: rename/delete...or not?' '\n+\ttest_setup_8d &&\n \t(\n \t\tcd 8d &&\n \n@@ -2250,7 +2279,7 @@ test_expect_success '8d-check: rename/delete...or not?' '\n # about the ramifications of doing that, I didn't know how to rule out\n # that opening other weird edge and corner cases so I just punted.\n \n-test_expect_success '8e-setup: Both sides rename, one side adds to original directory' '\n+test_setup_8e () {\n \ttest_create_repo 8e &&\n \t(\n \t\tcd 8e &&\n@@ -2279,9 +2308,10 @@ test_expect_success '8e-setup: Both sides rename, one side adds to original dire\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '8e-check: Both sides rename, one side adds to original directory' '\n+test_expect_success '8e: Both sides rename, one side adds to original directory' '\n+\ttest_setup_8e &&\n \t(\n \t\tcd 8e &&\n \n@@ -2333,7 +2363,7 @@ test_expect_success '8e-check: Both sides rename, one side adds to original dire\n # of which one had the most paths going to it. A naive implementation\n # of that could take the new file in commit B at z/i to x/w/i or x/i.\n \n-test_expect_success '9a-setup: Inner renamed directory within outer renamed directory' '\n+test_setup_9a () {\n \ttest_create_repo 9a &&\n \t(\n \t\tcd 9a &&\n@@ -2366,9 +2396,10 @@ test_expect_success '9a-setup: Inner renamed directory within outer renamed dire\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9a-check: Inner renamed directory within outer renamed directory' '\n+test_expect_success '9a: Inner renamed directory within outer renamed directory' '\n+\ttest_setup_9a &&\n \t(\n \t\tcd 9a &&\n \n@@ -2404,7 +2435,7 @@ test_expect_success '9a-check: Inner renamed directory within outer renamed dire\n # Commit B: z/{b,c,d_3}\n # Expected: y/{b,c,d_merged}\n \n-test_expect_success '9b-setup: Transitive rename with content merge' '\n+test_setup_9b () {\n \ttest_create_repo 9b &&\n \t(\n \t\tcd 9b &&\n@@ -2436,9 +2467,10 @@ test_expect_success '9b-setup: Transitive rename with content merge' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9b-check: Transitive rename with content merge' '\n+test_expect_success '9b: Transitive rename with content merge' '\n+\ttest_setup_9b &&\n \t(\n \t\tcd 9b &&\n \n@@ -2491,7 +2523,7 @@ test_expect_success '9b-check: Transitive rename with content merge' '\n # away, then ignore that particular rename from the other side of\n # history for any implicit directory renames.\n \n-test_expect_success '9c-setup: Doubly transitive rename?' '\n+test_setup_9c () {\n \ttest_create_repo 9c &&\n \t(\n \t\tcd 9c &&\n@@ -2526,9 +2558,10 @@ test_expect_success '9c-setup: Doubly transitive rename?' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9c-check: Doubly transitive rename?' '\n+test_expect_success '9c: Doubly transitive rename?' '\n+\ttest_setup_9c &&\n \t(\n \t\tcd 9c &&\n \n@@ -2579,7 +2612,7 @@ test_expect_success '9c-check: Doubly transitive rename?' '\n # simple rules that are consistent with what we need for all the other\n # testcases and simplifies things for the user.\n \n-test_expect_success '9d-setup: N-way transitive rename?' '\n+test_setup_9d () {\n \ttest_create_repo 9d &&\n \t(\n \t\tcd 9d &&\n@@ -2614,9 +2647,10 @@ test_expect_success '9d-setup: N-way transitive rename?' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9d-check: N-way transitive rename?' '\n+test_expect_success '9d: N-way transitive rename?' '\n+\ttest_setup_9d &&\n \t(\n \t\tcd 9d &&\n \n@@ -2653,7 +2687,7 @@ test_expect_success '9d-check: N-way transitive rename?' '\n # Expected: combined/{a,b,c,d,e,f,g,h,i,j,k,l}, CONFLICT(Nto1) warnings,\n # dir1/yo, dir2/yo, dir3/yo, dirN/yo\n \n-test_expect_success '9e-setup: N-to-1 whammo' '\n+test_setup_9e () {\n \ttest_create_repo 9e &&\n \t(\n \t\tcd 9e &&\n@@ -2696,9 +2730,10 @@ test_expect_success '9e-setup: N-to-1 whammo' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success C_LOCALE_OUTPUT '9e-check: N-to-1 whammo' '\n+test_expect_success C_LOCALE_OUTPUT '9e: N-to-1 whammo' '\n+\ttest_setup_9e &&\n \t(\n \t\tcd 9e &&\n \n@@ -2745,7 +2780,7 @@ test_expect_success C_LOCALE_OUTPUT '9e-check: N-to-1 whammo' '\n # Commit B: goal/{a,b}/$more_files, goal/c\n # Expected: priority/{a,b}/$more_files, priority/c\n \n-test_expect_success '9f-setup: Renamed directory that only contained immediate subdirs' '\n+test_setup_9f () {\n \ttest_create_repo 9f &&\n \t(\n \t\tcd 9f &&\n@@ -2774,9 +2809,10 @@ test_expect_success '9f-setup: Renamed directory that only contained immediate s\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9f-check: Renamed directory that only contained immediate subdirs' '\n+test_expect_success '9f: Renamed directory that only contained immediate subdirs' '\n+\ttest_setup_9f &&\n \t(\n \t\tcd 9f &&\n \n@@ -2809,7 +2845,7 @@ test_expect_success '9f-check: Renamed directory that only contained immediate s\n # Commit B: goal/{a,b}/$more_files, goal/c\n # Expected: priority/{alpha,bravo}/$more_files, priority/c\n \n-test_expect_success '9g-setup: Renamed directory that only contained immediate subdirs, immediate subdirs renamed' '\n+test_setup_9g () {\n \ttest_create_repo 9g &&\n \t(\n \t\tcd 9g &&\n@@ -2841,9 +2877,9 @@ test_expect_success '9g-setup: Renamed directory that only contained immediate s\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_failure '9g-check: Renamed directory that only contained immediate subdirs, immediate subdirs renamed' '\n+test_expect_failure '9g: Renamed directory that only contained immediate subdirs, immediate subdirs renamed' '\n \t(\n \t\tcd 9g &&\n \n@@ -2877,7 +2913,7 @@ test_expect_failure '9g-check: Renamed directory that only contained immediate s\n # Expected: y/{b,c}, x/d_2\n # NOTE: If we applied the z/ -> y/ rename to z/d, then we'd end up with\n # a rename/rename(1to2) conflict (z/d -> y/d vs. x/d)\n-test_expect_success '9h-setup: Avoid dir rename on merely modified path' '\n+test_setup_9h () {\n \ttest_create_repo 9h &&\n \t(\n \t\tcd 9h &&\n@@ -2910,9 +2946,10 @@ test_expect_success '9h-setup: Avoid dir rename on merely modified path' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '9h-check: Avoid dir rename on merely modified path' '\n+test_expect_success '9h: Avoid dir rename on merely modified path' '\n+\ttest_setup_9h &&\n \t(\n \t\tcd 9h &&\n \n@@ -2957,7 +2994,7 @@ test_expect_success '9h-check: Avoid dir rename on merely modified path' '\n # Expected: Aborted Merge +\n # ERROR_MSG(untracked working tree files would be overwritten by merge)\n \n-test_expect_success '10a-setup: Overwrite untracked with normal rename/delete' '\n+test_setup_10a () {\n \ttest_create_repo 10a &&\n \t(\n \t\tcd 10a &&\n@@ -2983,9 +3020,10 @@ test_expect_success '10a-setup: Overwrite untracked with normal rename/delete' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '10a-check: Overwrite untracked with normal rename/delete' '\n+test_expect_success '10a: Overwrite untracked with normal rename/delete' '\n+\ttest_setup_10a &&\n \t(\n \t\tcd 10a &&\n \n@@ -3021,7 +3059,7 @@ test_expect_success '10a-check: Overwrite untracked with normal rename/delete' '\n # z/c_1 -> z/d_1 rename recorded at stage 3 for y/d +\n # ERROR_MSG(refusing to lose untracked file at 'y/d')\n \n-test_expect_success '10b-setup: Overwrite untracked with dir rename + delete' '\n+test_setup_10b () {\n \ttest_create_repo 10b &&\n \t(\n \t\tcd 10b &&\n@@ -3050,9 +3088,10 @@ test_expect_success '10b-setup: Overwrite untracked with dir rename + delete' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '10b-check: Overwrite untracked with dir rename + delete' '\n+test_expect_success '10b: Overwrite untracked with dir rename + delete' '\n+\ttest_setup_10b &&\n \t(\n \t\tcd 10b &&\n \n@@ -3098,10 +3137,10 @@ test_expect_success '10b-check: Overwrite untracked with dir rename + delete' '\n # y/c~B^0 +\n # ERROR_MSG(Refusing to lose untracked file at y/c)\n \n-test_expect_success '10c-setup: Overwrite untracked with dir rename/rename(1to2)' '\n-\ttest_create_repo 10c &&\n+test_setup_10c () {\n+\ttest_create_repo 10c_$1 &&\n \t(\n-\t\tcd 10c &&\n+\t\tcd 10c_$1 &&\n \n \t\tmkdir z x &&\n \t\techo a >z/a &&\n@@ -3128,11 +3167,12 @@ test_expect_success '10c-setup: Overwrite untracked with dir rename/rename(1to2)\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2)' '\n+test_expect_success '10c1: Overwrite untracked with dir rename/rename(1to2)' '\n+\ttest_setup_10c 1 &&\n \t(\n-\t\tcd 10c &&\n+\t\tcd 10c_1 &&\n \n \t\tgit checkout A^0 &&\n \t\techo important >y/c &&\n@@ -3163,9 +3203,10 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2)\n \t)\n '\n \n-test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2), other direction' '\n+test_expect_success '10c2: Overwrite untracked with dir rename/rename(1to2), other direction' '\n+\ttest_setup_10c 2 &&\n \t(\n-\t\tcd 10c &&\n+\t\tcd 10c_2 &&\n \n \t\tgit reset --hard &&\n \t\tgit clean -fdqx &&\n@@ -3208,7 +3249,7 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2)\n # CONFLICT(rename/rename) z/c_1 vs x/f_2 -> y/wham\n # ERROR_MSG(Refusing to lose untracked file at y/wham)\n \n-test_expect_success '10d-setup: Delete untracked with dir rename/rename(2to1)' '\n+test_setup_10d () {\n \ttest_create_repo 10d &&\n \t(\n \t\tcd 10d &&\n@@ -3240,9 +3281,10 @@ test_expect_success '10d-setup: Delete untracked with dir rename/rename(2to1)' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' '\n+test_expect_success '10d: Delete untracked with dir rename/rename(2to1)' '\n+\ttest_setup_10d &&\n \t(\n \t\tcd 10d &&\n \n@@ -3290,7 +3332,7 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' '\n # Commit B: z/{a,b,c}\n # Expected: y/{a,b,c} + untracked z/c\n \n-test_expect_success '10e-setup: Does git complain about untracked file that is not really in the way?' '\n+test_setup_10e () {\n \ttest_create_repo 10e &&\n \t(\n \t\tcd 10e &&\n@@ -3317,9 +3359,9 @@ test_expect_success '10e-setup: Does git complain about untracked file that is n\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_failure '10e-check: Does git complain about untracked file that is not really in the way?' '\n+test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' '\n \t(\n \t\tcd 10e &&\n \n@@ -3371,7 +3413,7 @@ test_expect_failure '10e-check: Does git complain about untracked file that is n\n # z/c~HEAD with contents of B:z/b_v2,\n # z/c with uncommitted mods on top of A:z/c_v1\n \n-test_expect_success '11a-setup: Avoid losing dirty contents with simple rename' '\n+test_setup_11a () {\n \ttest_create_repo 11a &&\n \t(\n \t\tcd 11a &&\n@@ -3398,9 +3440,10 @@ test_expect_success '11a-setup: Avoid losing dirty contents with simple rename'\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11a-check: Avoid losing dirty contents with simple rename' '\n+test_expect_success '11a: Avoid losing dirty contents with simple rename' '\n+\ttest_setup_11a &&\n \t(\n \t\tcd 11a &&\n \n@@ -3441,7 +3484,7 @@ test_expect_success '11a-check: Avoid losing dirty contents with simple rename'\n # ERROR_MSG(Refusing to lose dirty file at z/c)\n \n \n-test_expect_success '11b-setup: Avoid losing dirty file involved in directory rename' '\n+test_setup_11b () {\n \ttest_create_repo 11b &&\n \t(\n \t\tcd 11b &&\n@@ -3470,9 +3513,10 @@ test_expect_success '11b-setup: Avoid losing dirty file involved in directory re\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11b-check: Avoid losing dirty file involved in directory rename' '\n+test_expect_success '11b: Avoid losing dirty file involved in directory rename' '\n+\ttest_setup_11b &&\n \t(\n \t\tcd 11b &&\n \n@@ -3515,7 +3559,7 @@ test_expect_success '11b-check: Avoid losing dirty file involved in directory re\n # Expected: Abort_msg(\"following files would be overwritten by merge\") +\n # y/c left untouched (still has uncommitted mods)\n \n-test_expect_success '11c-setup: Avoid losing not-uptodate with rename + D/F conflict' '\n+test_setup_11c () {\n \ttest_create_repo 11c &&\n \t(\n \t\tcd 11c &&\n@@ -3545,9 +3589,10 @@ test_expect_success '11c-setup: Avoid losing not-uptodate with rename + D/F conf\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11c-check: Avoid losing not-uptodate with rename + D/F conflict' '\n+test_expect_success '11c: Avoid losing not-uptodate with rename + D/F conflict' '\n+\ttest_setup_11c &&\n \t(\n \t\tcd 11c &&\n \n@@ -3581,7 +3626,7 @@ test_expect_success '11c-check: Avoid losing not-uptodate with rename + D/F conf\n # Warning_Msg(\"Refusing to lose dirty file at z/c) +\n # y/{a,c~HEAD,c/d}, x/b, now-untracked z/c_v1 with uncommitted mods\n \n-test_expect_success '11d-setup: Avoid losing not-uptodate with rename + D/F conflict' '\n+test_setup_11d () {\n \ttest_create_repo 11d &&\n \t(\n \t\tcd 11d &&\n@@ -3612,9 +3657,10 @@ test_expect_success '11d-setup: Avoid losing not-uptodate with rename + D/F conf\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11d-check: Avoid losing not-uptodate with rename + D/F conflict' '\n+test_expect_success '11d: Avoid losing not-uptodate with rename + D/F conflict' '\n+\ttest_setup_11d &&\n \t(\n \t\tcd 11d &&\n \n@@ -3659,7 +3705,7 @@ test_expect_success '11d-check: Avoid losing not-uptodate with rename + D/F conf\n # y/c~HEAD has A:y/c_2 contents\n # y/c has dirty file from before merge\n \n-test_expect_success '11e-setup: Avoid deleting not-uptodate with dir rename/rename(1to2)/add' '\n+test_setup_11e () {\n \ttest_create_repo 11e &&\n \t(\n \t\tcd 11e &&\n@@ -3691,9 +3737,10 @@ test_expect_success '11e-setup: Avoid deleting not-uptodate with dir rename/rena\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rename(1to2)/add' '\n+test_expect_success '11e: Avoid deleting not-uptodate with dir rename/rename(1to2)/add' '\n+\ttest_setup_11e &&\n \t(\n \t\tcd 11e &&\n \n@@ -3744,7 +3791,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena\n # CONFLICT(rename/rename) x/c vs x/d -> y/wham\n # ERROR_MSG(Refusing to lose dirty file at y/wham)\n \n-test_expect_success '11f-setup: Avoid deleting not-uptodate with dir rename/rename(2to1)' '\n+test_setup_11f () {\n \ttest_create_repo 11f &&\n \t(\n \t\tcd 11f &&\n@@ -3773,9 +3820,10 @@ test_expect_success '11f-setup: Avoid deleting not-uptodate with dir rename/rena\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rename(2to1)' '\n+test_expect_success '11f: Avoid deleting not-uptodate with dir rename/rename(2to1)' '\n+\ttest_setup_11f &&\n \t(\n \t\tcd 11f &&\n \n@@ -3832,7 +3880,7 @@ test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rena\n # Commit B: node1/{leaf1,leaf2,leaf5}, node2/{leaf3,leaf4,leaf6}\n # Expected: node1/{leaf1,leaf2,leaf5,node2/{leaf3,leaf4,leaf6}}\n \n-test_expect_success '12a-setup: Moving one directory hierarchy into another' '\n+test_setup_12a () {\n \ttest_create_repo 12a &&\n \t(\n \t\tcd 12a &&\n@@ -3862,9 +3910,10 @@ test_expect_success '12a-setup: Moving one directory hierarchy into another' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '12a-check: Moving one directory hierarchy into another' '\n+test_expect_success '12a: Moving one directory hierarchy into another' '\n+\ttest_setup_12a &&\n \t(\n \t\tcd 12a &&\n \n@@ -3910,7 +3959,7 @@ test_expect_success '12a-check: Moving one directory hierarchy into another' '\n # To which, I can do no more than shrug my shoulders and say that\n # even simple rules give weird results when given weird inputs.\n \n-test_expect_success '12b-setup: Moving two directory hierarchies into each other' '\n+test_setup_12b () {\n \ttest_create_repo 12b &&\n \t(\n \t\tcd 12b &&\n@@ -3938,9 +3987,10 @@ test_expect_success '12b-setup: Moving two directory hierarchies into each other\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '12b-check: Moving two directory hierarchies into each other' '\n+test_expect_success '12b: Moving two directory hierarchies into each other' '\n+\ttest_setup_12b &&\n \t(\n \t\tcd 12b &&\n \n@@ -3976,7 +4026,7 @@ test_expect_success '12b-check: Moving two directory hierarchies into each other\n # NOTE: This is *exactly* like 12c, except that every path is modified on\n # each side of the merge.\n \n-test_expect_success '12c-setup: Moving one directory hierarchy into another w/ content merge' '\n+test_setup_12c () {\n \ttest_create_repo 12c &&\n \t(\n \t\tcd 12c &&\n@@ -4008,9 +4058,10 @@ test_expect_success '12c-setup: Moving one directory hierarchy into another w/ c\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '12c-check: Moving one directory hierarchy into another w/ content merge' '\n+test_expect_success '12c: Moving one directory hierarchy into another w/ content merge' '\n+\ttest_setup_12c &&\n \t(\n \t\tcd 12c &&\n \n@@ -4057,7 +4108,7 @@ test_expect_success '12c-check: Moving one directory hierarchy into another w/ c\n # Commit B: a/b/subdir/foo, a/b/bar\n # Expected: subdir/foo, bar\n \n-test_expect_success '12d-setup: Rename/merge subdir into the root, variant 1' '\n+test_setup_12d () {\n \ttest_create_repo 12d &&\n \t(\n \t\tcd 12d &&\n@@ -4078,9 +4129,10 @@ test_expect_success '12d-setup: Rename/merge subdir into the root, variant 1' '\n \t\tgit checkout B &&\n \t\ttest_commit a/b/bar\n \t)\n-'\n+}\n \n-test_expect_success '12d-check: Rename/merge subdir into the root, variant 1' '\n+test_expect_success '12d: Rename/merge subdir into the root, variant 1' '\n+\ttest_setup_12d &&\n \t(\n \t\tcd 12d &&\n \n@@ -4114,7 +4166,7 @@ test_expect_success '12d-check: Rename/merge subdir into the root, variant 1' '\n # Commit B: a/b/foo, a/b/bar\n # Expected: foo, bar\n \n-test_expect_success '12e-setup: Rename/merge subdir into the root, variant 2' '\n+test_setup_12e () {\n \ttest_create_repo 12e &&\n \t(\n \t\tcd 12e &&\n@@ -4135,9 +4187,10 @@ test_expect_success '12e-setup: Rename/merge subdir into the root, variant 2' '\n \t\tgit checkout B &&\n \t\ttest_commit a/b/bar\n \t)\n-'\n+}\n \n-test_expect_success '12e-check: Rename/merge subdir into the root, variant 2' '\n+test_expect_success '12e: Rename/merge subdir into the root, variant 2' '\n+\ttest_setup_12e &&\n \t(\n \t\tcd 12e &&\n \n@@ -4182,10 +4235,10 @@ test_expect_success '12e-check: Rename/merge subdir into the root, variant 2' '\n # Commit B: z/{b,c,d,e/f}\n # Expected: y/{b,c,d,e/f}, with notices/conflicts for both y/d and y/e/f\n \n-test_expect_success '13a-setup: messages for newly added files' '\n-\ttest_create_repo 13a &&\n+test_setup_13a () {\n+\ttest_create_repo 13a_$1 &&\n \t(\n-\t\tcd 13a &&\n+\t\tcd 13a_$1 &&\n \n \t\tmkdir z &&\n \t\techo b >z/b &&\n@@ -4211,11 +4264,12 @@ test_expect_success '13a-setup: messages for newly added files' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '13a-check(conflict): messages for newly added files' '\n+test_expect_success '13a(conflict): messages for newly added files' '\n+\ttest_setup_13a conflict &&\n \t(\n-\t\tcd 13a &&\n+\t\tcd 13a_conflict &&\n \n \t\tgit checkout A^0 &&\n \n@@ -4235,9 +4289,10 @@ test_expect_success '13a-check(conflict): messages for newly added files' '\n \t)\n '\n \n-test_expect_success '13a-check(info): messages for newly added files' '\n+test_expect_success '13a(info): messages for newly added files' '\n+\ttest_setup_13a info &&\n \t(\n-\t\tcd 13a &&\n+\t\tcd 13a_info &&\n \n \t\tgit reset --hard &&\n \t\tgit checkout A^0 &&\n@@ -4267,10 +4322,10 @@ test_expect_success '13a-check(info): messages for newly added files' '\n # Expected: y/{b,c,d_merged}, with two conflict messages for y/d,\n # one about content, and one about file location\n \n-test_expect_success '13b-setup: messages for transitive rename with conflicted content' '\n-\ttest_create_repo 13b &&\n+test_setup_13b () {\n+\ttest_create_repo 13b_$1 &&\n \t(\n-\t\tcd 13b &&\n+\t\tcd 13b_$1 &&\n \n \t\tmkdir x &&\n \t\tmkdir z &&\n@@ -4299,11 +4354,12 @@ test_expect_success '13b-setup: messages for transitive rename with conflicted c\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '13b-check(conflict): messages for transitive rename with conflicted content' '\n+test_expect_success '13b(conflict): messages for transitive rename with conflicted content' '\n+\ttest_setup_13b conflict &&\n \t(\n-\t\tcd 13b &&\n+\t\tcd 13b_conflict &&\n \n \t\tgit checkout A^0 &&\n \n@@ -4321,9 +4377,10 @@ test_expect_success '13b-check(conflict): messages for transitive rename with co\n \t)\n '\n \n-test_expect_success '13b-check(info): messages for transitive rename with conflicted content' '\n+test_expect_success '13b(info): messages for transitive rename with conflicted content' '\n+\ttest_setup_13b info &&\n \t(\n-\t\tcd 13b &&\n+\t\tcd 13b_info &&\n \n \t\tgit reset --hard &&\n \t\tgit checkout A^0 &&\n@@ -4352,10 +4409,10 @@ test_expect_success '13b-check(info): messages for transitive rename with confli\n # d and B had full knowledge, but that's a slippery slope as\n # shown in testcase 13d.\n \n-test_expect_success '13c-setup: messages for rename/rename(1to1) via transitive rename' '\n-\ttest_create_repo 13c &&\n+test_setup_13c () {\n+\ttest_create_repo 13c_$1 &&\n \t(\n-\t\tcd 13c &&\n+\t\tcd 13c_$1 &&\n \n \t\tmkdir x &&\n \t\tmkdir z &&\n@@ -4383,11 +4440,12 @@ test_expect_success '13c-setup: messages for rename/rename(1to1) via transitive\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '13c-check(conflict): messages for rename/rename(1to1) via transitive rename' '\n+test_expect_success '13c(conflict): messages for rename/rename(1to1) via transitive rename' '\n+\ttest_setup_13c conflict &&\n \t(\n-\t\tcd 13c &&\n+\t\tcd 13c_conflict &&\n \n \t\tgit checkout A^0 &&\n \n@@ -4404,9 +4462,10 @@ test_expect_success '13c-check(conflict): messages for rename/rename(1to1) via t\n \t)\n '\n \n-test_expect_success '13c-check(info): messages for rename/rename(1to1) via transitive rename' '\n+test_expect_success '13c(info): messages for rename/rename(1to1) via transitive rename' '\n+\ttest_setup_13c info &&\n \t(\n-\t\tcd 13c &&\n+\t\tcd 13c_info &&\n \n \t\tgit reset --hard &&\n \t\tgit checkout A^0 &&\n@@ -4438,10 +4497,10 @@ test_expect_success '13c-check(info): messages for rename/rename(1to1) via trans\n # * B renames a/y to c/y, and A renames c/->d/ => a/y -> d/y\n # No conflict in where a/y ends up, so put it in d/y.\n \n-test_expect_success '13d-setup: messages for rename/rename(1to1) via dual transitive rename' '\n-\ttest_create_repo 13d &&\n+test_setup_13d () {\n+\ttest_create_repo 13d_$1 &&\n \t(\n-\t\tcd 13d &&\n+\t\tcd 13d_$1 &&\n \n \t\tmkdir a &&\n \t\tmkdir b &&\n@@ -4470,11 +4529,12 @@ test_expect_success '13d-setup: messages for rename/rename(1to1) via dual transi\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '13d-check(conflict): messages for rename/rename(1to1) via dual transitive rename' '\n+test_expect_success '13d(conflict): messages for rename/rename(1to1) via dual transitive rename' '\n+\ttest_setup_13d conflict &&\n \t(\n-\t\tcd 13d &&\n+\t\tcd 13d_conflict &&\n \n \t\tgit checkout A^0 &&\n \n@@ -4494,9 +4554,10 @@ test_expect_success '13d-check(conflict): messages for rename/rename(1to1) via d\n \t)\n '\n \n-test_expect_success '13d-check(info): messages for rename/rename(1to1) via dual transitive rename' '\n+test_expect_success '13d(info): messages for rename/rename(1to1) via dual transitive rename' '\n+\ttest_setup_13d info &&\n \t(\n-\t\tcd 13d &&\n+\t\tcd 13d_info &&\n \n \t\tgit reset --hard &&\n \t\tgit checkout A^0 &&\n@@ -4562,7 +4623,7 @@ test_expect_success '13d-check(info): messages for rename/rename(1to1) via dual\n # in the outer merge for this special kind of setup, but it at\n # least avoids hitting a BUG().\n #\n-test_expect_success '13e-setup: directory rename detection in recursive case' '\n+test_setup_13e () {\n \ttest_create_repo 13e &&\n \t(\n \t\tcd 13e &&\n@@ -4607,9 +4668,10 @@ test_expect_success '13e-setup: directory rename detection in recursive case' '\n \t\ttest_tick &&\n \t\tgit commit -m \"D\"\n \t)\n-'\n+}\n \n-test_expect_success '13e-check: directory rename detection in recursive case' '\n+test_expect_success '13e: directory rename detection in recursive case' '\n+\ttest_setup_13e &&\n \t(\n \t\tcd 13e &&\n \ndiff --git a/t/t6046-merge-skip-unneeded-updates.sh b/t/t6046-merge-skip-unneeded-updates.sh\nindex 3a47623ed3..b7e4669832 100755\n--- a/t/t6046-merge-skip-unneeded-updates.sh\n+++ b/t/t6046-merge-skip-unneeded-updates.sh\n@@ -36,10 +36,10 @@ test_description=\"merge cases\"\n # Commit B: b_3\n # Expected: b_2\n \n-test_expect_success '1a-setup: Modify(A)/Modify(B), change on B subset of A' '\n-\ttest_create_repo 1a &&\n+test_setup_1a () {\n+\ttest_create_repo 1a_$1 &&\n \t(\n-\t\tcd 1a &&\n+\t\tcd 1a_$1 &&\n \n \t\ttest_write_lines 1 2 3 4 5 6 7 8 9 10 >b &&\n \t\tgit add b &&\n@@ -62,13 +62,12 @@ test_expect_success '1a-setup: Modify(A)/Modify(B), change on B subset of A' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '1a-check-L: Modify(A)/Modify(B), change on B subset of A' '\n-\ttest_when_finished \"git -C 1a reset --hard\" &&\n-\ttest_when_finished \"git -C 1a clean -fd\" &&\n+test_expect_success '1a-L: Modify(A)/Modify(B), change on B subset of A' '\n+\ttest_setup_1a L &&\n \t(\n-\t\tcd 1a &&\n+\t\tcd 1a_L &&\n \n \t\tgit checkout A^0 &&\n \n@@ -96,11 +95,10 @@ test_expect_success '1a-check-L: Modify(A)/Modify(B), change on B subset of A' '\n \t)\n '\n \n-test_expect_success '1a-check-R: Modify(A)/Modify(B), change on B subset of A' '\n-\ttest_when_finished \"git -C 1a reset --hard\" &&\n-\ttest_when_finished \"git -C 1a clean -fd\" &&\n+test_expect_success '1a-R: Modify(A)/Modify(B), change on B subset of A' '\n+\ttest_setup_1a R &&\n \t(\n-\t\tcd 1a &&\n+\t\tcd 1a_R &&\n \n \t\tgit checkout B^0 &&\n \n@@ -133,10 +131,10 @@ test_expect_success '1a-check-R: Modify(A)/Modify(B), change on B subset of A' '\n # Commit B: c_1\n # Expected: c_2\n \n-test_expect_success '2a-setup: Modify(A)/rename(B)' '\n-\ttest_create_repo 2a &&\n+test_setup_2a () {\n+\ttest_create_repo 2a_$1 &&\n \t(\n-\t\tcd 2a &&\n+\t\tcd 2a_$1 &&\n \n \t\ttest_seq 1 10 >b &&\n \t\tgit add b &&\n@@ -158,13 +156,12 @@ test_expect_success '2a-setup: Modify(A)/rename(B)' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '2a-check-L: Modify/rename, merge into modify side' '\n-\ttest_when_finished \"git -C 2a reset --hard\" &&\n-\ttest_when_finished \"git -C 2a clean -fd\" &&\n+test_expect_success '2a-L: Modify/rename, merge into modify side' '\n+\ttest_setup_2a L &&\n \t(\n-\t\tcd 2a &&\n+\t\tcd 2a_L &&\n \n \t\tgit checkout A^0 &&\n \n@@ -189,11 +186,10 @@ test_expect_success '2a-check-L: Modify/rename, merge into modify side' '\n \t)\n '\n \n-test_expect_success '2a-check-R: Modify/rename, merge into rename side' '\n-\ttest_when_finished \"git -C 2a reset --hard\" &&\n-\ttest_when_finished \"git -C 2a clean -fd\" &&\n+test_expect_success '2a-R: Modify/rename, merge into rename side' '\n+\ttest_setup_2a R &&\n \t(\n-\t\tcd 2a &&\n+\t\tcd 2a_R &&\n \n \t\tgit checkout B^0 &&\n \n@@ -224,10 +220,10 @@ test_expect_success '2a-check-R: Modify/rename, merge into rename side' '\n # Commit B: b_3\n # Expected: c_2\n \n-test_expect_success '2b-setup: Rename+Mod(A)/Mod(B), B mods subset of A' '\n-\ttest_create_repo 2b &&\n+test_setup_2b () {\n+\ttest_create_repo 2b_$1 &&\n \t(\n-\t\tcd 2b &&\n+\t\tcd 2b_$1 &&\n \n \t\ttest_write_lines 1 2 3 4 5 6 7 8 9 10 >b &&\n \t\tgit add b &&\n@@ -251,13 +247,12 @@ test_expect_success '2b-setup: Rename+Mod(A)/Mod(B), B mods subset of A' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '2b-check-L: Rename+Mod(A)/Mod(B), B mods subset of A' '\n-\ttest_when_finished \"git -C 2b reset --hard\" &&\n-\ttest_when_finished \"git -C 2b clean -fd\" &&\n+test_expect_success '2b-L: Rename+Mod(A)/Mod(B), B mods subset of A' '\n+\ttest_setup_2b L &&\n \t(\n-\t\tcd 2b &&\n+\t\tcd 2b_L &&\n \n \t\tgit checkout A^0 &&\n \n@@ -288,11 +283,10 @@ test_expect_success '2b-check-L: Rename+Mod(A)/Mod(B), B mods subset of A' '\n \t)\n '\n \n-test_expect_success '2b-check-R: Rename+Mod(A)/Mod(B), B mods subset of A' '\n-\ttest_when_finished \"git -C 2b reset --hard\" &&\n-\ttest_when_finished \"git -C 2b clean -fd\" &&\n+test_expect_success '2b-R: Rename+Mod(A)/Mod(B), B mods subset of A' '\n+\ttest_setup_2b R &&\n \t(\n-\t\tcd 2b &&\n+\t\tcd 2b_R &&\n \n \t\tgit checkout B^0 &&\n \n@@ -332,7 +326,7 @@ test_expect_success '2b-check-R: Rename+Mod(A)/Mod(B), B mods subset of A' '\n # skip the update, then we're in trouble. This test verifies we do\n # not make that particular mistake.\n \n-test_expect_success '2c-setup: Modify b & add c VS rename b->c' '\n+test_setup_2c () {\n \ttest_create_repo 2c &&\n \t(\n \t\tcd 2c &&\n@@ -358,9 +352,10 @@ test_expect_success '2c-setup: Modify b & add c VS rename b->c' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '2c-check: Modify b & add c VS rename b->c' '\n+test_expect_success '2c: Modify b & add c VS rename b->c' '\n+\ttest_setup_2c &&\n \t(\n \t\tcd 2c &&\n \n@@ -428,10 +423,10 @@ test_expect_success '2c-check: Modify b & add c VS rename b->c' '\n # Commit B: bq_1, bar/whatever\n # Expected: bar/{bq_2, whatever}\n \n-test_expect_success '3a-setup: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_create_repo 3a &&\n+test_setup_3a () {\n+\ttest_create_repo 3a_$1 &&\n \t(\n-\t\tcd 3a &&\n+\t\tcd 3a_$1 &&\n \n \t\tmkdir foo &&\n \t\ttest_seq 1 10 >bq &&\n@@ -456,13 +451,12 @@ test_expect_success '3a-setup: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '3a-check-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_when_finished \"git -C 3a reset --hard\" &&\n-\ttest_when_finished \"git -C 3a clean -fd\" &&\n+test_expect_success '3a-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n+\ttest_setup_3a L &&\n \t(\n-\t\tcd 3a &&\n+\t\tcd 3a_L &&\n \n \t\tgit checkout A^0 &&\n \n@@ -487,11 +481,10 @@ test_expect_success '3a-check-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n \t)\n '\n \n-test_expect_success '3a-check-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_when_finished \"git -C 3a reset --hard\" &&\n-\ttest_when_finished \"git -C 3a clean -fd\" &&\n+test_expect_success '3a-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n+\ttest_setup_3a R &&\n \t(\n-\t\tcd 3a &&\n+\t\tcd 3a_R &&\n \n \t\tgit checkout B^0 &&\n \n@@ -522,10 +515,10 @@ test_expect_success '3a-check-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n # Commit B: bq_2, bar/whatever\n # Expected: bar/{bq_2, whatever}\n \n-test_expect_success '3b-setup: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_create_repo 3b &&\n+test_setup_3b () {\n+\ttest_create_repo 3b_$1 &&\n \t(\n-\t\tcd 3b &&\n+\t\tcd 3b_$1 &&\n \n \t\tmkdir foo &&\n \t\ttest_seq 1 10 >bq &&\n@@ -550,13 +543,12 @@ test_expect_success '3b-setup: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '3b-check-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_when_finished \"git -C 3b reset --hard\" &&\n-\ttest_when_finished \"git -C 3b clean -fd\" &&\n+test_expect_success '3b-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n+\ttest_setup_3b L &&\n \t(\n-\t\tcd 3b &&\n+\t\tcd 3b_L &&\n \n \t\tgit checkout A^0 &&\n \n@@ -581,11 +573,10 @@ test_expect_success '3b-check-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n \t)\n '\n \n-test_expect_success '3b-check-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n-\ttest_when_finished \"git -C 3b reset --hard\" &&\n-\ttest_when_finished \"git -C 3b clean -fd\" &&\n+test_expect_success '3b-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n+\ttest_setup_3b R &&\n \t(\n-\t\tcd 3b &&\n+\t\tcd 3b_R &&\n \n \t\tgit checkout B^0 &&\n \n@@ -621,7 +612,7 @@ test_expect_success '3b-check-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '\n # Working copy: b_4\n # Expected: b_2 for merge, b_4 in working copy\n \n-test_expect_success '4a-setup: Change on A, change on B subset of A, dirty mods present' '\n+test_setup_4a () {\n \ttest_create_repo 4a &&\n \t(\n \t\tcd 4a &&\n@@ -647,7 +638,7 @@ test_expect_success '4a-setup: Change on A, change on B subset of A, dirty mods\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n # NOTE: For as long as we continue using unpack_trees() without index_only\n # set to true, it will error out on a case like this claiming the the locally\n@@ -655,9 +646,8 @@ test_expect_success '4a-setup: Change on A, change on B subset of A, dirty mods\n # correct requires doing the merge in-memory first, then realizing that no\n # updates to the file are necessary, and thus that we can just leave the path\n # alone.\n-test_expect_failure '4a-check: Change on A, change on B subset of A, dirty mods present' '\n-\ttest_when_finished \"git -C 4a reset --hard\" &&\n-\ttest_when_finished \"git -C 4a clean -fd\" &&\n+test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' '\n+\ttest_setup_4a &&\n \t(\n \t\tcd 4a &&\n \n@@ -695,7 +685,7 @@ test_expect_failure '4a-check: Change on A, change on B subset of A, dirty mods\n # Working copy: c_4\n # Expected: c_2\n \n-test_expect_success '4b-setup: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mods present' '\n+test_setup_4b () {\n \ttest_create_repo 4b &&\n \t(\n \t\tcd 4b &&\n@@ -722,11 +712,10 @@ test_expect_success '4b-setup: Rename+Mod(A)/Mod(B), change on B subset of A, di\n \t\ttest_tick &&\n \t\tgit commit -m \"B\"\n \t)\n-'\n+}\n \n-test_expect_success '4b-check: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mods present' '\n-\ttest_when_finished \"git -C 4b reset --hard\" &&\n-\ttest_when_finished \"git -C 4b clean -fd\" &&\n+test_expect_success '4b: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mods present' '\n+\ttest_setup_4b &&\n \t(\n \t\tcd 4b &&\n \n-- \ngitgitgadget\n"},{"id":"384800","messageId":"nycvar.QRO.7.76.6.1910250020220.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"CABPp-BFYWhyLqUktEnk6A7v6135k4TQHO20Wiy32mRQekt-3cg@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-24T22:22:47Z","receivedAt":"2019-10-24T22:23:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Tue, 22 Oct 2019, Elijah Newren wrote:\n\n> [...]\n> Yes, t6043 is pretty long code-wise, and still has over five dozen\n> tests after ditching the separate \"setup\" tests -- but all of those\n> tests still run in 3.6s on my box. [...]\n\n$ time sh t6043-*.sh --quiet\nnot ok 74 - 9g-check: Renamed directory that only contained immediate subdirs, immediate subdirs renamed # TODO known breakage\nnot ok 87 - 10e-check: Does git complain about untracked file that is not really in the way? # TODO known breakage\n# still have 2 known breakage(s)\n# passed all remaining 117 test(s)\n1..119\n\nreal 7m22.393s\nuser 0m52.115s\nsys 3m53.212s\n\nAnd this is not a slow box. So yes, those extra spawned processes? They\naccumulate. Spawning processes is what Linux was optimized for. You're\noptimizing for Linux.\n\nCiao,\nDscho\n"},{"id":"384818","messageId":"CABPp-BHBUKq73Ru3D9HKp6ABo8eQNmkSkz6MjA+4J2a6xxtWjA@mail.gmail.com","threadId":"52025","inReplyTo":"nycvar.QRO.7.76.6.1910250020220.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-10-25T00:12:33Z","receivedAt":"2019-10-25T00:12:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Oct 24, 2019 at 3:23 PM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Tue, 22 Oct 2019, Elijah Newren wrote:\n>\n> > [...]\n> > Yes, t6043 is pretty long code-wise, and still has over five dozen\n> > tests after ditching the separate \"setup\" tests -- but all of those\n> > tests still run in 3.6s on my box. [...]\n>\n> $ time sh t6043-*.sh --quiet\n> not ok 74 - 9g-check: Renamed directory that only contained immediate subdirs, immediate subdirs renamed # TODO known breakage\n> not ok 87 - 10e-check: Does git complain about untracked file that is not really in the way? # TODO known breakage\n> # still have 2 known breakage(s)\n> # passed all remaining 117 test(s)\n> 1..119\n>\n> real 7m22.393s\n> user 0m52.115s\n> sys 3m53.212s\n>\n> And this is not a slow box. So yes, those extra spawned processes? They\n> accumulate. Spawning processes is what Linux was optimized for. You're\n> optimizing for Linux.\n>\n> Ciao,\n> Dscho\n\nWow, I knew it'd be slower on other platforms but I certainly didn't\nexpect a factor of 122; you've made me eat my words about performance\nfor this case.\n\nStill, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\nsanity in the face of refactoring and rewriting -- and as mentioned\nbefore they have caught refactoring bugs in those areas that appear at\nfirst blush as \"overzealous\", and have done so multiple times. So,\nwhat's the alternative -- mark the tests as linux only? Do that but\nalso add a second copy that is trimmed down so other platforms can run\nthat one? Keep a local copy of all these tests? Jump on the\nour-testing-pyramid-is-inverted bandwagon when it materializes and\nprovides a way to write unit tests instead of just end-to-end tests\n(I'm game for that one...)? Start discussing crazy ideas like a\nbusybox-like shell, possibly with git extensions (a \"git shell\" if you\nwill, though I know the name is already taken), just for running the\ngit testsuite faster? Those alternatives all sound either unappealing\nor like very large projects that'll take a while to materialize (and\nwhich I certainly won't be spearheading; I've got too many big\nbackburnered projects already). This performance is clearly bad, but\ngutting the tests isn't tenable either.\n"},{"id":"384860","messageId":"nycvar.QRO.7.76.6.1910251527440.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"CABPp-BHBUKq73Ru3D9HKp6ABo8eQNmkSkz6MjA+4J2a6xxtWjA@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-25T13:30:37Z","receivedAt":"2019-10-25T13:31:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Thu, 24 Oct 2019, Elijah Newren wrote:\n\n> On Thu, Oct 24, 2019 at 3:23 PM Johannes Schindelin\n> wrote:\n> >\n> > On Tue, 22 Oct 2019, Elijah Newren wrote:\n> >\n> > > [...]\n> > > Yes, t6043 is pretty long code-wise, and still has over five dozen\n> > > tests after ditching the separate \"setup\" tests -- but all of those\n> > > tests still run in 3.6s on my box. [...]\n> >\n> > $ time sh t6043-*.sh --quiet\n> > not ok 74 - 9g-check: Renamed directory that only contained immediate subdirs, immediate subdirs renamed # TODO known breakage\n> > not ok 87 - 10e-check: Does git complain about untracked file that is not really in the way? # TODO known breakage\n> > # still have 2 known breakage(s)\n> > # passed all remaining 117 test(s)\n> > 1..119\n> >\n> > real 7m22.393s\n> > user 0m52.115s\n> > sys 3m53.212s\n> >\n> > And this is not a slow box. So yes, those extra spawned processes? They\n> > accumulate. Spawning processes is what Linux was optimized for. You're\n> > optimizing for Linux.\n> >\n> > Ciao,\n> > Dscho\n>\n> Wow, I knew it'd be slower on other platforms but I certainly didn't\n> expect a factor of 122; you've made me eat my words about performance\n> for this case.\n\nI am glad that the numbers are more convincing than I am ;-)\n\n> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n> sanity in the face of refactoring and rewriting -- and as mentioned\n> before they have caught refactoring bugs in those areas that appear at\n> first blush as \"overzealous\", and have done so multiple times. So,\n> what's the alternative -- mark the tests as linux only? Do that but\n> also add a second copy that is trimmed down so other platforms can run\n> that one? Keep a local copy of all these tests? Jump on the\n> our-testing-pyramid-is-inverted bandwagon when it materializes and\n> provides a way to write unit tests instead of just end-to-end tests\n> (I'm game for that one...)? Start discussing crazy ideas like a\n> busybox-like shell, possibly with git extensions (a \"git shell\" if you\n> will, though I know the name is already taken), just for running the\n> git testsuite faster? Those alternatives all sound either unappealing\n> or like very large projects that'll take a while to materialize (and\n> which I certainly won't be spearheading; I've got too many big\n> backburnered projects already). This performance is clearly bad, but\n> gutting the tests isn't tenable either.\n\nOne idea would be to try to guard those extra careful tests behind the\n`EXPENSIVE` prereq.\n\nI _do_ agree with you that it makes sense, in particular with the\nrecursive merge code, in particular because you are in the middle of\nheavy refactoring, to add really, really overzealous tests.\n\nThat really helps getting confident in the changes.\n\nI just don't see that we should pay the price (time-wise, and also\nelectricity-wise) of running those expensive tests even after the\nrefactoring, or for that matter, even for unrelated patches that are\nmore than 99.9% certain not to even touch the recursive merge.\n\nCiao,\nDscho\n"},{"id":"385029","messageId":"xmqqpnig8i0h.fsf@gitster-ct.c.googlers.com","threadId":"52025","inReplyTo":"nycvar.QRO.7.76.6.1910251527440.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-29T01:20:14Z","receivedAt":"2019-10-29T01:20:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n>> sanity in the face of refactoring and rewriting -- and as mentioned\n>> before they have caught refactoring bugs in those areas that appear at\n>> first blush as \"overzealous\", ...\n>\n> One idea would be to try to guard those extra careful tests behind the\n> `EXPENSIVE` prereq.\n\nYeah, I like that---I think it is perfectly in line with the spirit\nof EXPENSIVE, too.\n"},{"id":"385143","messageId":"CABPp-BHPRrbCs2qM8-zBaiL5pO3+9k5cbjtk3YVhy9_k1d7t+w@mail.gmail.com","threadId":"52025","inReplyTo":"xmqqpnig8i0h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-10-30T07:01:06Z","receivedAt":"2019-10-30T07:01:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Oct 28, 2019 at 6:20 PM Junio C Hamano wrote:\n>\n> Johannes Schindelin writes:\n>\n> >> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n> >> sanity in the face of refactoring and rewriting -- and as mentioned\n> >> before they have caught refactoring bugs in those areas that appear at\n> >> first blush as \"overzealous\", ...\n> >\n> > One idea would be to try to guard those extra careful tests behind the\n> > `EXPENSIVE` prereq.\n>\n> Yeah, I like that---I think it is perfectly in line with the spirit\n> of EXPENSIVE, too.\n\nOr perhaps EXPENSIVE_ON_WINDOWS, since it's actually pretty cheap on\nlinux and not that bad on Mac However, if we're going down that\nroute, perhaps t9001-send-email.sh could be wrapped in an EXPENSIVE\nprerequisite? That single test file takes an inordinate percentage of\noverall runtime. One one box with a few extra cpus, that test both\nstarts first and finishes last...and it's not far from that on even\nnormal boxes.\n"},{"id":"385189","messageId":"nycvar.QRO.7.76.6.1910302315000.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"CABPp-BHPRrbCs2qM8-zBaiL5pO3+9k5cbjtk3YVhy9_k1d7t+w@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-30T22:16:46Z","receivedAt":"2019-10-30T22:17:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 30 Oct 2019, Elijah Newren wrote:\n\n> On Mon, Oct 28, 2019 at 6:20 PM Junio C Hamano wrote:\n> >\n> > Johannes Schindelin writes:\n> >\n> > >> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n> > >> sanity in the face of refactoring and rewriting -- and as mentioned\n> > >> before they have caught refactoring bugs in those areas that appear at\n> > >> first blush as \"overzealous\", ...\n> > >\n> > > One idea would be to try to guard those extra careful tests behind the\n> > > `EXPENSIVE` prereq.\n> >\n> > Yeah, I like that---I think it is perfectly in line with the spirit\n> > of EXPENSIVE, too.\n>\n> Or perhaps EXPENSIVE_ON_WINDOWS, since it's actually pretty cheap on\n> linux and not that bad on Mac\n\nWhy the complexity? If you separate out the expensive tests (even if\nthey are only expensive in terms of run time on Windows), it will make\nthe regression tests so much more readable to the occasional reader\n(making them less expensive in terms of reading time...).\n\n> However, if we're going down that route, perhaps t9001-send-email.sh\n> could be wrapped in an EXPENSIVE prerequisite? That single test file\n> takes an inordinate percentage of overall runtime. One one box with a\n> few extra cpus, that test both starts first and finishes last...and\n> it's not far from that on even normal boxes.\n\nI would be okay with that.\n\nNo, let me try that again. I would be _totally_ okay with that.\n\nCiao,\nDscho\n"},{"id":"385191","messageId":"CABPp-BG1ir2kXkxMmNQ2btDKvpSWg9QwcUnoy_5teuNO=JhAFA@mail.gmail.com","threadId":"52025","inReplyTo":"nycvar.QRO.7.76.6.1910302315000.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-10-30T22:31:09Z","receivedAt":"2019-10-30T22:31:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Wed, Oct 30, 2019 at 3:17 PM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Wed, 30 Oct 2019, Elijah Newren wrote:\n>\n> > On Mon, Oct 28, 2019 at 6:20 PM Junio C Hamano wrote:\n> > >\n> > > Johannes Schindelin writes:\n> > >\n> > > >> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n> > > >> sanity in the face of refactoring and rewriting -- and as mentioned\n> > > >> before they have caught refactoring bugs in those areas that appear at\n> > > >> first blush as \"overzealous\", ...\n> > > >\n> > > > One idea would be to try to guard those extra careful tests behind the\n> > > > `EXPENSIVE` prereq.\n> > >\n> > > Yeah, I like that---I think it is perfectly in line with the spirit\n> > > of EXPENSIVE, too.\n> >\n> > Or perhaps EXPENSIVE_ON_WINDOWS, since it's actually pretty cheap on\n> > linux and not that bad on Mac\n>\n> Why the complexity? If you separate out the expensive tests (even if\n> they are only expensive in terms of run time on Windows), it will make\n> the regression tests so much more readable to the occasional reader\n> (making them less expensive in terms of reading time...).\n\nThe \"extra careful\" things you were complaining about with the new\ntest I was adding to t6043 was true of every single test in that\nfile...and likely much of t6036, t6042, and perhaps even t6046 (though\nthose have fewer tests than t6043). I have no clue where I'd even\nbegin to draw the line between them. If it's possible, it sounds\nextremely complex. Just using the EXPENSIVE_ON_WINDOWS prereq that\nalready exists would be easy and simple.\n\nOr did you mean you wanted me to duplicate every single test and\nattempt to trim down the duplicates somehow? That'd be a rather large\nundertaking that sounds rather unappealing on a few fronts, but maybe\nthat's what you had in mind?\n\n> > However, if we're going down that route, perhaps t9001-send-email.sh\n> > could be wrapped in an EXPENSIVE prerequisite? That single test file\n> > takes an inordinate percentage of overall runtime. One one box with a\n> > few extra cpus, that test both starts first and finishes last...and\n> > it's not far from that on even normal boxes.\n>\n> I would be okay with that.\n>\n> No, let me try that again. I would be _totally_ okay with that.\n\nOoh, sweet, sounds like I should propose it.\n"},{"id":"385204","messageId":"nycvar.QRO.7.76.6.1910310927540.46@tvgsbejvaqbjf.bet","threadId":"52025","inReplyTo":"CABPp-BG1ir2kXkxMmNQ2btDKvpSWg9QwcUnoy_5teuNO=JhAFA@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-31T08:28:52Z","receivedAt":"2019-10-31T08:29:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 30 Oct 2019, Elijah Newren wrote:\n\n> On Wed, Oct 30, 2019 at 3:17 PM Johannes Schindelin\n> wrote:\n> >\n> > On Wed, 30 Oct 2019, Elijah Newren wrote:\n> >\n> > > On Mon, Oct 28, 2019 at 6:20 PM Junio C Hamano wrote:\n> > > >\n> > > > Johannes Schindelin writes:\n> > > >\n> > > > >> Still, I rely pretty heavily on t6036, t6042, t6043, and t6046 for\n> > > > >> sanity in the face of refactoring and rewriting -- and as mentioned\n> > > > >> before they have caught refactoring bugs in those areas that appear at\n> > > > >> first blush as \"overzealous\", ...\n> > > > >\n> > > > > One idea would be to try to guard those extra careful tests behind the\n> > > > > `EXPENSIVE` prereq.\n> > > >\n> > > > Yeah, I like that---I think it is perfectly in line with the spirit\n> > > > of EXPENSIVE, too.\n> > >\n> > > Or perhaps EXPENSIVE_ON_WINDOWS, since it's actually pretty cheap on\n> > > linux and not that bad on Mac\n> >\n> > Why the complexity? If you separate out the expensive tests (even if\n> > they are only expensive in terms of run time on Windows), it will make\n> > the regression tests so much more readable to the occasional reader\n> > (making them less expensive in terms of reading time...).\n>\n> The \"extra careful\" things you were complaining about with the new\n> test I was adding to t6043 was true of every single test in that\n> file...and likely much of t6036, t6042, and perhaps even t6046 (though\n> those have fewer tests than t6043). I have no clue where I'd even\n> begin to draw the line between them. If it's possible, it sounds\n> extremely complex. Just using the EXPENSIVE_ON_WINDOWS prereq that\n> already exists would be easy and simple.\n>\n> Or did you mean you wanted me to duplicate every single test and\n> attempt to trim down the duplicates somehow? That'd be a rather large\n> undertaking that sounds rather unappealing on a few fronts, but maybe\n> that's what you had in mind?\n\nMy suggestion was in reply to your question, and not intended for\nalready-existing tests. That would indeed require a lot of work, and I\nam not sure that it would be worth it.\n\nThe suggestion was intended for future patches.\n\nCiao,\nDscho\n"}]}