Hi Ævar, On Fri, 5 Jan 2018, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jan 05 2018, Johannes Schindelin jotted: > > > [...] > > > > In short: the Unix shell script t3070 manages to write what it thinks is a > > file called 'foo*', but Git only sees 'foo'. > > > > I tried to address this problem with this patch: > > ...I don't see any particular value in trying to do these full roundtrip > tests on platforms like Windows. Perhaps we should just do these on a > whitelist of POSIX systems for now, and leave expanding that list to > some future step. I don't think so. Windows is already handled as a second-class citizen, as if nobody developed on it. As a consequence, only very few of the gazillions of Windows developers... develop Git. We could worsify the situation, of course, but why? Shouldn't we at least pretend to try the opposite? > > -- snip -- > > diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh > > index f606f91acbb..51dcb675e7b 100755 > > --- a/t/t3070-wildmatch.sh > > +++ b/t/t3070-wildmatch.sh > > @@ -4,6 +4,14 @@ test_description='wildmatch tests' > > > > . ./test-lib.sh > > > > +if test_have_prereq !MINGW && touch -- 'a*b' 2>/dev/null > > +then > > + test_set_prereq FILENAMESWITHSTARS > > +else > > + say 'Your filesystem does not allow stars in filenames.' > > +fi > > +rm -f 'a*b' > > + > > create_test_file() { > > file=$1 > > > > @@ -28,6 +36,17 @@ create_test_file() { > > */) > > return 1 > > ;; > > + # On Windows, stars are not allowed in filenames. Git for Windows' > > + # Bash, however, is based on Cygwin which plays funny names with a > > + # private Unicode page to emulate stars in filenames. Meaning that > > + # the shell script will "succeed" to write the file, but Git will > > + # not see it. Nor any other, regular Windows process. > > + *\**|*\[*) > > + if ! test_have_prereq FILENAMESWITHSTARS > > + then > > + return 1 > > + fi > > + ;; > > # On Windows, \ in paths is silently converted to /, which > > # would result in the "touch" below working, but the test > > # itself failing. See 6fd1106aa4 ("t3700: Skip a test with > > -- snap -- > > > > This gets us further. But not by much: > > Okey, that's very weird. So you can: > > touch "./*"; echo $? > > And it'll return 0 but then the file won't exist? Almost. The file exists, but it won't have the name '*'. It will have as name a Unicode character that is in a private page, not standardized by the Unicode specification. > How about this: > > touch "./*" && test -e "./*"; echo $? Would return 0. Why? Because *you are still in a Unix shell script, so the Cygwin cuteness still applies*. > The reason this latest version stopped creating files with "\" in them > unless under BSLASHPSPEC is because Cygwin would silently translate it, > so it would create the file but it would actually mean something the > tests didn't expect. I understand that. And I would wish that the test would be designed in a more cross-platform-aware mindset. > For anything else, such as stars not being allowed in filenames I was > expecting that "touch" command to return an error, but if that's not the > case maybe we need the "test -e" as well, unless I'm missing something > here. This is one of the many bad consequences of Git relying so much on Unix shell scripting. Despite what many, many, many Git developers think: shell scripting is not portable. Cygwin does a good job of pretending that it is, and MSYS2 exacerbates that notion, but it comes back to haunt you right here and right now. The `touch` invocation will *report success*, but it will have done something different than you wanted. It's like the Thinking: Fast and Slow. > > fatal: Invalid path '\[ab]': No such file or directory > > > > You see, any path starting with a backslash *is* an absolute path on > > Windows. It is relative to the current drive. > > Right, which I was trying to avoid by not actually creating \[ab], but > "./\[ab]", is that the same filename on Windows? Whoops. I managed to copy-paste the *wrong* command's error message. Sorry about that. The correct one: $ git --glob-pathspecs ls-files -z -- '\[ab]' fatal: \[ab]: '\[ab]' is outside repository Note how it is Git reporting that you asked for a path that is outside? That's because it thinks you are referring to C:\[ab] (if your current directory is on the C: drive). And it would be correct to complain on Windows: the `\[ab]` parameter refers to an absolute path. > > This affects *quite* a few of the test cases you added. > > > > And even if I just comment all of those out, I run into the next problem > > where you try to create a file whose name consists of a single space, > > which is also illegal on Windows. > > Okey, but ditto above about touch not catching it, does: > > touch "./ "; echo $? > > Not return an error? Then how about: > > touch "./ " && test -e "./ "; echo $? Again: as long as you stay within the bounds of the Unix shell script (did I point out enough yet how non-portable this design is? Even Subversion knew better than to implement parts of its operations as Unix shell scripts. I mean, for PoCing, okay, but for production code?) you fall prey to Cygwin's emulation of POSIX-y filenames. As soon as you leave that bubble (e.g. by calling git.exe), you're not going to see those illegal file names, but the ones with the unprintable Unicode characters. > > These woes demonstrate one problem with the approach of overzealously > > testing *everything*[...] > > I think the rest of this gets into topics I've covered above. I.e. that > this increased test coverage has caught bugs. That's all good and dandy, but what about regressions? I know how much I will curse in your vague direction when I encounter the next wildmatch-related bug in, say, half a year and have to wade through the jungle of unintuitive tests in t3070. Can't we do a lot better than this? Shouldn't it be a lot more obvious what the heck went wrong when running t3070 with -i -v -x? Ciao, Johannes