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

[Elijah Newren <newren@gmail.com>]

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.

> 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









>
> ---- 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