Avoid realpath drive resolution to fix symlink-related test failures on Windows

[tristanlabelle]

Fixes the SourceKit/Indexing/indexstore_multifile.swift test and several others (mostly in sourcekit) which would perform realpath path substitution and expand substitute paths on Windows, resulting in failing string comparisons. This is the swift repo counterpart of https://reviews.llvm.org/D154130 , where we eliminated the use of realpath resolution in lit.py since this prevented us from using substitute drives on Windows to avoid running into MAX_PATH issues. The new symbolic link resolution logic attempts to use real_path on Windows, but will fall back to make_absolute if that would change the drive on which the path is based. Some symbolic link tests must thus be skipped because we will not honor the symlink resolution if the source tree is on a substitute drive.

[compnerd]

@swift-ci please test

[bnbarham]

Thanks Tristan! Just a couple changes left :)

[compnerd]

@swift-ci please test

[bnbarham]

LGTM, thanks again Tristan! The failing test is because you fixed a bug by the looks of it. I had:

// TODO: This should be B/b.swift, but there's currently a bug with multiple overlays.
//       See rdar://90578880 or https://github.com/llvm/llvm-project/issues/53306.
//       Replace with CHECK-FIXED when that's fixed.

In that test case, but that issue is actually already fixed. So seems like the real filesystem was still being used somewhere here and that's now fixed? You should just be able to remove the CHECK line and replace CHECK-FIXED with CHECK.

[tristanlabelle]

I pushed a fix to the substitute drive check failing with the old Python version installed in the CI.

The SourceKit/CursorInfo/cursor_swift_overlay.swift failure on mac could be because related changes are happening? https://reviews.llvm.org/D123398

[compnerd]

@swift-ci please test

[bnbarham]

I pushed a fix to the substitute drive check failing with the old Python version installed in the CI.

The SourceKit/CursorInfo/cursor_swift_overlay.swift failure on mac could be because related changes are happening? https://reviews.llvm.org/D123398

See my comment above:

LGTM, thanks again Tristan! The failing test is because you fixed a bug by the looks of it. I had:

// TODO: This should be B/b.swift, but there's currently a bug with multiple overlays.
//       See rdar://90578880 or https://github.com/llvm/llvm-project/issues/53306.
//       Replace with CHECK-FIXED when that's fixed.

In that test case, but that issue is actually already fixed. So seems like the real filesystem was still being used somewhere here and that's now fixed? You should just be able to remove the CHECK line and replace CHECK-FIXED with CHECK.

The change you linked has been in forever (and is the one I mention there). We must have been using the real filesystem somewhere.

[tristanlabelle]

@bnbarham Ok this failure turns out to be tricky :/. With my changes, it works on macOS but fails on Windows.

My understanding is that there are layered FileSystems: a RedirectingFileSystem over an OverlayFileSystem over the RealFileSystem. Some code calls getRealPath on the outer filesystem, which does the redirection and chains things down. However, because the final RealFileSystem resolves to a different drive, we ignore the whole getRealPath chain and fallback to makeAbsolute.

The better fix would be to have the realpath-on-same-drive logic be either in RealFileSystem, or just above it, in a decorating filesystem. However this expands the scope of this PR a lot and involves an LLVM change first... :/

00 sourcekitdInProc!llvm::sys::fs::real_path
01 sourcekitdInProc!`anonymous namespace'::RealFileSystem::getRealPath
02 sourcekitdInProc!llvm::vfs::OverlayFileSystem::getRealPath
03 sourcekitdInProc!llvm::vfs::OverlayFileSystem::getRealPath
04 sourcekitdInProc!llvm::SmallVectorImpl<char>::{dtor}
05 sourcekitdInProc!llvm::SmallVector<char,256>::{dtor}
06 sourcekitdInProc!llvm::vfs::RedirectingFileSystem::getRealPath
07 sourcekitdInProc!clang::FileManager::getCanonicalName
08 sourcekitdInProc!clang::FileManager::getCanonicalName
09 sourcekitdInProc!clang::ModuleMap::canonicalizeModuleMapPath
0a sourcekitdInProc!clang::HeaderSearch::getCachedModuleFileNameImpl
0b sourcekitdInProc!clang::HeaderSearch::getCachedModuleFileName
0c sourcekitdInProc!clang::HeaderSearch::getCachedModuleFileName
0d sourcekitdInProc!selectModuleSource
0e sourcekitdInProc!clang::CompilerInstance::findOrCompileModuleAndReadAST
0f sourcekitdInProc!clang::CompilerInstance::loadModule
10 sourcekitdInProc!`swift::ClangImporter::Implementation::loadModuleClang'::`2'::<lambda_1>::operator()
11 sourcekitdInProc!swift::ClangImporter::Implementation::loadModuleClang
12 sourcekitdInProc!swift::ClangImporter::Implementation::loadModule
13 sourcekitdInProc!swift::ClangImporter::loadModule

[bnbarham]

The better fix would be to have the realpath-on-same-drive logic be either in RealFileSystem, or just above it, in a decorating filesystem. However this expands the scope of this PR a lot and involves an LLVM change first... :/

Hmmm... Is there any way we can realpath and then map back to the substituted drive?

To give some context on the use of the VFS overlay here (actually a RedirectingFileSystem, names are confusing 😅) - consider the case where there's a separate "build arena" for indexing, where rather than a full build it only contains the necessary files required to be able to build the AST for a given file. So eg. if A depends on B and we want to build the AST of a file in A, that build arena would contain the minimal module of B. This arena is the "overlay" on top of the full build.

If a file is somehow missing in that overlay but is in the full build, this test is checking that cursor info gives back the path in the full build, rather than the overlay (where it doesn't exist).