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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 03.01.2018 um 22:02 schrieb Elijah Newren:
On Wed, Jan 3, 2018 at 2:57 AM, Johannes Sixt <j6t@xxxxxxxx> 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@xxxxxxxxxxxxxx/

I have an ancient MinGW setup, where I build "vanilla" Git (not exactly vanilla, but also not with the many patches that Git for Windows carries).

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.

Please squash into the relevant commits so that the series is bisectable if the need arises.

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?

I can test on Monday at the earliest. If it's that easy to fix my failures, I'd appreciate to go this route. But otherwise, I can deal with the situation, so we don't need to complicate things just to please my exotic setup.

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

I can't recall having seen issues around tildas in file names, either. It may be a new situation. I'll investigate.

-- Hannes



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux