[llvm] Use the VFS to make path absolute

[jansvoboda11]

For the redirecting VFS, the 'overlay-relative' option controls whether external paths should be appended to the overlay directory. This didn't always work as expected: when the overlay file path itself was relative, its absolute path was decided by the real FS, not the underlying VFS, and the resulting external path didn't exist in the underlying VFS. This PR fixes this issue.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Jan Svoboda (jansvoboda11)

Changes

For the redirecting VFS, the 'overlay-relative' option controls whether external paths should be appended to the overlay directory. This didn't always work as expected: when the overlay file path itself was relative, its absolute path was decided by the real FS, not the underlying VFS, and the resulting external path didn't exist in the underlying VFS. This PR fixes this issue.

---

Full diff: https://github.com/llvm/llvm-project/pull/161271.diff

2 Files Affected:

• (modified) llvm/lib/Support/VirtualFileSystem.cpp (+1-1)
• (modified) llvm/unittests/Support/VirtualFileSystemTest.cpp (+23)

diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 7ff62d43ba205..64a6607603ff0 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -2204,7 +2204,7 @@ RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
     //  FS->OverlayFileDir => /<absolute_path_to>/dummy.cache/vfs
     //
     SmallString<256> OverlayAbsDir = sys::path::parent_path(YAMLFilePath);
-    std::error_code EC = llvm::sys::fs::make_absolute(OverlayAbsDir);
+    std::error_code EC = ExternalFS->makeAbsolute(OverlayAbsDir);
     assert(!EC && "Overlay dir final path must be absolute");
     (void)EC;
     FS->setOverlayFileDir(OverlayAbsDir);
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index d47a4ee986778..f52f25f93744d 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1941,6 +1941,29 @@ TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, RelativeFileDirWithOverlayRelativeSetting) {
+  auto Lower = makeIntrusiveRefCnt<DummyFileSystem>();
+  Lower->addDirectory("//root/foo/bar");
+  Lower->addRegularFile("//root/foo/bar/a");
+  Lower->setCurrentWorkingDirectory("//root/foo");
+  IntrusiveRefCntPtr<vfs::FileSystem> FS =
+      getFromYAMLString("{\n"
+                        "  'case-sensitive': false,\n"
+                        "  'overlay-relative': true,\n"
+                        "  'roots': [\n"
+                        "    { 'name': '//root/foo/bar/b', 'type': 'file',\n"
+                        "      'external-contents': 'a'\n"
+                        "    }\n"
+                        "  ]\n"
+                        "}",
+                        Lower, "bar/overlay");
+
+  ASSERT_NE(FS.get(), nullptr);
+  ErrorOr<vfs::Status> S = FS->status("//root/foo/bar/b");
+  ASSERT_FALSE(S.getError());
+  EXPECT_EQ("//root/foo/bar/a", S->getName());
+}
+
 TEST_F(VFSFromYAMLTest, RootRelativeToOverlayDirTest) {
   auto Lower = makeIntrusiveRefCnt<DummyFileSystem>();
   Lower->addDirectory("//root/foo/bar");

[cachemeifyoucan]

LGTM

[jansvoboda11]

This PR was failing in the Windows CI.

The reason is that we used ExternalFS->makeAbsolute() and llvm::sys::path::append() that don't handle path separators right on Windows.

I fixed this by switching to calling FS->makeAbsolute(StringRef, SmallVectorImpl<char> &), which is explained in more detail here: https://reviews.llvm.org/D71092 or here:

llvm-project/llvm/lib/Support/VirtualFileSystem.cpp

Lines 1400 to 1437 in 2802ab6

	std::error_code
	RedirectingFileSystem::makeAbsolute(StringRef WorkingDir,
	SmallVectorImpl<char> &Path) const {
	// We can't use sys::fs::make_absolute because that assumes the path style
	// is native and there is no way to override that. Since we know WorkingDir
	// is absolute, we can use it to determine which style we actually have and
	// append Path ourselves.
	if (!WorkingDir.empty() &&
	!sys::path::is_absolute(WorkingDir, sys::path::Style::posix) &&
	!sys::path::is_absolute(WorkingDir,
	sys::path::Style::windows_backslash)) {
	return std::error_code();
	}
	sys::path::Style style = sys::path::Style::windows_backslash;
	if (sys::path::is_absolute(WorkingDir, sys::path::Style::posix)) {
	style = sys::path::Style::posix;
	} else {
	// Distinguish between windows_backslash and windows_slash; getExistingStyle
	// returns posix for a path with windows_slash.
	if (getExistingStyle(WorkingDir) != sys::path::Style::windows_backslash)
	style = sys::path::Style::windows_slash;
	}
	std::string Result = std::string(WorkingDir);
	StringRef Dir(Result);
	if (!Dir.ends_with(sys::path::get_separator(style))) {
	Result += sys::path::get_separator(style);
	}
	// backslashes '\' are legit path charactors under POSIX. Windows APIs
	// like CreateFile accepts forward slashes '/' as path
	// separator (even when mixed with backslashes). Therefore,
	// `Path` should be directly appended to `WorkingDir` without converting
	// path separator.
	Result.append(Path.data(), Path.size());
	Path.assign(Result.begin(), Result.end());
	return {};
	}

.