From 8fd7f754771a3793d1089e3845320342cf6d61bb Mon Sep 17 00:00:00 2001 From: felly Date: Fri, 14 Apr 2017 20:09:49 +0200 Subject: Migrate UnixGlob to Path#statIfFound() instead of #statNullable(). The latter swallows all filesystem failures, and does not disambiguate missing files from filesystem problems. The syscall cache now tracks IOExceptions if they are present, just as it does with readdir(). RELNOTES: None PiperOrigin-RevId: 153185433 --- .../build/lib/pkgcache/PathPackageLocator.java | 23 +++++++++++------ .../build/lib/skyframe/PerBuildSyscallCache.java | 28 +++++++++++++-------- .../google/devtools/build/lib/vfs/UnixGlob.java | 17 ++++++++----- .../google/devtools/build/lib/vfs/GlobTest.java | 29 ++++++++++++++++++++-- 4 files changed, 72 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java index 30609168f3..f828538419 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.UnixGlob; +import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; @@ -111,10 +112,14 @@ public class PathPackageLocator implements Serializable { // invocation in Parser#include(). Path buildFile = outputBase.getRelative( packageIdentifier.getSourceRoot()).getRelative("BUILD"); - FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW); - if (stat != null && stat.isFile()) { - return buildFile; - } else { + try { + FileStatus stat = cache.get().statIfFound(buildFile, Symlinks.FOLLOW); + if (stat != null && stat.isFile()) { + return buildFile; + } else { + return null; + } + } catch (IOException e) { return null; } } @@ -225,9 +230,13 @@ public class PathPackageLocator implements Serializable { AtomicReference cache) { for (Path pathEntry : pathEntries) { Path buildFile = pathEntry.getRelative(suffix); - FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW); - if (stat != null && stat.isFile()) { - return buildFile; + try { + FileStatus stat = cache.get().statIfFound(buildFile, Symlinks.FOLLOW); + if (stat != null && stat.isFile()) { + return buildFile; + } + } catch (IOException ignored) { + // Treat IOException as a missing file. } } return null; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java index 646d6093dd..d9eb4506a6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java @@ -30,13 +30,14 @@ import java.util.Collection; */ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls { - private final LoadingCache, FileStatus> statCache; + private final LoadingCache, Pair> statCache; private final LoadingCache, Pair, IOException>> readdirCache; private static final FileStatus NO_STATUS = new FakeFileStatus(); - private PerBuildSyscallCache(LoadingCache, FileStatus> statCache, + private PerBuildSyscallCache( + LoadingCache, Pair> statCache, LoadingCache, Pair, IOException>> readdirCache) { this.statCache = statCache; this.readdirCache = readdirCache; @@ -104,9 +105,12 @@ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls { } @Override - public FileStatus statNullable(Path path, Symlinks symlinks) { - FileStatus status = statCache.getUnchecked(Pair.of(path, symlinks)); - return (status == NO_STATUS) ? null : status; + public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException { + Pair status = statCache.getUnchecked(Pair.of(path, symlinks)); + if (status.getFirst() != null) { + return (status.getFirst() == NO_STATUS) ? null : status.getFirst(); + } + throw status.getSecond(); } public void clear() { @@ -162,12 +166,16 @@ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls { * Input: (path, following_symlinks) * Output: FileStatus */ - private static CacheLoader, FileStatus> newStatLoader() { - return new CacheLoader, FileStatus>() { + private static CacheLoader, Pair> newStatLoader() { + return new CacheLoader, Pair>() { @Override - public FileStatus load(Pair p) { - FileStatus f = p.first.statNullable(p.second); - return (f == null) ? NO_STATUS : f; + public Pair load(Pair p) { + try { + FileStatus f = p.first.statIfFound(p.second); + return Pair.of((f == null) ? NO_STATUS : f, null); + } catch (IOException e) { + return Pair.of(null, e); + } } }; } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java index d05677fe24..261072db75 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java @@ -271,7 +271,7 @@ public final class UnixGlob { /** * Return the stat() for the given path, or null. */ - FileStatus statNullable(Path path, Symlinks symlinks); + FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException; } public static FilesystemCalls DEFAULT_SYSCALLS = new FilesystemCalls() { @@ -281,8 +281,8 @@ public final class UnixGlob { } @Override - public FileStatus statNullable(Path path, Symlinks symlinks) { - return path.statNullable(symlinks); + public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException { + return path.statIfFound(symlinks); } }; @@ -547,14 +547,19 @@ public final class UnixGlob { * Same as {@link #glob}, except does so asynchronously and returns a {@link Future} for the * result. */ - public Future> globAsync( + Future> globAsync( Path base, Collection patterns, boolean excludeDirectories, Predicate dirPred, FilesystemCalls syscalls) { - FileStatus baseStat = syscalls.statNullable(base, Symlinks.FOLLOW); + FileStatus baseStat; + try { + baseStat = syscalls.statIfFound(base, Symlinks.FOLLOW); + } catch (IOException e) { + return Futures.immediateFailedFuture(e); + } if (baseStat == null || patterns.isEmpty()) { return Futures.immediateFuture(Collections.emptyList()); } @@ -782,7 +787,7 @@ public final class UnixGlob { if (!pattern.contains("*") && !pattern.contains("?")) { // We do not need to do a readdir in this case, just a stat. Path child = base.getChild(pattern); - FileStatus status = context.syscalls.statNullable(child, Symlinks.FOLLOW); + FileStatus status = context.syscalls.statIfFound(child, Symlinks.FOLLOW); if (status == null || (!status.isDirectory() && !status.isFile())) { // The file is a dangling symlink, fifo, does not exist, etc. return; diff --git a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java index 430078ae86..c74e61d3b1 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java @@ -184,12 +184,37 @@ public class GlobTest { return expectedFiles; } + @Test + public void testIOFailureOnStat() throws Exception { + UnixGlob.FilesystemCalls syscalls = new UnixGlob.FilesystemCalls() { + @Override + public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException { + throw new IOException("EIO"); + } + + @Override + public Collection readdir(Path path, Symlinks symlinks) { + throw new IllegalStateException(); + } + }; + + try { + new UnixGlob.Builder(tmpPath) + .addPattern("foo/bar/wiz/file") + .setFilesystemCalls(new AtomicReference<>(syscalls)) + .glob(); + fail("Expected failure"); + } catch (IOException e) { + assertThat(e).hasMessageThat().isEqualTo("EIO"); + } + } + @Test public void testGlobWithoutWildcardsDoesNotCallReaddir() throws Exception { UnixGlob.FilesystemCalls syscalls = new UnixGlob.FilesystemCalls() { @Override - public FileStatus statNullable(Path path, Symlinks symlinks) { - return UnixGlob.DEFAULT_SYSCALLS.statNullable(path, symlinks); + public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException { + return UnixGlob.DEFAULT_SYSCALLS.statIfFound(path, symlinks); } @Override -- cgit v1.2.3