From: Johannes Sixt Date: Wed, 03 Jan 2018 22:23:27 GMT Subject: Re: [PATCH v5 00/34] Add directory rename detection to git Message-ID: In-Reply-To: Am 03.01.2018 um 22:02 schrieb Elijah Newren: > On Wed, Jan 3, 2018 at 2:57 AM, Johannes Sixt 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/ 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