ninja: Build fails when using case sensitive folders on Windows

Ninja is unable to find files in folders using upper case letters when case sensitive folders are enabled.

Issue can be reproduced by setting up the following folder structure and using the build.ninja file below. test_ninja.zip

`-- test_ninja
     |-- bar
     |    `-- hello.c
     |-- Foo
     |    `-- hello.c

test_ninja must be a case sensitive folder. fsutil.exe file setCaseSensitiveInfo .\test_ninja\ enable

# build.ninja
cc     = gcc

rule compile
  command = $cc -c $in -o $out

rule link
  command = $cc $in -o $out

#Example 1: Using incorrect casing for 'foo'. Ninja correctly reports an error
#build hello.o: compile foo/hello.c
#Output => ninja: error: 'foo/hello.c', needed by 'hello.o', missing and no known rule to make it

#Example 2: Using correct casing for 'Foo'. Ninja reports the same error, but it should be able to find it.
#build hello.o: compile Foo/hello.c
#Output => ninja: error: 'Foo/hello.c', needed by 'hello.o', missing and no known rule to make it

#Example 3: Using incorrect casing for 'Bar'. Ninja reports the expected error "No such file or directory"
#build hello.o: compile Bar/hello.c
#[1/2] gcc -c Bar/hello.c -o hello.o
#FAILED: hello.o
#gcc -c Bar/hello.c -o hello.o
#gcc: error: Bar/hello.c: No such file or directory
#gcc: fatal error: no input files
#compilation terminated.
#ninja: build stopped: subcommand failed.

#Example 4: Using correct casing for 'bar'. Everything is lower case and so it works
#build hello.o: compile bar/hello.c

build hello: link hello.o

default hello

I would expect Example 2 above to work. Changing the following function RealDiskInterface::Stat in disk_interface.cc to not perform a transform to lower case makes it work as expected.

TimeStamp RealDiskInterface::Stat(const string& path, string* err) const {
....

**Removing the two lines below fixes the issue**
  //transform(dir.begin(), dir.end(), dir.begin(), ::tolower);
  //transform(base.begin(), base.end(), base.begin(), ::tolower);

....
}

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 16 (7 by maintainers)

Most upvoted comments

Makes sense, thanks for the explanation. I’ve merged the PR 😃

jhasse on Jan 20, 2022

Thanks, I am glad I was helpful in analyzing and progressing this PR. 😄

Bagira80 on Jan 20, 2022

Had this PR been merged already, both Visual Studio versions would have this fix already.

The PR has been created after v1.10.2 was released, so no, it would not have this fix already.

Sorry, my bad. I misread 2020 as 2021.

[…] but are there some further tests that I can run, so the PR can be accepted?

Is there something that can be done to further progress this PR?

Unfortunately there are no easy-to-run scripts to check for performance regressions (see #1733). Creating such a script or manually checking would be the next step. Tests where the DirCache matters on Windows in particular.

To be honest, I do not really understand why you expect this PR to have any impact on the performance, in particular with the DirCache.

If I read it correctly, the changes of this PR are equivalent to the following, even more simple diff:

--- a/src/disk_interface.cc
+++ b/src/disk_interface.cc
@@ -180,13 +180,14 @@ TimeStamp RealDiskInterface::Stat(const string& path, string* err) const {
     dir = path;
   }

+  string dir_origcase = dir;
   transform(dir.begin(), dir.end(), dir.begin(), ::tolower);
   transform(base.begin(), base.end(), base.begin(), ::tolower);

   Cache::iterator ci = cache_.find(dir);
   if (ci == cache_.end()) {
     ci = cache_.insert(make_pair(dir, DirCache())).first;
-    if (!StatAllFilesInDir(dir.empty() ? "." : dir, &ci->second, err)) {
+    if (!StatAllFilesInDir(dir_origcase.empty() ? "." : dir_origcase, &ci->second, err)) {
       cache_.erase(ci);
       return -1;
     }

From this it should be obvious that the only difference is that StatAllFilesInDir() (and internally FindFirstFileExA) now receives a directory name which is no longer guaranteed to be all lower-case.

As a result there will not be any difference if the file-system is not case-sensitive. (Even when looking for the wrongly cased filename, FindFirstFileExA will find the file; as before.)
On the other hand, for case-sensitive file-systems it now makes a difference: Without the changes from this PR the file cannot be found if its real case is not all lower-case. While with this PR it now can be found.

Still, the changes from this PR should not have any impact on performance for none case-sensitive file-systems.
The impact on performance for case-sensitive file-systems is noticeable only insofar as the build-process now no longer will fail.

Bagira80 on Jan 19, 2022

@jhasse what are the next steps then? It sounds like both @Bagira80 and I have tested it with good results, but are there some further tests that I can run, so the PR can be accepted?

MartinBP on Oct 19, 2021