git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v5 00/34] Add directory rename detection to git

From
Elijah Newren <newren@gmail.com>
Date
Jan 3, 2018, 21:02 UTC
Message-ID
<CABPp-BF0Tby4+Va_MV-j3kC-phV8D84PJboBhE-AR2pXQMwL2g@mail.gmail.com>
In-Reply-To
<7ecea1b4-d713-7298-1697-ae25532e26e0@kdbg.org>
On Wed, Jan 3, 2018 at 2:57 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> I tested the series on Windows recently. It requires the patch below.
> I don't know whether this is indicating some portability issues of grep
> (^ being used in the middle of a RE instead of at the very beginning) or
> just a quirk in my setup.

Thanks for testing it out. What version of Windows were you running on? With cygwin or without? I tested previously on cygwin (I think on Windows Server 2012??) and got all the tests passing there, eventually[1]. I'm not sure I can find access to any other Windows systems, but I'd be happy to take a look if I can.

[1] https://public-inbox.org/git/CABPp-BEJ6-mrY0OCz1WWetrtG_iehKzOdCUOn_PUuKVywaU9Zw@mail.gmail.com/

The need to backslash escape a caret for a literal match when it appears in the middle of the string makes sense. Thanks for sending along the patch. Would you prefer I squashed it into the series (still sitting in 'pu'), or keep your patch separate? I'm fine with either, I'm just unsure the protocol here.

