Restore toleration of missing to-be-deleted files on Windows

[bradking]

Since #1892 we use DeleteFile without checking if it failed due to a path missing. Fix the GetFileAttributes logic to check for multiple variants of "file not found" as we do in StatSingleFile. Then fall back to the original remove() logic that already checks for ENOENT on failure. Add the check after DeleteFile too.

Issue: #1886

[bradking]

I run nightly testing of CMake with a nightly build of ninja. After #1892, I started noticing failures with output from ninja:

Cleaning... ninja: error: remove(...): The system cannot find the path specified.

[bradking]

Cc: @jhasse @mathstuf

[jhasse]

Thx!

You've also added the A suffix to the winapi functions, any reason for that?

[bradking]

The A suffix on winapi functions is already used elsewhere in the same source file. I added it for consistency. If we add unicode support in the future, they should all be converted to W suffixes together.

[jhasse]

Then fall back to the original remove() logic that already checks for ENOENT on failure.

Is this really needed? Couldn't this patch be broken down to just adding || win_err == ERROR_PATH_NOT_FOUND?

[bradking]

I was trying to minimize the net change from before #1892 rather than a current net change, since the older code has been deployed and working for years. Try

git diff 2d7f7e55~1 HEAD -- src/disk_interface.cc

If you want to switch to DeleteFileA then I can re-work the change.

[bradking]

@jhasse something is wrong with the macOS builder in CI.

[jhasse]

I'm not a fan of the A/W suffix. When we switch to UNICODE this will just create an unnecessary huge diff. std::wstring has a c_str() function, too, so that line could eventually be untouched. That's why I didn't add it in #1892.

The reason to catch an acceptable error directly after GetFileAttributes was to avoid the subsequent calls.

I also deliberately used DeleteFile instead of remove, because it has better Unicode support. Otherwise we would have to #ifdefon Windows anyway and use _wremove and _wcserror instead.

@jhasse something is wrong with the macOS builder in CI.

Seems to be working now.

[jhasse]

I kept the remove(%s) wording as people might have grep'ed for it.

Can you keep the DeleteFile code path?

[bradking]

Thanks for explaining your reasoning. I added a corresponding comment about remove().

[jhasse]

Thank you for the PR!

[jhasse]

I've just discovered that Windows now supports UTF-8 and we might not need UNICODE after all ...
#1915

If that really works, using remove sounds like the better idea. Sorry for the bad timing. If you find another issue with DeleteFile, feel free to to create a PR with your idea of using remove on Windows, too.