[clang][cas] Fix caching builds with -fdiagnostics-absolute-paths #12033
Conversation
Add a fallback path to diagnostic handling for -fdiagnostics-absolute-paths to try the real filesystem, since the include tree filesystem will not be able to resolve the path. This only affects diagonstic formatting, not the cached result, so it is safe. rdar://166408406
|
@swift-ci please test llvm |
| // filesytem since the text diagnostic output is not captured directly. | ||
| auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); | ||
| auto FS = llvm::vfs::getRealFileSystem(); | ||
| if (FS->getRealPath(Filename, TmpFilename) == std::error_code()) |
There was a problem hiding this comment.
Can you explain again with llvm::sys::path::make_absolute is not sufficient here?
There was a problem hiding this comment.
My goal for this PR is to match the behaviour without caching, which uses getCanonicalName -> getRealPath.
As to why it's done this way in the first place: I dug through the history of changes and review comments on this code and it seems like the motivation is to eliminate . and .. components in paths, which seems to be motivated by two issues: 1) some editors apparently were opening a separate tab for (a/b/../c and a/c), and 2) there are some cases where the extra dots make the absolute path really long. The code on Windows handles this directly, but it's not correct on other systems.
As a separate change, I am thinking of proposing that this behaviour change to make it instead do basically what Windows is doing (make absolute and remove dots), but add an additional check on non-Windows system that the two paths resolve to the same file. That will hopefully be cheaper in most cases, since stat() will be cheaper than realpath(). If the paths really are different files then we can fallback to only removing single dots. Even if there is some possibility it leaves some of the file paths "too long" in rare situations with symlinks, it seems like a better tradeoff to me overall.
|
@swift-ci please test llvm |
Add a fallback path to diagnostic handling for -fdiagnostics-absolute-paths to try the real filesystem, since the include tree filesystem will not be able to resolve the path. This only affects diagonstic formatting, not the cached result, so it is safe.
rdar://166408406