Skip to content

Commit

Permalink
Normalize filepaths on Windows.
Browse files Browse the repository at this point in the history
Summary:
This diff invokes [`generic_string`](https://en.cppreference.com/w/cpp/filesystem/path/generic_string) in `db.cpp` to normalize Windows paths to use forward slashes. Note that absolute paths are maintained with this approach, e.g. `C:\foo\bar` -> `C:/foo/bar`.

`PathTest.cpp` is also fixed to run correctly on Windows.

Reviewed By: malanka

Differential Revision: D56561550

fbshipit-source-id: 41393c5671ee5f68d942154a0c564253b67fe080
  • Loading branch information
mpark authored and facebook-github-bot committed Apr 25, 2024
1 parent 2ceed47 commit c4b0fe7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
6 changes: 3 additions & 3 deletions glean/lang/clang/db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ ClangDB::fileFromEntry(
#endif
}();
if (!buffer) {
LOG(WARNING) << "Couldn't get MemoryBuffer for " << path.string();
LOG(WARNING) << "Couldn't get MemoryBuffer for " << path.generic_string();
#if GLEAN_FACEBOOK
if (isBuckOutPath(path)) {
return {};
}
#endif
const auto file = batch.fact<Src::File>(path.string());
const auto file = batch.fact<Src::File>(path.generic_string());
return std::pair{file, std::move(path)};
}
// compute the SHA1 digest of the file content and get the file size
Expand All @@ -94,7 +94,7 @@ ClangDB::fileFromEntry(
path = replaceBuckOutHash(path, hash);
}
#endif
const auto file = batch.fact<Src::File>(path.string());
const auto file = batch.fact<Src::File>(path.generic_string());
const uint64_t size = entry.getSize();
batch.fact<Digest::FileDigest>(file, Digest::Digest{hash, size});

Expand Down
38 changes: 22 additions & 16 deletions glean/lang/clang/tests/PathTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,34 @@
namespace facebook::glean::clangx {

TEST(PathTest, goodPath) {
EXPECT_EQ(goodPath("/foo", "bar").native(), "bar");
static std::filesystem::path root =
#ifdef _WIN32
"C:\\";
#else
"/";
#endif

EXPECT_EQ(goodPath("/foo", "bar/../baz").native(), "baz");
EXPECT_EQ(goodPath(root/"foo", "bar").generic_string(), "bar");

EXPECT_EQ(goodPath("/foo", "../foo/bar").native(), "bar");
EXPECT_EQ(goodPath("/1/2/3", "../../2/3/foo").native(), "foo");
EXPECT_EQ(goodPath(root/"foo", "bar/../baz").generic_string(), "baz");

EXPECT_EQ(goodPath("/1/2", "/1/2/3/foo").native(), "3/foo");
EXPECT_EQ(goodPath(root/"foo", "../foo/bar").generic_string(), "bar");
EXPECT_EQ(goodPath(root/"1/2/3", "../../2/3/foo").generic_string(), "foo");

EXPECT_EQ(goodPath(root/"1/2", root/"1/2/3/foo").generic_string(), "3/foo");
// FIXME: should this be ../bar?
EXPECT_EQ(goodPath("/foo", "/bar").native(), "/bar");
EXPECT_EQ(goodPath(root/"foo", root/"bar").generic_string(), root/"bar");
// FIXME: should this be ../4/5?
EXPECT_EQ(goodPath("/1/2/3", "/1/2/4/5").native(), "/1/2/4/5");
EXPECT_EQ(goodPath(root/"1/2/3", root/"1/2/4/5").generic_string(), root/"1/2/4/5");

EXPECT_EQ(goodPath("/1/2/3","../4/5").native(), "../4/5");
EXPECT_EQ(goodPath(root/"1/2/3","../4/5").generic_string(), "../4/5");

EXPECT_EQ(goodPath("/1/2/3", "4/../../3/5").native(), "5");
EXPECT_EQ(goodPath(root/"1/2/3", "4/../../3/5").generic_string(), "5");

// FIXME: don't know what we want here
EXPECT_TRUE(goodPath("/foo", "/foo").empty());

EXPECT_TRUE(goodPath("/foo","").empty());
EXPECT_TRUE(goodPath(root/"foo", root/"foo").empty());

EXPECT_TRUE(goodPath(root/"foo", "").empty());
}

TEST(PathTest, followSymlinksInsideRoot) {
Expand All @@ -53,14 +59,14 @@ TEST(PathTest, followSymlinksInsideRoot) {

create_directory(root/"a");
create_directory(root/"a/c");
create_symlink("a", root/"b");
create_symlink(out, root/"escape");
create_directory_symlink("a", root/"b");
create_directory_symlink(out, root/"escape");

// symlink that traverses another symlink
create_symlink("../b/c", root/"a/d");
create_directory_symlink("../b/c", root/"a/d");

// symlink going back into root
create_symlink("../root/a/c", out/"back");
create_directory_symlink("../root/a/c", out/"back");

// no symlinks
EXPECT_EQ(followSymlinksInsideRoot(root, path("a/c")), path("a/c"));
Expand Down

0 comments on commit c4b0fe7

Please sign in to comment.