Properly support for Windows long file paths through Unicode Win32 API calls. #2410
Conversation
digit-google
force-pushed
the
win32-paths
branch
2 times, most recently
from
4c25208
to
e4085e5
Compare
|
I tried this fork, but I still get the ninja error "Filename longer than 260 characters" on my jenkins when compiling my cmake project. |
|
I've also tested this in an attempt to fix the issue when building a gn project, but it doesn't work. Same question as @JRStolle. |
|
During early testing this fix seems to work for us (although other tooling outside of ninja may eventually fail too). However we did bump into an issue where many dependency targets were incorrectly re-run after the first invocation. I added a fix on our side to resolve: discord@c5996dc |
digit-google
force-pushed
the
win32-paths
branch
from
e4085e5
to
c06fa2c
Compare
|
Thank you @markh-discord , I rebased and integrated a similar fix for the weird GetFullPathnameW behavior. Let me know if this works for you. @jhasse, would you prefer to split this PR into a series of smaller ones for easier review? |
digit-google
force-pushed
the
win32-paths
branch
2 times, most recently
from
947ed1e
to
8d3ed14
Compare
|
As I've said in a previous PR: I'm actually very much against this change. To convince me I would like to know exactly in which cases the current solution (UTF-8 and largeFileSupport in manifest) does NOT work? |
For me, it is the Qt build using clang. The Qt build has very long paths in the clang-cl output. The output seems to be handled using the max path length restrictions for the maximum buffer size of a path. |
|
I think we need exact reproduction steps that confirm that the problem is with Ninja and not some other tooling. |
UTF8ToWin32Unicode() converts an UTF-8 string into the corresponding Windows UCS-2 one, and will be used to call Unicode Win32 API directly, in order to properly support long file paths everywhere. Win32UnicodeToUTF8() does the opposite, and will generally be used to display such native Win32 paths in error messages.
Add an intermediate DiskInterface derived class named SystemDiskInterface that performs real disk i/o without any intermediate caching. RealDiskInterface is now a derived class of SystemDiskInterface that adds a caching layer on Win32 only. Keeping the same name reduces the number of changes in this commit to the strict minimum. NullDiskInterface is provided to simplify custom DiskInterface implementations used in tests, in particular because new DiskInterface methods are going to be added in future commits, and adding the same method overrides NullDiskInterface will lower the changes to do in those implementations.
Ensure Win32 Unicode APIs are called when trying to perform real disk i/o. This is necessary to properly support long file paths on Windows. For example, FindFirstFileExW() used in RealDiskInterface has not MAC_PATH restriction, though FindFirstFileExA() still does, even when long path support is enabled on the host machine. Note that generally speaking, it is unknown whether Windows CRT functions, such as fopen(), rename(), _unlink(), always properly support long file paths (it may very well depend on the version of MSVCRT linked to the executable), so it is better to err on the side of caution and always try to use the wide-char versions of these functions, when available, or to fall back to Unicode Win32 API functions otherwise. This applies here and the commits following this one in the same pull request.
Use CreateFileW() to ensure proper long path support on Win32.
+ Allow it to compile with cross-toolchains like Mingw.
Make the BuildLog class take a DiskInterface reference in its constructor, to ensure that all i/o operations use the same interface. + Adjust call sites accordingly. For tests, always use a SystemDiskInterface instead of the VirtualFileSystem instance when the latter is available, as this is exactly what the previous code was doing.
digit-google
force-pushed
the
win32-paths
branch
from
8d3ed14
to
03284e1
Compare
This is an attempt to properly support Windows long file paths by only using Win32 Unicode API calls, and getting rid of the ANSI ones (some of which are still plagued by MAX_PATH limitations, even when long paths support is enabled on the machine).
This is achieved in the following ways (see individual commits for details):
Add
UTF8ToWin32Unicode()andWin32UnicodeToUTF8()functions to util.hFor simplicity, introduce SystemDiskInterface and NullDiskInterface, and make RealDiskInterface a derived class of SystemDiskInterface that adds a caching layer.
(this opens the door to other caching implementations on Posix).
Augment the
DiskInterfaceAPI to addRenameFile()andOpenFile()methods to ... rename a file (just likerename() / _rename()), and open an stdio FILE instance (just likefopen()).Modify the DiskInterface implementation to only use Unicode Win32 APIs.
Modify sources that rely on direct calls to
fopen(),unlink(),rename()to use a DiskInterface instance. This requires injecting such an instance in the constructors of BuildLog and DepsLog btw.Modify other misc Win32-specific sources to use Win32 Unicode API calls directly (e.g. subprocess-win32.cc)
NOTE: VirtualFileSystem::OpenFile() only supports read access for now. Unlike its real SystemDiskInterface implementation, it doesn't perform \r\n to \n conversion on Windows (do we really want this?). That is why many tests (e.g. build_log_test.cc) do not use it, but rely on a real DiskInterface instance. This can be fixed by another PR after this one.
This should fix issue #1900 once and for all (crossing fingers here :-))