Skip to content

Commit 6871462

Browse files
JDevlieghereJDevlieghere
committed
Revert "[VirtualFileSystem] Support virtual working directory in the RedirectingFS"
This reverts the original commit and the follow up: Revert "[VirtualFileSystem] Support virtual working directory in the RedirectingFS" Revert "[test] Update YAML mapping in VirtualFileSystemTest" llvm-svn: 374935 (cherry picked from commit 409b4b5)
1 parent bcb6fcf commit 6871462

File tree

3 files changed

+16
-217
lines changed

3 files changed

+16
-217
lines changed

‎llvm/include/llvm/Support/VirtualFileSystem.h

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -647,19 +647,9 @@ class RedirectingFileSystem : public vfs::FileSystem {
647647
friend class VFSFromYamlDirIterImpl;
648648
friend class RedirectingFileSystemParser;
649649

650-
bool shouldUseExternalFS() const {
651-
return ExternalFSValidWD && IsFallthrough;
652-
}
653-
654650
/// The root(s) of the virtual file system.
655651
std::vector<std::unique_ptr<Entry>> Roots;
656652

657-
/// The current working directory of the file system.
658-
std::string WorkingDirectory;
659-
660-
/// Whether the current working directory is valid for the external FS.
661-
bool ExternalFSValidWD = false;
662-
663653
/// The file system to use for external references.
664654
IntrusiveRefCntPtr<FileSystem> ExternalFS;
665655

@@ -699,7 +689,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
699689
true;
700690
#endif
701691

702-
RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
692+
RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS)
693+
: ExternalFS(std::move(ExternalFS)) {}
703694

704695
/// Looks up the path <tt>[Start, End)</tt> in \p From, possibly
705696
/// recursing into the contents of \p From if it is a directory.

‎llvm/lib/Support/VirtualFileSystem.cpp

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -986,16 +986,6 @@ std::error_code InMemoryFileSystem::isLocal(const Twine &Path, bool &Result) {
986986
// RedirectingFileSystem implementation
987987
//===-----------------------------------------------------------------------===/
988988

989-
RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
990-
: ExternalFS(std::move(FS)) {
991-
if (ExternalFS)
992-
if (auto ExternalWorkingDirectory =
993-
ExternalFS->getCurrentWorkingDirectory()) {
994-
WorkingDirectory = *ExternalWorkingDirectory;
995-
ExternalFSValidWD = true;
996-
}
997-
}
998-
999989
// FIXME: reuse implementation common with OverlayFSDirIterImpl as these
1000990
// iterators are conceptually similar.
1001991
class llvm::vfs::VFSFromYamlDirIterImpl
@@ -1042,27 +1032,12 @@ class llvm::vfs::VFSFromYamlDirIterImpl
10421032

10431033
llvm::ErrorOr<std::string>
10441034
RedirectingFileSystem::getCurrentWorkingDirectory() const {
1045-
return WorkingDirectory;
1035+
return ExternalFS->getCurrentWorkingDirectory();
10461036
}
10471037

10481038
std::error_code
10491039
RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
1050-
// Don't change the working directory if the path doesn't exist.
1051-
if (!exists(Path))
1052-
return errc::no_such_file_or_directory;
1053-
1054-
// Always change the external FS but ignore its result.
1055-
if (ExternalFS) {
1056-
auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
1057-
ExternalFSValidWD = !static_cast<bool>(EC);
1058-
}
1059-
1060-
SmallString<128> AbsolutePath;
1061-
Path.toVector(AbsolutePath);
1062-
if (std::error_code EC = makeAbsolute(AbsolutePath))
1063-
return EC;
1064-
WorkingDirectory = AbsolutePath.str();
1065-
return {};
1040+
return ExternalFS->setCurrentWorkingDirectory(Path);
10661041
}
10671042

10681043
std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
@@ -1075,7 +1050,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
10751050
ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Dir);
10761051
if (!E) {
10771052
EC = E.getError();
1078-
if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory)
1053+
if (IsFallthrough && EC == errc::no_such_file_or_directory)
10791054
return ExternalFS->dir_begin(Dir, EC);
10801055
return {};
10811056
}
@@ -1093,7 +1068,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
10931068
auto *D = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(*E);
10941069
return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
10951070
Dir, D->contents_begin(), D->contents_end(),
1096-
/*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC));
1071+
/*IterateExternalFS=*/IsFallthrough, *ExternalFS, EC));
10971072
}
10981073

