facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-09-11 10:36:52 +03:00
Code Issues Projects Releases Wiki Activity

Fix glob with overlapping patterns

Browse Source 
Summary:
If EdenFS' globFiles API is given two patterns, and one pattern is a prefix of the other, EdenFS effectively ignores the longer pattern. Given the patterns `project/src/*` and `project/src/*/*`, when EdenFS encounters `project/src/dir/`, it generates a result (because `project/src/dir/` matches the first pattern) but does not recurse into its children.

The problem caused by an incorrect understanding of the GlobNode::isLeaf_ flag. isLeaf_ means "this GlobNode should generate results", but GlobNode::evaluateImpl understands it to mean "this GlobNode should generate results, and this GlobNode has no children". Given the patterns `project/src/*` and `project/src/*/*`, the GlobNode representing `project/src/*` has isLeaf_=true but also has children.

Fix the bug by not using isLeaf_ for determining whether to recurse, and instead relying on the presence of child GlobNode-s.

Reviewed By: chadaustin

Differential Revision: D15078089

fbshipit-source-id: 1c480d11361f89193b35965266e6873c57181113
This commit is contained in:
Matt Glazar 2019-04-26 14:15:16 -07:00 committed by Facebook Github Bot
parent 74d6086e58
commit bc6e11b9dc
2 changed files with 62 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
eden/fs/inodes/GlobNode.cpp
View File

@ -305,7 +305,8 @@ Future<vector<GlobNode::GlobResult>> GlobNode::evaluateImpl(
auto recurseIfNecessary = [&](PathComponentPiece name,
GlobNode* node,
const auto& entry) {
if (root.entryIsTree(entry)) {
if ((!node->children_.empty() || !node->recursiveChildren_.empty()) &&
root.entryIsTree(entry)) {
if (root.entryShouldLoadChildTree(entry)) {
recurse.emplace_back(std::make_pair(name, node));
} else {
@ -339,7 +340,6 @@ Future<vector<GlobNode::GlobResult>> GlobNode::evaluateImpl(
if (fileBlobsToPrefetch && root.entryShouldPrefetch(entry)) {
fileBlobsToPrefetch->wlock()->emplace_back(root.entryHash(entry));
}
continue;
}
// Not the leaf of a pattern; if this is a dir, we need to recurse
@ -356,8 +356,6 @@ Future<vector<GlobNode::GlobResult>> GlobNode::evaluateImpl(
fileBlobsToPrefetch->wlock()->emplace_back(
root.entryHash(entry));
}
continue;
}
// Not the leaf of a pattern; if this is a dir, we need to
// recurse

60
eden/fs/inodes/test/GlobNodeTest.cpp
View File

@ -70,6 +70,10 @@ class GlobNodeTest : public ::testing::TestWithParam<
bool includeDotfiles) {
GlobNode globRoot(/*includeDotfiles=*/includeDotfiles);
globRoot.parse(pattern);
return doGlob(globRoot);
}
std::vector<GlobResult> doGlob(GlobNode& globRoot) {
globRoot.debugDump();
if (shouldPrefetch()) {
@ -189,6 +193,62 @@ TEST_P(GlobNodeTest, recursiveTxtWithChanges) {
}
}
TEST_P(GlobNodeTest, matchGlobDirectoryAndDirectoryChild) {
GlobNode globRoot(/*includeDotfiles=*/false);
globRoot.parse("dir/*");
globRoot.parse("dir/*/*");
auto matches = doGlob(globRoot);
std::vector<GlobResult> expect{
GlobResult("dir/a.txt"_relpath, dtype_t::Regular),
GlobResult("dir/sub"_relpath, dtype_t::Dir),
GlobResult("dir/sub/b.txt"_relpath, dtype_t::Regular),
};
EXPECT_EQ(expect, matches);
}
TEST_P(GlobNodeTest, matchGlobDirectoryAndDirectoryRecursiveChildren) {
GlobNode globRoot(/*includeDotfiles=*/false);
globRoot.parse("dir/*");
globRoot.parse("dir/*/**");
auto matches = doGlob(globRoot);
std::vector<GlobResult> expect{
GlobResult("dir/a.txt"_relpath, dtype_t::Regular),
GlobResult("dir/sub"_relpath, dtype_t::Dir),
GlobResult("dir/sub/b.txt"_relpath, dtype_t::Regular),
};
EXPECT_EQ(expect, matches);
}
TEST_P(GlobNodeTest, matchLiteralDirectoryAndDirectoryChild) {
GlobNode globRoot(/*includeDotfiles=*/false);
globRoot.parse("dir");
globRoot.parse("dir/a.txt");
auto matches = doGlob(globRoot);
std::vector<GlobResult> expect{
GlobResult("dir"_relpath, dtype_t::Dir),
GlobResult("dir/a.txt"_relpath, dtype_t::Regular),
};
EXPECT_EQ(expect, matches);
}
TEST_P(GlobNodeTest, matchLiteralDirectoryAndDirectoryRecursiveChildren) {
GlobNode globRoot(/*includeDotfiles=*/false);
globRoot.parse("dir");
globRoot.parse("dir/**");
auto matches = doGlob(globRoot);
std::vector<GlobResult> expect{
GlobResult("dir"_relpath, dtype_t::Dir),
GlobResult("dir/a.txt"_relpath, dtype_t::Regular),
GlobResult("dir/sub"_relpath, dtype_t::Dir),
GlobResult("dir/sub/b.txt"_relpath, dtype_t::Regular),
};
EXPECT_EQ(expect, matches);
}
const std::pair<enum StartReady, enum Prefetch> combinations[] = {
{StartReady::StartReady, Prefetch::NoPrefetch},
{StartReady::StartReady, Prefetch::PrefetchBlobs},