Skip to content

Commit

Permalink
inodes: fix globbing bug with **
Browse files Browse the repository at this point in the history
Summary:
When matching against the ** pattern, EdenFS would match against the full path
instead of against just the current entry. This means that what was matched by
the glob prior to ** will be rechecked against by the pattern that follows **,
which isn't what is expected.

As a bonus, the `candidateName` variable will be constructed less than before
leading to less memory allocations.

Reviewed By: chadaustin

Differential Revision: D29079762

fbshipit-source-id: af15ecf229ce7119100dd375df23269bb7cdb1c0
  • Loading branch information
xavierd authored and facebook-github-bot committed Jun 15, 2021
1 parent 16bfb84 commit ea2e2f8
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
15 changes: 7 additions & 8 deletions eden/fs/inodes/GlobNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,13 +510,11 @@ Future<vector<GlobNode::GlobResult>> GlobNode::evaluateRecursiveComponentImpl(
{
const auto& contents = root.lockContents();
for (auto& entry : root.iterate(contents)) {
auto candidateName = rootPath + root.entryName(entry);

auto name = root.entryName(entry);
for (auto& node : recursiveChildren_) {
if (node->alwaysMatch_ ||
node->matcher_.match(candidateName.stringPiece())) {
if (node->alwaysMatch_ || node->matcher_.match(name.stringPiece())) {
results.emplace_back(
root.entryToResult(candidateName.copy(), entry, originRootId));
root.entryToResult(rootPath + name, entry, originRootId));
if (fileBlobsToPrefetch && root.entryShouldPrefetch(entry)) {
fileBlobsToPrefetch->wlock()->emplace_back(root.entryHash(entry));
}
Expand All @@ -527,13 +525,14 @@ Future<vector<GlobNode::GlobResult>> GlobNode::evaluateRecursiveComponentImpl(

// Remember to recurse through child dirs after we've released
// the lock on the contents.
if (root.entryIsTree(entry)) {
if (root.entryIsTree(entry) &&
(includeDotfiles_ || !name.stringPiece().startsWith('.'))) {
if (root.entryShouldLoadChildTree(entry)) {
subDirNames.emplace_back(std::move(candidateName));
subDirNames.emplace_back(rootPath + name);
} else {
futures.emplace_back(
store->getTree(root.entryHash(entry), context)
.thenValue([candidateName = std::move(candidateName),
.thenValue([candidateName = rootPath + name,
store,
&context,
this,
Expand Down
22 changes: 22 additions & 0 deletions eden/fs/inodes/test/GlobNodeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,29 @@ TEST_P(GlobNodeTest, starExcludeDot) {
EXPECT_EQ(expect, matches);
}

TEST_P(GlobNodeTest, starStarExcludeDot) {
auto matches = doGlobExcludeDotFiles("dir/sub/**/sub/b.txt", kZeroRootId);

std::vector<GlobResult> expect;
EXPECT_EQ(expect, matches);
}

TEST_P(GlobNodeTest, starStarRootExcludeDot) {
auto matches = doGlobExcludeDotFiles("**/root", kZeroRootId);

std::vector<GlobResult> expect;
EXPECT_EQ(expect, matches);
}

#ifndef _WIN32
TEST_P(GlobNodeTest, starStarRootIncludeDot) {
auto matches = doGlobIncludeDotFiles("**/root", kZeroRootId);

std::vector<GlobResult> expect{
GlobResult(".eden/root"_relpath, dtype_t::Symlink, kZeroRootId)};
EXPECT_EQ(expect, matches);
}

TEST_P(GlobNodeTest, recursiveTxtWithChanges) {
// Ensure that we enumerate things from the overlay
mount_.addFile("root.txt", "added\n");
Expand Down

0 comments on commit ea2e2f8

Please sign in to comment.