10991074
void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1595,7 +1570,7 @@ RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
15951570
RedirectingFileSystemParser P(Stream);
15961571

15971572
std::unique_ptr<RedirectingFileSystem> FS(
1598-
new RedirectingFileSystem(ExternalFS));
1573+
new RedirectingFileSystem(std::move(ExternalFS)));
15991574

16001575
if (!YAMLFilePath.empty()) {
16011576
// Use the YAML path from -ivfsoverlay to compute the dir to be prefixed
@@ -1724,7 +1699,7 @@ ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path,
17241699
ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
17251700
ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
17261701
if (!Result) {
1727-
if (shouldUseExternalFS() &&
1702+
if (IsFallthrough &&
17281703
Result.getError() == llvm::errc::no_such_file_or_directory) {
17291704
return ExternalFS->status(Path);
17301705
}
@@ -1762,7 +1737,7 @@ ErrorOr<std::unique_ptr<File>>
17621737
RedirectingFileSystem::openFileForRead(const Twine &Path) {
17631738
ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
17641739
if (!E) {
1765-
if (shouldUseExternalFS() &
1740+
if (IsFallthrough &&
17661741
E.getError() == llvm::errc::no_such_file_or_directory) {
17671742
return ExternalFS->openFileForRead(Path);
17681743
}
@@ -1793,7 +1768,7 @@ RedirectingFileSystem::getRealPath(const Twine &Path,
17931768
SmallVectorImpl<char> &Output) const {
17941769
ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
17951770
if (!Result) {
1796-
if (shouldUseExternalFS() &&
1771+
if (IsFallthrough &&
17971772
Result.getError() == llvm::errc::no_such_file_or_directory) {
17981773
return ExternalFS->getRealPath(Path, Output);
17991774
}
@@ -1806,8 +1781,8 @@ RedirectingFileSystem::getRealPath(const Twine &Path,
18061781
}
18071782
// Even if there is a directory entry, fall back to ExternalFS if allowed,
18081783
// because directories don't have a single external contents path.
1809-
return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output)
1810-
: llvm::errc::invalid_argument;
1784+
return IsFallthrough ? ExternalFS->getRealPath(Path, Output)
1785+
: llvm::errc::invalid_argument;
18111786
}
18121787

18131788
IntrusiveRefCntPtr<FileSystem>

‎llvm/unittests/Support/VirtualFileSystemTest.cpp

Lines changed: 4 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ struct DummyFile : public vfs::File {
4141
class DummyFileSystem : public vfs::FileSystem {
4242
int FSID; // used to produce UniqueIDs
4343
int FileID; // used to produce UniqueIDs
44-
std::string WorkingDirectory;
4544
std::map<std::string, vfs::Status> FilesAndDirs;
46-
typedef std::map<std::string, vfs::Status>::const_iterator const_iterator;
4745

4846
static int getNextFSID() {
4947
static int Count = 0;
@@ -54,7 +52,8 @@ class DummyFileSystem : public vfs::FileSystem {
5452
DummyFileSystem() : FSID(getNextFSID()), FileID(0) {}
5553

5654
ErrorOr<vfs::Status> status(const Twine &Path) override {
57-
auto I = findEntry(Path);
55+
std::map<std::string, vfs::Status>::iterator I =
56+
FilesAndDirs.find(Path.str());
5857
if (I == FilesAndDirs.end())
5958
return make_error_code(llvm::errc::no_such_file_or_directory);
6059
return I->second;
@@ -67,16 +66,15 @@ class DummyFileSystem : public vfs::FileSystem {
6766
return S.getError();
6867
}
6968
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
70-
return WorkingDirectory;
69+
return std::string();
7170
}
7271
std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
73-
WorkingDirectory = Path.str();
7472
return std::error_code();
7573
}
7674
// Map any symlink to "/symlink".
7775
std::error_code getRealPath(const Twine &Path,
7876
SmallVectorImpl<char> &Output) const override {
79-
auto I = findEntry(Path);
77+
auto I = FilesAndDirs.find(Path.str());
8078
if (I == FilesAndDirs.end())
8179
return make_error_code(llvm::errc::no_such_file_or_directory);
8280
if (I->second.isSymlink()) {
@@ -138,14 +136,6 @@ class DummyFileSystem : public vfs::FileSystem {
138136
FilesAndDirs[Path] = Status;
139137
}
140138

141-
const_iterator findEntry(const Twine &Path) const {
142-
SmallString<128> P;
143-
Path.toVector(P);
144-
std::error_code EC = makeAbsolute(P);
145-
assert(!EC);
146-
return FilesAndDirs.find(P.str());
147-
}
148-
149139
void addRegularFile(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) {
150140
vfs::Status S(Path, UniqueID(FSID, FileID++),
151141
std::chrono::system_clock::now(), 0, 0, 1024,
@@ -168,12 +158,6 @@ class DummyFileSystem : public vfs::FileSystem {
168158
}
169159
};
170160

171-
class ErrorDummyFileSystem : public DummyFileSystem {
172-
std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
173-
return llvm::errc::no_such_file_or_directory;
174-
}
175-
};
176-
177161
/// Replace back-slashes by front-slashes.
178162
std::string getPosixPath(std::string S) {
179163
SmallString<128> Result;
@@ -2010,154 +1994,3 @@ TEST_F(VFSFromYAMLTest, GetRealPath) {
20101994
EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
20111995
errc::no_such_file_or_directory);
20121996
}
2013-
2014-
TEST_F(VFSFromYAMLTest, WorkingDirectory) {
2015-
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
2016-
Lower->addDirectory("//root/");
2017-
Lower->addDirectory("//root/foo");
2018-
Lower->addRegularFile("//root/foo/a");
2019-
Lower->addRegularFile("//root/foo/b");
2020-
IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
2021-
"{ 'use-external-names': false,\n"
2022-
" 'roots': [\n"
2023-
"{\n"
2024-
" 'type': 'directory',\n"
2025-
" 'name': '//root/bar',\n"
2026-
" 'contents': [ {\n"
2027-
" 'type': 'file',\n"
2028-
" 'name': 'a',\n"
2029-
" 'external-contents': '//root/foo/a'\n"
2030-
" }\n"
2031-
" ]\n"
2032-
"}\n"
2033-
"]\n"
2034-
"}",
2035-
Lower);
2036-
ASSERT_TRUE(FS.get() != nullptr);
2037-
std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar/");
2038-
ASSERT_FALSE(EC);
2039-
2040-
llvm::ErrorOr<std::string> WorkingDir = FS->getCurrentWorkingDirectory();
2041-
ASSERT_TRUE(WorkingDir);
2042-
EXPECT_EQ(*WorkingDir, "//root/bar/");
2043-
2044-
llvm::ErrorOr<vfs::Status> Status = FS->status("./a");
2045-
ASSERT_FALSE(Status.getError());
2046-
EXPECT_TRUE(Status->isStatusKnown());
2047-
EXPECT_FALSE(Status->isDirectory());
2048-
EXPECT_TRUE(Status->isRegularFile());
2049-
EXPECT_FALSE(Status->isSymlink());
2050-
EXPECT_FALSE(Status->isOther());
2051-
EXPECT_TRUE(Status->exists());
2052-
2053-
EC = FS->setCurrentWorkingDirectory("bogus");
2054-
ASSERT_TRUE(EC);
2055-
WorkingDir = FS->getCurrentWorkingDirectory();
2056-
ASSERT_TRUE(WorkingDir);
2057-
EXPECT_EQ(*WorkingDir, "//root/bar/");
2058-
2059-
EC = FS->setCurrentWorkingDirectory("//root/");
2060-
ASSERT_FALSE(EC);
2061-
WorkingDir = FS->getCurrentWorkingDirectory();
2062-
ASSERT_TRUE(WorkingDir);
2063-
EXPECT_EQ(*WorkingDir, "//root/");
2064-
2065-
EC = FS->setCurrentWorkingDirectory("bar/");
2066-
ASSERT_FALSE(EC);
2067-
WorkingDir = FS->getCurrentWorkingDirectory();
2068-
ASSERT_TRUE(WorkingDir);
2069-
EXPECT_EQ(*WorkingDir, "//root/bar/");
2070-
}
2071-
2072-
TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
2073-
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
2074-
Lower->addDirectory("//root/");
2075-
Lower->addDirectory("//root/foo");
2076-
Lower->addRegularFile("//root/foo/a");
2077-
Lower->addRegularFile("//root/foo/b");
2078-
Lower->addRegularFile("//root/c");
2079-
IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
2080-
"{ 'use-external-names': false,\n"
2081-
" 'roots': [\n"
2082-
"{\n"
2083-
" 'type': 'directory',\n"
2084-
" 'name': '//root/bar',\n"
2085-
" 'contents': [ {\n"
2086-
" 'type': 'file',\n"
2087-
" 'name': 'a',\n"
2088-
" 'external-contents': '//root/foo/a'\n"
2089-
" }\n"
2090-
" ]\n"
2091-
"}\n"
2092-
"]\n"
2093-
"}",
2094-
Lower);
2095-
ASSERT_TRUE(FS.get() != nullptr);
2096-
std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
2097-
ASSERT_FALSE(EC);
2098-
ASSERT_TRUE(FS.get() != nullptr);
2099-
2100-
llvm::ErrorOr<vfs::Status> Status = FS->status("bar/a");
2101-
ASSERT_FALSE(Status.getError());
2102-
EXPECT_TRUE(Status->exists());
2103-
2104-
Status = FS->status("foo/a");
2105-
ASSERT_FALSE(Status.getError());
2106-
EXPECT_TRUE(Status->exists());
2107-
2108-
EC = FS->setCurrentWorkingDirectory("//root/bar/");
2109-
ASSERT_FALSE(EC);
2110-
2111-
Status = FS->status("./a");
2112-
ASSERT_FALSE(Status.getError());
2113-
EXPECT_TRUE(Status->exists());
2114-
2115-
Status = FS->status("./b");
2116-
ASSERT_TRUE(Status.getError());
2117-
2118-
Status = FS->status("./c");
2119-
ASSERT_TRUE(Status.getError());
2120-
2121-
EC = FS->setCurrentWorkingDirectory("//root/");
2122-
ASSERT_FALSE(EC);
2123-
2124-
Status = FS->status("c");
2125-
ASSERT_FALSE(Status.getError());
2126-
EXPECT_TRUE(Status->exists());
2127-
}
2128-
2129-
TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) {
2130-
IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
2131-
Lower->addDirectory("//root/");
2132-
Lower->addDirectory("//root/foo");
2133-
Lower->addRegularFile("//root/foo/a");
2134-
Lower->addRegularFile("//root/foo/b");
2135-
Lower->addRegularFile("//root/c");
2136-
IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
2137-
"{ 'use-external-names': false,\n"
2138-
" 'roots': [\n"
2139-
"{\n"
2140-
" 'type': 'directory',\n"
2141-
" 'name': '//root/bar',\n"
2142-
" 'contents': [ {\n"
2143-
" 'type': 'file',\n"
2144-
" 'name': 'a',\n"
2145-
" 'external-contents': '//root/foo/a'\n"
2146-
" }\n"
2147-
" ]\n"
2148-
"}\n"
2149-
"]\n"
2150-
"}",
2151-
Lower);
2152-
ASSERT_TRUE(FS.get() != nullptr);
2153-
std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
2154-
ASSERT_FALSE(EC);
2155-
ASSERT_TRUE(FS.get() != nullptr);
2156-
2157-
llvm::ErrorOr<vfs::Status> Status = FS->status("bar/a");
2158-
ASSERT_FALSE(Status.getError());
2159-
EXPECT_TRUE(Status->exists());
2160-
2161-
Status = FS->status("foo/a");
2162-
ASSERT_TRUE(Status.getError());
2163-
}

0 commit comments

Comments
 (0)