Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Endless CMake configuration re-running loop on Windows #1535

Open
steinlec opened this issue Mar 6, 2019 · 8 comments
Open

Endless CMake configuration re-running loop on Windows #1535

steinlec opened this issue Mar 6, 2019 · 8 comments

Comments

@steinlec
Copy link

steinlec commented Mar 6, 2019

I have actually the problem in my project that Ninja runs an endless loop re-configuring the project. I use CMake 3.11.3 in combination with Ninja 1.9.0 and the Microsoft Compiler on Windows. I have now analyzed the problem and come up with two aspects:

  1. The first aspect is that CMake adds some dependency files to the Ninja file build.ninja which can not be processed by Ninja:

`
#############################################

Re-run CMake if any of its inputs changed.

build build.ninja: RERUN_CMAKE | <dependency files>
#############################################

A missing CMake input file is not an error.

build <dependency files>
`

As Ninja can not process these dependency files, it reruns the configuration of CMake forever, because nobody takes care of these files. That is not really good in my opinion.

I have had a more precise look to Ninja what happens, and find the following code in file disk_interface.cc:

`
TimeStamp StatSingleFile(const string& path, string* err) {
WIN32_FILE_ATTRIBUTE_DATA attrs;
if (!GetFileAttributesExA(path.c_str(), GetFileExInfoStandard, &attrs)) {
DWORD win_err = GetLastError();
if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND)
return 0;
err = "GetFileAttributesEx(" + path + "): " + GetLastErrorString();
return -1;
}
return TimeStampFromFileTime(attrs.ftLastWriteTime);
}
...
bool StatAllFilesInDir(const string& dir, map<string, TimeStamp>
stamps,
string* err) {
// FindExInfoBasic is 30% faster than FindExInfoStandard.
static bool can_use_basic_info = IsWindows7OrLater();
// This is not in earlier SDKs.
const FINDEX_INFO_LEVELS kFindExInfoBasic =
static_cast<FINDEX_INFO_LEVELS>(1);
FINDEX_INFO_LEVELS level =
can_use_basic_info ? kFindExInfoBasic : FindExInfoStandard;
WIN32_FIND_DATAA ffd;
HANDLE find_handle = FindFirstFileExA((dir + "\*").c_str(), level, &ffd,
FindExSearchNameMatch, NULL, 0);

if (find_handle == INVALID_HANDLE_VALUE) {
DWORD win_err = GetLastError();
if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND)
return true;
*err = "FindFirstFileExA(" + dir + "): " + GetLastErrorString();
return false;
}
`

So both functions can not find the dependency file, but feature no special treatment. That is why Ninja runs into the endless loop.

  1. In the second aspect, I have searched for the reason why CMake can access a file that Ninja can't. And I found it in the WIN32 API, and especially the MAX_PATH limitation. This means precisely that the original file has less than MAX_PATH characters. CMake computes the location of this file relative to the current directory of Ninja. This location is then also below MAX_PATH. However giving this relative file location to a Windows API function breaks the Windows API function, because the concatenation of the relative file location to the current directory leads to a string with more than MAX_PATH characters. This is told by the error message of the function GetFullPathName. The function FindFirstFile or GetFileAttributes tells that it can not find the path. This knowledge I have gained from a separate test program. In this test program, I added also a mechanism to compute the absolute path to the file based on the relative location with respect to this knowledge, and it works. The mechanism itself is quite simple. It just walks directory per directory through the relative location and computes the intermediary absolute path. So as the relative location contains initially some "..", the path gets shorter before going deeper. Of course, this mechanism is not fast but operational. I think it can be optimized by walking not directory by directory, but compute the amount of directories in combination with the current path, so that the number of needed characters are below MAX_PATH.
    So even if there is a check for MAX_PATH in the source code of Ninja in disk_interface:

TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { METRIC_RECORD("node stat"); #ifdef _WIN32 // MSDN: "Naming Files, Paths, and Namespaces" // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx if (!path.empty() && path[0] != '\\' && path.size() > MAX_PATH) { ostringstream err_stream; err_stream << "Stat(" << path << "): Filename longer than " << MAX_PATH << " characters"; *err = err_stream.str(); return -1; }

it would be now clear that this check does not meet my circumstance, because the relative file location needs less than MAX_PATH characters.

So I would be very pleased, if someone can help me with these two aspects?

@jhasse
Copy link
Collaborator

jhasse commented Apr 11, 2019

@steinlec
Copy link
Author

As I understand #1514, the problem is, that the absolute path is longer than 254 characters. Consequentially there are just the well-known options, which are stated in the ticket. In the ticket here, the problem is that the relative path is longer than MAX_PATH, but the absolute path is below MAX_PATH. So the failure of the WIN32 API could be coped with by simply computing the absolute path before using the WIN32 API. Even if such an approach will of course not completely solve the problem, it would improve the situation. The bigger problem is that inaccessible files lead to an endless loop. Surely enabling long path will hide all problems stated in this ticket, but the algorithmic endless loop still exists. Just its probability of occurence is reduced. And by the way, i have no chance to introduce Windows 10 to solve this problem.

@steinlec
Copy link
Author

In the meantime I developed a workaround that works under my constraints. It simply uses some other Windows functions to make the filenames absolute before using them. So I can now cope with relative path that gets bigger than MAX_PATH under the constraint that their absolute representation is shorter than MAX_PATH.

However this is obviously not a solution to the fact that mtime is set to -1 for filenames bigger than MAX_PATH. This circumstance will still trigger 100 times CMake reconfigure without any luck for success.

Nevertheless the following patch will solve my actual problem on Windows:

MaxPath-Ninja.patch.log

@foundry-markf
Copy link

I'm curious what the maintainers of ninja current thoughts are to this issue, and the patch provided?

I encountered it entirely independently, with a current working directory and a relative path to a header, both individually of a sensible length, but when concatenated, exceeded MAX_PATH, so that FindFirstFileExA returned an invalid handle and set the ERROR_PATH_NOT_FOUND error. ninja -d explain told me of header files that didn't exist (yet they did), marking them dirty, such that every invocation of ninja caused some source files to rebuild that hadn't been changed.

When normalised, the overall path length was less than MAX_PATH, so that worked in my circumstances.

This isn't a blocker for me, as I can relocate where files are to avoid such long combined paths, but it was a gotcha that I spent a fair amount of time tracking down, so others might get stuck on it too.

Thanks.

@jhasse
Copy link
Collaborator

jhasse commented Oct 14, 2021

I'm curious what the maintainers of ninja current thoughts are to this issue, and the patch provided?

  1. Microsoft should lift the 260 character path limit for *A functions, too. See Should support filename longer than 260 characters #1900.
  2. Using -1 as a magic number is bad and the general error handling could be improved to fix this endless loop.

Btw: Has someone checked if this bug still appears after #1964?

@foundry-markf
Copy link

I can't comment on the original problem, but the one I described in #1535 (comment), I was debugging the Ninja master branch (cloned and built yesterday) and saw -d explain messages of the kind

output [path] of phony edge with no inputs doesn't exist

@steinlec
Copy link
Author

The patch simply tries to evaluate an absolute path before using the methods of Microsoft. So as a relative path commonly looks like C:\a\b\c\d......\e\f, it is modified to C:\a\e\f before using it. I still need this patch, but i am actually not using head code, because i can not relocate the files, and i am not able to convince Microsoft to lift their limit

@jcelerier
Copy link

jcelerier commented Oct 6, 2022

also seeing this here (ninja 1.11.1, cmake 3.24.2, visual studio 2022)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants