Skip to content

Commit

Permalink
Always explicitly specify the syscall cache for UnixGlob, instead of …
Browse files Browse the repository at this point in the history
…defaulting to SyscallCache.NO_CACHE.

PiperOrigin-RevId: 428122705
  • Loading branch information
janakdr authored and copybara-github committed Feb 12, 2022
1 parent d533996 commit 152e705
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.google.devtools.build.lib.util.VarInt;
import com.google.devtools.build.lib.vfs.DigestUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.lib.vfs.UnixGlob;
import java.io.ByteArrayOutputStream;
import java.io.DataInputStream;
Expand Down Expand Up @@ -279,11 +280,15 @@ private static CompactPersistentActionCache logAndThrowOrRecurse(
private static void renameCorruptedFiles(Path cacheRoot) {
try {
for (Path path :
UnixGlob.forPath(cacheRoot).addPattern("action_*_v" + VERSION + ".*").glob()) {
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
.addPattern("action_*_v" + VERSION + ".*")
.glob()) {
path.renameTo(path.getParentDirectory().getChild(path.getBaseName() + ".bad"));
}
for (Path path :
UnixGlob.forPath(cacheRoot).addPattern("filename_*_v" + VERSION + ".*").glob()) {
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
.addPattern("filename_*_v" + VERSION + ".*")
.glob()) {
path.renameTo(path.getParentDirectory().getChild(path.getBaseName() + ".bad"));
}
} catch (UnixGlob.BadPattern ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,7 @@ private ImmutableList<Artifact> getHintedInclusionsLegacy(Artifact artifact) {
// foo/bar/**/*.h. No examples of this currently exist in the INCLUDE_HINTS
// file.
logger.atFine().log("Globbing: %s %s", root, rule.findFilter);
hints.addAll(
new UnixGlob.Builder(root)
.setFilesystemCalls(syscallCache)
.addPattern(rule.findFilter)
.glob());
hints.addAll(new UnixGlob.Builder(root, syscallCache).addPattern(rule.findFilter).glob());
} catch (UnixGlob.BadPattern | IOException e) {
logger.atWarning().withCause(e).log("Error in hint expansion");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,10 @@ Future<List<Path>> safeGlobUnsorted(String pattern, Globber.Operation globberOpe
throw new BadGlobException(error + " (in glob pattern '" + pattern + "')");
}
try {
return UnixGlob.forPath(packageDirectory)
return new UnixGlob.Builder(packageDirectory, syscallCache)
.addPattern(pattern)
.setPathDiscriminator(new GlobUnixPathDiscriminator(globberOperation))
.setExecutor(globExecutor)
.setFilesystemCalls(syscallCache)
.globAsync();
} catch (UnixGlob.BadPattern ex) {
throw new BadGlobException(ex.getMessage());
Expand Down
21 changes: 4 additions & 17 deletions src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

package com.google.devtools.build.lib.vfs;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -280,10 +278,6 @@ private static Pattern makePatternFromWildcard(String pattern) {
return Pattern.compile(regexp.toString());
}

public static Builder forPath(Path path) {
return new Builder(path);
}

/**
* Builder class for UnixGlob.
*
Expand All @@ -292,15 +286,14 @@ public static Builder forPath(Path path) {
public static class Builder {
private final Path base;
private final List<String> patterns;
private final SyscallCache syscallCache;
private UnixGlobPathDiscriminator pathDiscriminator = DEFAULT_DISCRIMINATOR;
private Executor executor;
private SyscallCache syscallCache = SyscallCache.NO_CACHE;

/**
* Creates a glob builder with the given base path.
*/
public Builder(Path base) {
/** Creates a glob builder with the given base path. */
public Builder(Path base, SyscallCache syscallCache) {
this.base = base;
this.syscallCache = syscallCache;
this.patterns = Lists.newArrayList();
}

Expand Down Expand Up @@ -334,12 +327,6 @@ public Builder addPatterns(Collection<String> patterns) {
return this;
}

/** Sets the SyscallCache interface to use on this glob(). */
public Builder setFilesystemCalls(SyscallCache syscallCache) {
this.syscallCache = checkNotNull(syscallCache);
return this;
}

/**
* Sets the executor to use for parallel glob evaluation. If unset, evaluation is done
* in-thread.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.lib.vfs.UnixGlob;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -45,8 +46,10 @@ public void testCorruptionActionCacheErrorMessage() throws Exception {
outputBase.getChild("action_cache_temp").renameTo(outputBase.getChild("action_cache"));

// now corrupt filename index datafile by truncating it
for (Path path : UnixGlob.forPath(outputBase.getChild("action_cache"))
.addPattern("filename*.blaze").globInterruptible()) {
for (Path path :
new UnixGlob.Builder(outputBase.getChild("action_cache"), SyscallCache.NO_CACHE)
.addPattern("filename*.blaze")
.globInterruptible()) {
path.getOutputStream().close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,10 @@ public void testSpecialRegexCharacter() throws Exception {
FileSystemUtils.createEmptyFile(pkgPath.getChild("aab"));
// Note: this contains two asterisks because otherwise a RE is not built,
// as an optimization.
assertThat(UnixGlob.forPath(pkgPath).addPattern("*a.b*").globInterruptible())
assertThat(
new UnixGlob.Builder(pkgPath, SyscallCache.NO_CACHE)
.addPattern("*a.b*")
.globInterruptible())
.containsExactly(aDotB);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.testutil.BlazeTestUtils;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.lib.vfs.UnixGlob;
import java.io.IOException;
import org.junit.Before;
Expand Down Expand Up @@ -375,7 +376,9 @@ public void testPersistentCache_missingFilenameIndexCausesActionReexecution() th
// Remove filename index file.
assertThat(
Iterables.getOnlyElement(
UnixGlob.forPath(cacheRoot).addPattern("filename_index*").globInterruptible())
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
.addPattern("filename_index*")
.globInterruptible())
.delete())
.isTrue();

Expand Down Expand Up @@ -420,7 +423,9 @@ public void testPersistentCache_failedIntegrityCheckCausesActionReexecution() th
// Get filename index path and store a copy of it.
Path indexPath =
Iterables.getOnlyElement(
UnixGlob.forPath(cacheRoot).addPattern("filename_index*").globInterruptible());
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
.addPattern("filename_index*")
.globInterruptible());
Path indexCopy = scratch.resolve("index_copy");
FileSystemUtils.copyFile(indexPath, indexCopy);

Expand Down
Loading

0 comments on commit 152e705

Please sign in to comment.