Show 9 quoted lines
> But it still does not pass the test suite because the system does not
> like file names such as y/c~HEAD:
>
> ++ grep 'Refusing to lose dirty file at z/c' out
> Refusing to lose dirty file at z/c
> ++ grep -q stuff x/b y/a y/c y/c~HEAD z/c
> grep: y/c: Invalid request code
> error: last command exited with $?=2
> not ok 94 - 11d-check: Avoid losing not-uptodate with rename + D/F conflict
This is exceptionally odd.  The actual line from the testsuite was
  grep -q stuff */*
which suggests your shell is both doing the pathname expansion and
treating the resulting filename not as a string but as something to be
interpreted that happens to have some kind of special
characters/commands, and then choking on the result.  Super weird.  I
could probably work around this by just running
  grep -q stuff z/c

I think I had the asterisks in there because I was thinking in terms of directory rename detection potentially moving the file, but that's probably just overkill. Does the test pass for you with that change? (If so, there are also two similar tests that I'd need to make similar changes to.)

However, although that might fix this particular case, it suggests some fragility of the tests and filenames for whatever system you happen to be using. merge-recursive.c's unique_path has created filenames with tilde's in them for many years, it may just be that I'm the first to use the resulting file in combination with grep to ensure the contents are as we expect. There may be other issues lurking (even if not yet appearing in the testsuite) for your system when dealing with merge conflicts.

Thanks, Elijah

Show 37 quoted lines
>
> ---- 8< ----
> From: Johannes Sixt <j6t@kdbg.org>
> Date: Fri, 22 Dec 2017 09:33:13 +0100
> Subject: [PATCH] fixup directory rename tests
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  t/t6043-merge-rename-directories.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
> index f0af66b8a9..b8cd428341 100755
> --- a/t/t6043-merge-rename-directories.sh
> +++ b/t/t6043-merge-rename-directories.sh
> @@ -2940,8 +2940,8 @@ test_expect_success '10b-check: Overwrite untracked with dir rename + delete' '
>                 echo contents >y/e &&
>
>                 test_must_fail git merge -s recursive B^0 >out 2>err &&
> -               test_i18ngrep "CONFLICT (rename/delete).*Version B^0 of y/d left in tree at y/d~B^0" out &&
> -               test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B^0 instead" out &&
> +               test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out &&
> +               test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out &&
>
>                 test 3 -eq $(git ls-files -s | wc -l) &&
>                 test 2 -eq $(git ls-files -u | wc -l) &&
> @@ -3010,7 +3010,7 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2)
>
>                 test_must_fail git merge -s recursive B^0 >out 2>err &&
>                 test_i18ngrep "CONFLICT (rename/rename)" out &&
> -               test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B^0 instead" out &&
> +               test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out &&
>
>                 test 6 -eq $(git ls-files -s | wc -l) &&
>                 test 3 -eq $(git ls-files -u | wc -l) &&
> --
> 2.14.2.808.g3bc32f2729
Previous: Johannes SixtNext: Johannes Sixt
Message 45 of 46 in “Add directory rename detection to git”
  1. 00/34 Add directory rename detection to gitElijah Newren, Dec 28, 2017
  2. 03/34 merge-recursive: add explanation for src_entry and dst_entryElijah Newren, Dec 28, 2017
  3. 04/34 directory rename detection: basic testcasesElijah Newren, Dec 28, 2017
  4. SZEDER GáborJan 3, 2018
  5. Elijah NewrenJan 3, 2018
  6. 07/34 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Dec 28, 2017
  7. 08/34 directory rename detection: files/directories in the way of some renamesElijah Newren, Dec 28, 2017
  8. 14/34 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Dec 28, 2017
  9. 10/34 directory rename detection: more involved edge/corner testcasesElijah Newren, Dec 28, 2017
  10. 11/34 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Dec 28, 2017
  11. 19/34 merge-recursive: split out code for determining diff_filepairsElijah Newren, Dec 28, 2017
  12. 22/34 merge-recursive: add get_directory_renames()Elijah Newren, Dec 28, 2017
  13. 28/34 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Dec 28, 2017
  14. 29/34 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Dec 28, 2017
  15. 32/34 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Dec 28, 2017
  16. 30/34 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Dec 28, 2017
  17. 31/34 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Dec 28, 2017
  18. 34/34 merge-recursive: ensure we write updates for directory-renamed fileElijah Newren, Dec 28, 2017
  19. 33/34 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Dec 28, 2017
  20. 23/34 merge-recursive: check for directory level conflictsElijah Newren, Dec 28, 2017
  21. 26/34 merge-recursive: check for file level conflicts then get new nameElijah Newren, Dec 28, 2017
  22. 25/34 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Dec 28, 2017
  23. 21/34 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Dec 28, 2017
  24. 20/34 merge-recursive: add a new hashmap for storing directory renamesElijah Newren, Dec 28, 2017
  25. 24/34 merge-recursive: add a new hashmap for storing file collisionsElijah Newren, Dec 28, 2017
  26. 27/34 merge-recursive: when comparing files, don't include treesElijah Newren, Dec 28, 2017
  27. 12/34 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Dec 28, 2017
  28. 18/34 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Dec 28, 2017
  29. 16/34 merge-recursive: introduce new functions to handle rename logicElijah Newren, Dec 28, 2017
  30. 17/34 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Dec 28, 2017
  31. 13/34 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Dec 28, 2017
  32. SZEDER GáborJan 4, 2018
  33. Elijah NewrenJan 4, 2018
  34. SZEDER GáborJan 5, 2018
  35. Thomas GummererJan 5, 2018
  36. Elijah NewrenJan 5, 2018
  37. 15/34 merge-recursive: move the get_renames() functionElijah Newren, Dec 28, 2017
  38. 09/34 directory rename detection: testcases checking which side did the renameElijah Newren, Dec 28, 2017
  39. 02/34 merge-recursive: fix logic ordering issueElijah Newren, Dec 28, 2017
  40. 06/34 directory rename detection: testcases to avoid taking detection too farElijah Newren, Dec 28, 2017
  41. 01/34 Tighten and correct a few testcases for merging and cherry-pickingElijah Newren, Dec 28, 2017
  42. 05/34 directory rename detection: directory splitting testcasesElijah Newren, Dec 28, 2017
  43. Elijah NewrenJan 3, 2018
  44. Johannes SixtJan 3, 2018
  45. Elijah NewrenJan 3, 2018
  46. Johannes SixtJan 3, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.