Allow modules to specify additional nodes in the graph to be invalidated and use this functionality to properly invalidate http download nodes if the downloaded zip file doesn't match the specified hash.
This still means that the actual files downloaded may not match, but checking all such files may be too expensive.
This helps with #336 but before that issue can be closed all remote repositories (git, etc.), should have similar functionality added.
--
MOS_MIGRATED_REVID=99317085
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 620f24e..76d135e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -14,6 +14,9 @@
package com.google.devtools.build.lib.bazel;
+import static com.google.common.hash.Hashing.sha256;
+import static com.google.devtools.build.lib.bazel.repository.HttpDownloader.getHash;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -27,6 +30,7 @@
import com.google.devtools.build.lib.bazel.repository.GitRepositoryFunction;
import com.google.devtools.build.lib.bazel.repository.HttpArchiveFunction;
import com.google.devtools.build.lib.bazel.repository.HttpDownloadFunction;
+import com.google.devtools.build.lib.bazel.repository.HttpDownloadValue;
import com.google.devtools.build.lib.bazel.repository.HttpFileFunction;
import com.google.devtools.build.lib.bazel.repository.HttpJarFunction;
import com.google.devtools.build.lib.bazel.repository.JarFunction;
@@ -62,17 +66,24 @@
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.util.Clock;
+import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.common.options.OptionsProvider;
+import java.io.IOException;
import java.util.Map.Entry;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
+import javax.annotation.Nullable;
+
/**
* Adds support for fetching external code.
*/
@@ -127,6 +138,33 @@
return ImmutableSet.of(RepositoryFunction.getExternalRepositoryDirectory(directories));
}
+ private static final SkyValueDirtinessChecker HTTP_DOWNLOAD_CHECKER =
+ new SkyValueDirtinessChecker() {
+ @Override
+ @Nullable
+ public DirtyResult maybeCheck(
+ SkyKey skyKey, SkyValue skyValue, TimestampGranularityMonitor tsgm) {
+ if (!skyKey.functionName().equals(HttpDownloadFunction.NAME)) {
+ return null;
+ }
+ HttpDownloadValue httpDownloadValue = (HttpDownloadValue) skyValue;
+ Path path = httpDownloadValue.getPath();
+ try {
+ return ((HttpDownloadFunction.HttpDescriptor) skyKey.argument())
+ .getSha256().equals(getHash(sha256().newHasher(), path))
+ ? DirtyResult.NOT_DIRTY
+ : DirtyResult.DIRTY;
+ } catch (IOException e) {
+ return DirtyResult.DIRTY;
+ }
+ }
+ };
+
+ @Override
+ public Iterable<SkyValueDirtinessChecker> getCustomDirtinessCheckers() {
+ return ImmutableList.of(HTTP_DOWNLOAD_CHECKER);
+ }
+
@Override
public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
for (Entry<String, RepositoryFunction> handler : repositoryHandlers.entrySet()) {
@@ -167,7 +205,7 @@
// Helper SkyFunctions.
downloadFunction = new HttpDownloadFunction();
- builder.put(SkyFunctionName.create(HttpDownloadFunction.NAME), downloadFunction);
+ builder.put(HttpDownloadFunction.NAME, downloadFunction);
gitCloneFunction = new GitCloneFunction();
builder.put(SkyFunctionName.create(GitCloneFunction.NAME), gitCloneFunction);
builder.put(JarFunction.NAME, new JarFunction());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloadFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloadFunction.java
index 640f4f9..5c7bf51 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloadFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloadFunction.java
@@ -35,7 +35,7 @@
* Downloads an archive file over HTTP.
*/
public class HttpDownloadFunction implements SkyFunction {
- public static final String NAME = "HTTP_DOWNLOAD";
+ public static final SkyFunctionName NAME = SkyFunctionName.create("HTTP_DOWNLOAD");
private Reporter reporter;
public void setReporter(Reporter reporter) {
@@ -47,10 +47,10 @@
public SkyValue compute(SkyKey skyKey, Environment env) throws RepositoryFunctionException {
HttpDescriptor descriptor = (HttpDescriptor) skyKey.argument();
try {
- // The downloaded file is not added to Skyframe, as changes to it cannot affect a build
- // (it's essentially a temporary file). The downloaded file is always an archive and its
- // contents, once decompressed, _can_ be dependencies of the build and _are_ added to
- // Skyframe (through the normal package mechanism).
+ // The downloaded file is not added to Skyframe here, as changes to it do not necessarily
+ // affect a build (it's usually essentially a temporary file). However, if the downloaded
+ // file's sha256 is not the expected value on subsequent builds, this is taken as a signal
+ // that it must be re-downloaded. This might happen if the user deleted the downloaded file.
return new HttpDownloadValue(
new HttpDownloader(
reporter,
@@ -79,23 +79,27 @@
String sha256 = mapper.get("sha256", Type.STRING);
String type = mapper.has("type", Type.STRING) ? mapper.get("type", Type.STRING) : "";
return new SkyKey(
- SkyFunctionName.create(NAME),
- new HttpDownloadFunction.HttpDescriptor(url, sha256, outputDirectory, type));
+ NAME, new HttpDownloadFunction.HttpDescriptor(url, sha256, outputDirectory, type));
}
- static final class HttpDescriptor {
+ /** Data about a remote repository to be accessed via http. */
+ public static final class HttpDescriptor {
private String url;
private String sha256;
private Path outputDirectory;
private String type;
- public HttpDescriptor(String url, String sha256, Path outputDirectory, String type) {
+ HttpDescriptor(String url, String sha256, Path outputDirectory, String type) {
this.url = url;
this.sha256 = sha256;
this.outputDirectory = outputDirectory;
this.type = type;
}
+ public String getSha256() {
+ return sha256;
+ }
+
@Override
public String toString() {
return url + " -> " + outputDirectory + " (" + sha256 + ")";
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 8d43019..a0d0aff 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.rules.test.CoverageReportActionFactory;
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorFactory;
import com.google.devtools.build.lib.syntax.BaseFunction;
@@ -193,6 +194,10 @@
return ImmutableMap.<String, String>of();
}
+ public Iterable<SkyValueDirtinessChecker> getCustomDirtinessCheckers() {
+ return ImmutableList.of();
+ }
+
/**
* Services provided for Blaze modules via BlazeRuntime.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index d8a590f..d1793cf 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -89,6 +89,7 @@
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutorFactory;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorFactory;
import com.google.devtools.build.lib.syntax.Label;
@@ -1732,13 +1733,29 @@
precomputedValues.addAll(module.getPrecomputedSkyframeValues());
}
+ ImmutableList.Builder<SkyValueDirtinessChecker> customDirtinessCheckers =
+ ImmutableList.builder();
+ for (BlazeModule module : blazeModules) {
+ customDirtinessCheckers.addAll(module.getCustomDirtinessCheckers());
+ }
+
final PackageFactory pkgFactory =
new PackageFactory(ruleClassProvider, platformRegexps, extensions);
- SkyframeExecutor skyframeExecutor = skyframeExecutorFactory.create(reporter, pkgFactory,
- timestampMonitor, directories, workspaceStatusActionFactory,
- ruleClassProvider.getBuildInfoFactories(), immutableDirectories, diffAwarenessFactories,
- allowedMissingInputs, preprocessorFactorySupplier, skyFunctions.build(),
- precomputedValues.build());
+ SkyframeExecutor skyframeExecutor =
+ skyframeExecutorFactory.create(
+ reporter,
+ pkgFactory,
+ timestampMonitor,
+ directories,
+ workspaceStatusActionFactory,
+ ruleClassProvider.getBuildInfoFactories(),
+ immutableDirectories,
+ diffAwarenessFactories,
+ allowedMissingInputs,
+ preprocessorFactorySupplier,
+ skyFunctions.build(),
+ precomputedValues.build(),
+ customDirtinessCheckers.build());
if (configurationFactory == null) {
configurationFactory = new ConfigurationFactory(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
new file mode 100644
index 0000000..c02b8be
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
@@ -0,0 +1,118 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import static com.google.devtools.build.lib.skyframe.SkyFunctions.DIRECTORY_LISTING_STATE;
+import static com.google.devtools.build.lib.skyframe.SkyFunctions.FILE_STATE;
+
+import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import java.io.IOException;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+
+/** Utilities for checking dirtiness of keys (mainly filesystem keys) in the graph. */
+class DirtinessCheckerUtils {
+ private DirtinessCheckerUtils() {}
+
+ static class BasicFilesystemDirtinessChecker implements SkyValueDirtinessChecker {
+ protected boolean applies(SkyKey skyKey) {
+ SkyFunctionName functionName = skyKey.functionName();
+ return (functionName.equals(FILE_STATE) || functionName.equals(DIRECTORY_LISTING_STATE));
+ }
+
+ @Override
+ @Nullable
+ public DirtyResult maybeCheck(SkyKey skyKey, SkyValue skyValue,
+ TimestampGranularityMonitor tsgm) {
+ if (!applies(skyKey)) {
+ return null;
+ }
+ RootedPath rootedPath = (RootedPath) skyKey.argument();
+ if (skyKey.functionName().equals(FILE_STATE)) {
+ return checkFileStateValue(rootedPath, (FileStateValue) skyValue, tsgm);
+ } else {
+ return checkDirectoryListingStateValue(rootedPath, (DirectoryListingStateValue) skyValue);
+ }
+ }
+
+ private static DirtyResult checkFileStateValue(
+ RootedPath rootedPath, FileStateValue fileStateValue, TimestampGranularityMonitor tsgm) {
+ try {
+ FileStateValue newValue = FileStateValue.create(rootedPath, tsgm);
+ return newValue.equals(fileStateValue)
+ ? DirtyResult.NOT_DIRTY
+ : DirtyResult.dirtyWithNewValue(newValue);
+ } catch (InconsistentFilesystemException | IOException e) {
+ // TODO(bazel-team): An IOException indicates a failure to get a file digest or a symlink
+ // target, not a missing file. Such a failure really shouldn't happen, so failing early
+ // may be better here.
+ return DirtyResult.DIRTY;
+ }
+ }
+
+ private static DirtyResult checkDirectoryListingStateValue(
+ RootedPath dirRootedPath, DirectoryListingStateValue directoryListingStateValue) {
+ try {
+ DirectoryListingStateValue newValue = DirectoryListingStateValue.create(dirRootedPath);
+ return newValue.equals(directoryListingStateValue)
+ ? DirtyResult.NOT_DIRTY
+ : DirtyResult.dirtyWithNewValue(newValue);
+ } catch (IOException e) {
+ return DirtyResult.DIRTY;
+ }
+ }
+ }
+
+ static final class MissingDiffDirtinessChecker extends BasicFilesystemDirtinessChecker {
+ private final Set<Path> missingDiffPaths;
+
+ MissingDiffDirtinessChecker(final Set<Path> missingDiffPaths) {
+ this.missingDiffPaths = missingDiffPaths;
+ }
+
+ @Override
+ protected boolean applies(SkyKey skyKey) {
+ return super.applies(skyKey)
+ && missingDiffPaths.contains(((RootedPath) skyKey.argument()).getRoot());
+ }
+ }
+
+ /** {@link SkyValueDirtinessChecker} that encompasses a union of other dirtiness checkers. */
+ static final class UnionDirtinessChecker implements SkyValueDirtinessChecker {
+ private final Iterable<SkyValueDirtinessChecker> dirtinessCheckers;
+
+ UnionDirtinessChecker(Iterable<SkyValueDirtinessChecker> dirtinessCheckers) {
+ this.dirtinessCheckers = dirtinessCheckers;
+ }
+
+ @Override
+ @Nullable
+ public DirtyResult maybeCheck(SkyKey key, SkyValue oldValue, TimestampGranularityMonitor tsgm) {
+ for (SkyValueDirtinessChecker dirtinessChecker : dirtinessCheckers) {
+ DirtyResult dirtyResult = dirtinessChecker.maybeCheck(key, oldValue, tsgm);
+ if (dirtyResult != null) {
+ return dirtyResult;
+ }
+ }
+ return null;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index d2f4a06..941ddbd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -19,8 +19,6 @@
import com.google.common.base.Suppliers;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Range;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
@@ -28,12 +26,12 @@
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.concurrent.Sharder;
import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker.DirtyResult;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.BatchStat;
import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
-import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.Differencer;
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -65,9 +63,6 @@
private static final int DIRTINESS_CHECK_THREADS = 50;
private static final Logger LOG = Logger.getLogger(FilesystemValueChecker.class.getName());
- private static final Predicate<SkyKey> FILE_STATE_AND_DIRECTORY_LISTING_STATE_FILTER =
- SkyFunctionName.functionIsIn(ImmutableSet.of(SkyFunctions.FILE_STATE,
- SkyFunctions.DIRECTORY_LISTING_STATE));
private static final Predicate<SkyKey> ACTION_FILTER =
SkyFunctionName.functionIs(SkyFunctions.ACTION_EXECUTION);
@@ -94,43 +89,20 @@
});
}
- Iterable<SkyKey> getFilesystemSkyKeys() {
- return Iterables.filter(valuesSupplier.get().keySet(),
- FILE_STATE_AND_DIRECTORY_LISTING_STATE_FILTER);
- }
-
- Differencer.Diff getDirtyFilesystemSkyKeys() throws InterruptedException {
- return getDirtyFilesystemValues(getFilesystemSkyKeys());
- }
-
/**
- * Check the given file and directory values for modifications. {@code values} is assumed to only
- * have {@link FileValue}s and {@link DirectoryListingStateValue}s.
+ * Returns a {@link Differencer.Diff} containing keys that are dirty according to the passed-in
+ * {@param dirtinessChecker}.
*/
- Differencer.Diff getDirtyFilesystemValues(Iterable<SkyKey> values)
+ Differencer.Diff getDirtyKeys(SkyValueDirtinessChecker dirtinessChecker)
throws InterruptedException {
- return getDirtyValues(values, FILE_STATE_AND_DIRECTORY_LISTING_STATE_FILTER,
- new DirtyChecker() {
- @Override
- public DirtyResult check(SkyKey key, SkyValue oldValue, TimestampGranularityMonitor tsgm) {
- if (key.functionName() == SkyFunctions.FILE_STATE) {
- return checkFileStateValue((RootedPath) key.argument(), (FileStateValue) oldValue,
- tsgm);
- } else if (key.functionName() == SkyFunctions.DIRECTORY_LISTING_STATE) {
- return checkDirectoryListingStateValue((RootedPath) key.argument(),
- (DirectoryListingStateValue) oldValue);
- } else {
- throw new IllegalStateException("Unexpected key type " + key);
- }
- }
- });
+ return getDirtyValues(valuesSupplier.get().keySet(), dirtinessChecker);
}
/**
* Return a collection of action values which have output files that are not in-sync with
* the on-disk file value (were modified externally).
*/
- public Collection<SkyKey> getDirtyActionValues(@Nullable final BatchStat batchStatter)
+ Collection<SkyKey> getDirtyActionValues(@Nullable final BatchStat batchStatter)
throws InterruptedException {
// CPU-bound (usually) stat() calls, plus a fudge factor.
LOG.info("Accumulating dirty actions");
@@ -262,10 +234,8 @@
return modifiedOutputFilesCounter.get();
}
- /**
- * Returns the number of modified output files that occur during the previous build.
- */
- public int getNumberOfModifiedOutputFilesDuringPreviousBuild() {
+ /** Returns the number of modified output files that occur during the previous build. */
+ int getNumberOfModifiedOutputFilesDuringPreviousBuild() {
return modifiedOutputFilesIntraBuildCounter.get();
}
@@ -293,29 +263,31 @@
return isDirty;
}
- private BatchDirtyResult getDirtyValues(Iterable<SkyKey> values,
- Predicate<SkyKey> keyFilter,
- final DirtyChecker checker) throws InterruptedException {
- ExecutorService executor = Executors.newFixedThreadPool(DIRTINESS_CHECK_THREADS,
- new ThreadFactoryBuilder().setNameFormat("FileSystem Value Invalidator %d").build());
+ private BatchDirtyResult getDirtyValues(
+ Iterable<SkyKey> values, final SkyValueDirtinessChecker checker) throws InterruptedException {
+ ExecutorService executor =
+ Executors.newFixedThreadPool(
+ DIRTINESS_CHECK_THREADS,
+ new ThreadFactoryBuilder().setNameFormat("FileSystem Value Invalidator %d").build());
final BatchDirtyResult batchResult = new BatchDirtyResult();
ThrowableRecordingRunnableWrapper wrapper =
new ThrowableRecordingRunnableWrapper("FilesystemValueChecker#getDirtyValues");
for (final SkyKey key : values) {
- Preconditions.checkState(keyFilter.apply(key), key);
final SkyValue value = valuesSupplier.get().get(key);
- executor.execute(wrapper.wrap(new Runnable() {
- @Override
- public void run() {
- if (value != null) {
- DirtyResult result = checker.check(key, value, tsgm);
- if (result.isDirty()) {
- batchResult.add(key, result.getNewValue());
- }
- }
- }
- }));
+ executor.execute(
+ wrapper.wrap(
+ new Runnable() {
+ @Override
+ public void run() {
+ if (value != null) {
+ DirtyResult result = checker.maybeCheck(key, value, tsgm);
+ if (result != null && result.isDirty()) {
+ batchResult.add(key, result.getNewValue());
+ }
+ }
+ }
+ }));
}
boolean interrupted = ExecutorUtil.interruptibleShutdown(executor);
@@ -326,34 +298,9 @@
return batchResult;
}
- private static DirtyResult checkFileStateValue(RootedPath rootedPath,
- FileStateValue fileStateValue, TimestampGranularityMonitor tsgm) {
- try {
- FileStateValue newValue = FileStateValue.create(rootedPath, tsgm);
- return newValue.equals(fileStateValue)
- ? DirtyResult.NOT_DIRTY : DirtyResult.dirtyWithNewValue(newValue);
- } catch (InconsistentFilesystemException | IOException e) {
- // TODO(bazel-team): An IOException indicates a failure to get a file digest or a symlink
- // target, not a missing file. Such a failure really shouldn't happen, so failing early
- // may be better here.
- return DirtyResult.DIRTY;
- }
- }
-
- private static DirtyResult checkDirectoryListingStateValue(RootedPath dirRootedPath,
- DirectoryListingStateValue directoryListingStateValue) {
- try {
- DirectoryListingStateValue newValue = DirectoryListingStateValue.create(dirRootedPath);
- return newValue.equals(directoryListingStateValue)
- ? DirtyResult.NOT_DIRTY : DirtyResult.dirtyWithNewValue(newValue);
- } catch (IOException e) {
- return DirtyResult.DIRTY;
- }
- }
-
/**
- * Result of a batch call to {@link DirtyChecker#check}. Partitions the dirty values based on
- * whether we have a new value available for them or not.
+ * Result of a batch call to {@link SkyValueDirtinessChecker#maybeCheck}. Partitions the dirty
+ * values based on whether we have a new value available for them or not.
*/
private static class BatchDirtyResult implements Differencer.Diff {
@@ -381,41 +328,4 @@
}
}
- private static class DirtyResult {
-
- static final DirtyResult NOT_DIRTY = new DirtyResult(false, null);
- static final DirtyResult DIRTY = new DirtyResult(true, null);
-
- private final boolean isDirty;
- @Nullable private final SkyValue newValue;
-
- private DirtyResult(boolean isDirty, @Nullable SkyValue newValue) {
- this.isDirty = isDirty;
- this.newValue = newValue;
- }
-
- boolean isDirty() {
- return isDirty;
- }
-
- /**
- * If {@code isDirty()}, then either returns the new value for the value or {@code null} if
- * the new value wasn't computed. In the case where the value is dirty and a new value is
- * available, then the new value can be injected into the skyframe graph. Otherwise, the value
- * should simply be invalidated.
- */
- @Nullable
- SkyValue getNewValue() {
- Preconditions.checkState(isDirty());
- return newValue;
- }
-
- static DirtyResult dirtyWithNewValue(SkyValue newValue) {
- return new DirtyResult(true, newValue);
- }
- }
-
- private static interface DirtyChecker {
- DirtyResult check(SkyKey key, SkyValue oldValue, TimestampGranularityMonitor tsgm);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index c28bcad..000f21f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -21,11 +21,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BuildView;
@@ -41,6 +39,9 @@
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker;
+import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.MissingDiffDirtinessChecker;
+import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.UnionDirtinessChecker;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.ResourceUsage;
@@ -48,7 +49,6 @@
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.BuildDriver;
import com.google.devtools.build.skyframe.Differencer;
import com.google.devtools.build.skyframe.ImmutableDiff;
@@ -65,7 +65,7 @@
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.List;
+import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
@@ -93,22 +93,39 @@
private RecordingDifferencer recordingDiffer;
private final DiffAwarenessManager diffAwarenessManager;
+ private final Iterable<SkyValueDirtinessChecker> customDirtinessCheckers;
- private SequencedSkyframeExecutor(Reporter reporter, EvaluatorSupplier evaluatorSupplier,
- PackageFactory pkgFactory, TimestampGranularityMonitor tsgm,
- BlazeDirectories directories, Factory workspaceStatusActionFactory,
+ private SequencedSkyframeExecutor(
+ Reporter reporter,
+ EvaluatorSupplier evaluatorSupplier,
+ PackageFactory pkgFactory,
+ TimestampGranularityMonitor tsgm,
+ BlazeDirectories directories,
+ Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
Predicate<PathFragment> allowedMissingInputs,
Preprocessor.Factory.Supplier preprocessorFactorySupplier,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
- ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues) {
- super(reporter, evaluatorSupplier, pkgFactory, tsgm, directories,
- workspaceStatusActionFactory, buildInfoFactories, immutableDirectories,
- allowedMissingInputs, preprocessorFactorySupplier,
- extraSkyFunctions, extraPrecomputedValues, /*errorOnExternalFiles=*/false);
+ ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues,
+ Iterable<SkyValueDirtinessChecker> customDirtinessCheckers) {
+ super(
+ reporter,
+ evaluatorSupplier,
+ pkgFactory,
+ tsgm,
+ directories,
+ workspaceStatusActionFactory,
+ buildInfoFactories,
+ immutableDirectories,
+ allowedMissingInputs,
+ preprocessorFactorySupplier,
+ extraSkyFunctions,
+ extraPrecomputedValues, /*errorOnExternalFiles=*/
+ false);
this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories, reporter);
+ this.customDirtinessCheckers = customDirtinessCheckers;
}
public static SequencedSkyframeExecutor create(
@@ -123,7 +140,8 @@
Predicate<PathFragment> allowedMissingInputs,
Preprocessor.Factory.Supplier preprocessorFactorySupplier,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
- ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues) {
+ ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues,
+ Iterable<SkyValueDirtinessChecker> customDirtinessCheckers) {
SequencedSkyframeExecutor skyframeExecutor =
new SequencedSkyframeExecutor(
reporter,
@@ -138,7 +156,8 @@
allowedMissingInputs,
preprocessorFactorySupplier,
extraSkyFunctions,
- extraPrecomputedValues);
+ extraPrecomputedValues,
+ customDirtinessCheckers);
skyframeExecutor.init();
return skyframeExecutor;
}
@@ -150,12 +169,20 @@
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories) {
- return create(reporter, pkgFactory, tsgm, directories, workspaceStatusActionFactory,
- buildInfoFactories, immutableDirectories, diffAwarenessFactories,
+ return create(
+ reporter,
+ pkgFactory,
+ tsgm,
+ directories,
+ workspaceStatusActionFactory,
+ buildInfoFactories,
+ immutableDirectories,
+ diffAwarenessFactories,
Predicates.<PathFragment>alwaysFalse(),
Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
ImmutableMap.<SkyFunctionName, SkyFunction>of(),
- ImmutableList.<PrecomputedValue.Injected>of());
+ ImmutableList.<PrecomputedValue.Injected>of(),
+ ImmutableList.<SkyValueDirtinessChecker>of());
}
@Override
@@ -207,7 +234,8 @@
* it via an explicit Skyframe dependency. They need to be invalidated if the package locator
* changes.
*/
- private static final Set<SkyFunctionName> PACKAGE_LOCATOR_DEPENDENT_VALUES = ImmutableSet.of(
+ private static final Set<SkyFunctionName> PACKAGE_LOCATOR_DEPENDENT_VALUES =
+ ImmutableSet.of(
SkyFunctions.AST_FILE_LOOKUP,
SkyFunctions.FILE_STATE,
SkyFunctions.FILE,
@@ -305,7 +333,7 @@
private void handleDiffsWithMissingDiffInformation(
Set<Pair<Path, DiffAwarenessManager.ProcessableModifiedFileSet>>
pathEntriesWithoutDiffInformation) throws InterruptedException {
- if (pathEntriesWithoutDiffInformation.isEmpty()) {
+ if (pathEntriesWithoutDiffInformation.isEmpty() && Iterables.isEmpty(customDirtinessCheckers)) {
return;
}
@@ -319,26 +347,20 @@
// We need to manually check for changes to known files. This entails finding all dirty file
// system values under package roots for which we don't have diff information. If at least
// one path entry doesn't have diff information, then we're going to have to iterate over
- // the skyframe values at least once no matter what so we might as well do so now and avoid
- // doing so more than once.
- Iterable<SkyKey> filesystemSkyKeys = fsnc.getFilesystemSkyKeys();
- // Partition by package path entry.
- Multimap<Path, SkyKey> skyKeysByPathEntry = partitionSkyKeysByPackagePathEntry(
- ImmutableSet.copyOf(pkgLocator.get().getPathEntries()), filesystemSkyKeys);
-
- // Contains all file system values that we need to check for dirtiness.
- List<Path> pathEntriesChecked =
- Lists.newArrayListWithCapacity(pathEntriesWithoutDiffInformation.size());
- List<Iterable<SkyKey>> valuesToCheckManually = Lists.newArrayList();
+ // the skyframe values at least once no matter what.
+ Set<Path> pathEntries = new HashSet<>();
for (Pair<Path, DiffAwarenessManager.ProcessableModifiedFileSet> pair :
pathEntriesWithoutDiffInformation) {
- Path pathEntry = pair.getFirst();
- valuesToCheckManually.add(skyKeysByPathEntry.get(pathEntry));
- pathEntriesChecked.add(pathEntry);
+ pathEntries.add(pair.getFirst());
}
-
- Differencer.Diff diff = fsnc.getDirtyFilesystemValues(Iterables.concat(valuesToCheckManually));
- handleChangedFiles(pathEntriesChecked, diff);
+ Differencer.Diff diff =
+ fsnc.getDirtyKeys(
+ new UnionDirtinessChecker(
+ Iterables.concat(
+ customDirtinessCheckers,
+ ImmutableList.<SkyValueDirtinessChecker>of(
+ new MissingDiffDirtinessChecker(pathEntries)))));
+ handleChangedFiles(pathEntries, diff);
for (Pair<Path, DiffAwarenessManager.ProcessableModifiedFileSet> pair :
pathEntriesWithoutDiffInformation) {
@@ -346,31 +368,7 @@
}
}
- /**
- * Partitions the given filesystem values based on which package path root they are under.
- * Returns a {@link Multimap} {@code m} such that {@code m.containsEntry(k, pe)} is true for
- * each filesystem valuekey {@code k} under a package path root {@code pe}. Note that values not
- * under a package path root are not present in the returned {@link Multimap}; these values are
- * unconditionally checked for changes on each incremental build.
- */
- private static Multimap<Path, SkyKey> partitionSkyKeysByPackagePathEntry(
- Set<Path> pkgRoots, Iterable<SkyKey> filesystemSkyKeys) {
- ImmutableSetMultimap.Builder<Path, SkyKey> multimapBuilder =
- ImmutableSetMultimap.builder();
- for (SkyKey key : filesystemSkyKeys) {
- Preconditions.checkState(key.functionName() == SkyFunctions.FILE_STATE
- || key.functionName() == SkyFunctions.DIRECTORY_LISTING_STATE, key);
- Path root = ((RootedPath) key.argument()).getRoot();
- if (pkgRoots.contains(root)) {
- multimapBuilder.put(root, key);
- }
- // We don't need to worry about FileStateValues for external files because they have a
- // dependency on the build_id and so they get invalidated each build.
- }
- return multimapBuilder.build();
- }
-
- private void handleChangedFiles(List<Path> pathEntries, Differencer.Diff diff) {
+ private void handleChangedFiles(Collection<Path> pathEntries, Differencer.Diff diff) {
Collection<SkyKey> changedKeysWithoutNewValues = diff.changedKeysWithoutNewValues();
Map<SkyKey, ? extends SkyValue> changedKeysWithNewValues = diff.changedKeysWithNewValues();
@@ -449,7 +447,8 @@
Iterable<SkyKey> keys;
if (modifiedFileSet.treatEverythingAsModified()) {
Differencer.Diff diff =
- new FilesystemValueChecker(memoizingEvaluator, tsgm, null).getDirtyFilesystemSkyKeys();
+ new FilesystemValueChecker(memoizingEvaluator, tsgm, null)
+ .getDirtyKeys(new BasicFilesystemDirtinessChecker());
keys = diff.changedKeysWithoutNewValues();
recordingDiffer.inject(diff.changedKeysWithNewValues());
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
index 7e277a7..b39707f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
@@ -36,18 +36,33 @@
public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory {
@Override
- public SkyframeExecutor create(Reporter reporter, PackageFactory pkgFactory,
- TimestampGranularityMonitor tsgm, BlazeDirectories directories,
- Factory workspaceStatusActionFactory, ImmutableList<BuildInfoFactory> buildInfoFactories,
+ public SkyframeExecutor create(
+ Reporter reporter,
+ PackageFactory pkgFactory,
+ TimestampGranularityMonitor tsgm,
+ BlazeDirectories directories,
+ Factory workspaceStatusActionFactory,
+ ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
Predicate<PathFragment> allowedMissingInputs,
Preprocessor.Factory.Supplier preprocessorFactorySupplier,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
- ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues) {
- return SequencedSkyframeExecutor.create(reporter, pkgFactory, tsgm, directories,
- workspaceStatusActionFactory, buildInfoFactories, immutableDirectories,
- diffAwarenessFactories, allowedMissingInputs, preprocessorFactorySupplier,
- extraSkyFunctions, extraPrecomputedValues);
+ ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues,
+ Iterable<SkyValueDirtinessChecker> customDirtinessCheckers) {
+ return SequencedSkyframeExecutor.create(
+ reporter,
+ pkgFactory,
+ tsgm,
+ directories,
+ workspaceStatusActionFactory,
+ buildInfoFactories,
+ immutableDirectories,
+ diffAwarenessFactories,
+ allowedMissingInputs,
+ preprocessorFactorySupplier,
+ extraSkyFunctions,
+ extraPrecomputedValues,
+ customDirtinessCheckers);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java
new file mode 100644
index 0000000..3ebbc29
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java
@@ -0,0 +1,76 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import javax.annotation.Nullable;
+
+/**
+ * Given a {@link SkyKey} and the previous {@link SkyValue} it had, returns whether this value is
+ * up to date.
+ */
+public interface SkyValueDirtinessChecker {
+ /**
+ * Returns the result of checking whether this key's value is up to date, or null if this
+ * dirtiness checker does not apply to this key. If non-null, this answer is assumed to be
+ * definitive.
+ */
+ @Nullable DirtyResult maybeCheck(SkyKey key, SkyValue oldValue, TimestampGranularityMonitor tsgm);
+
+ /** An encapsulation of the result of checking to see if a value is up to date. */
+ class DirtyResult {
+ /** The external value is unchanged from the value in the graph. */
+ public static final DirtyResult NOT_DIRTY = new DirtyResult(false, null);
+ /**
+ * The external value is different from the value in the graph, but the new value is not known.
+ */
+ public static final DirtyResult DIRTY = new DirtyResult(true, null);
+
+ /**
+ * Creates a DirtyResult indicating that the external value is {@param newValue}, which is
+ * different from the value in the graph,
+ */
+ public static DirtyResult dirtyWithNewValue(SkyValue newValue) {
+ return new DirtyResult(true, newValue);
+ }
+
+ private final boolean isDirty;
+ @Nullable private final SkyValue newValue;
+
+ private DirtyResult(boolean isDirty, @Nullable SkyValue newValue) {
+ this.isDirty = isDirty;
+ this.newValue = newValue;
+ }
+
+ boolean isDirty() {
+ return isDirty;
+ }
+
+ /**
+ * If {@code isDirty()}, then either returns the new value for the value or {@code null} if
+ * the new value wasn't computed. In the case where the value is dirty and a new value is
+ * available, then the new value can be injected into the skyframe graph. Otherwise, the value
+ * should simply be invalidated.
+ */
+ @Nullable
+ SkyValue getNewValue() {
+ Preconditions.checkState(isDirty(), newValue);
+ return newValue;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
index a1615cf..7710fb0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
@@ -41,8 +41,6 @@
*
* @param reporter the reporter to be used by the executor
* @param pkgFactory the package factory
- * @param skyframeBuild use Skyframe for the build phase. Should be always true after we are in
- * the skyframe full mode.
* @param tsgm timestamp granularity monitor
* @param directories Blaze directories
* @param workspaceStatusActionFactory a factory for creating WorkspaceStatusAction objects
@@ -52,11 +50,15 @@
* @param preprocessorFactorySupplier
* @param extraSkyFunctions
* @param extraPrecomputedValues
+ * @param customDirtinessCheckers
* @return an instance of the SkyframeExecutor
* @throws AbruptExitException if the executor cannot be created
*/
- SkyframeExecutor create(Reporter reporter, PackageFactory pkgFactory,
- TimestampGranularityMonitor tsgm, BlazeDirectories directories,
+ SkyframeExecutor create(
+ Reporter reporter,
+ PackageFactory pkgFactory,
+ TimestampGranularityMonitor tsgm,
+ BlazeDirectories directories,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -64,5 +66,7 @@
Predicate<PathFragment> allowedMissingInputs,
Preprocessor.Factory.Supplier preprocessorFactorySupplier,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
- ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues) throws AbruptExitException;
+ ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues,
+ Iterable<SkyValueDirtinessChecker> customDirtinessCheckers)
+ throws AbruptExitException;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 94975bb..7c2351a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -50,6 +50,7 @@
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.syntax.Label;
@@ -161,16 +162,21 @@
throws Exception {
this.ruleClassProvider = ruleClassProvider;
PackageFactory pkgFactory = new PackageFactory(ruleClassProvider);
- skyframeExecutor = SequencedSkyframeExecutor.create(reporter, pkgFactory,
- new TimestampGranularityMonitor(BlazeClock.instance()), directories,
- workspaceStatusActionFactory,
- ruleClassProvider.getBuildInfoFactories(), ImmutableSet.<Path>of(),
- ImmutableList.<DiffAwareness.Factory>of(),
- Predicates.<PathFragment>alwaysFalse(),
- Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
- ImmutableMap.<SkyFunctionName, SkyFunction>of(),
- getPrecomputedValues()
- );
+ skyframeExecutor =
+ SequencedSkyframeExecutor.create(
+ reporter,
+ pkgFactory,
+ new TimestampGranularityMonitor(BlazeClock.instance()),
+ directories,
+ workspaceStatusActionFactory,
+ ruleClassProvider.getBuildInfoFactories(),
+ ImmutableSet.<Path>of(),
+ ImmutableList.<DiffAwareness.Factory>of(),
+ Predicates.<PathFragment>alwaysFalse(),
+ Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
+ ImmutableMap.<SkyFunctionName, SkyFunction>of(),
+ getPrecomputedValues(),
+ ImmutableList.<SkyValueDirtinessChecker>of());
skyframeExecutor.preparePackageLoading(pkgLocator,
Options.getDefaults(PackageCacheOptions.class).defaultVisibility, true,
3, ruleClassProvider.getDefaultsPackageContent(), UUID.randomUUID());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 453815f..ab6f345 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -107,6 +107,7 @@
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.Label.SyntaxException;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
@@ -186,18 +187,21 @@
new AnalysisTestUtil.DummyWorkspaceStatusActionFactory(directories);
mutableActionGraph = new MapBasedActionGraph();
ruleClassProvider = getRuleClassProvider();
- skyframeExecutor = SequencedSkyframeExecutor.create(reporter,
- new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
- new TimestampGranularityMonitor(BlazeClock.instance()), directories,
- workspaceStatusActionFactory,
- ruleClassProvider.getBuildInfoFactories(),
- ImmutableSet.<Path>of(),
- ImmutableList.<DiffAwareness.Factory>of(),
- Predicates.<PathFragment>alwaysFalse(),
- getPreprocessorFactorySupplier(),
- ImmutableMap.<SkyFunctionName, SkyFunction>of(),
- getPrecomputedValues()
- );
+ skyframeExecutor =
+ SequencedSkyframeExecutor.create(
+ reporter,
+ new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
+ new TimestampGranularityMonitor(BlazeClock.instance()),
+ directories,
+ workspaceStatusActionFactory,
+ ruleClassProvider.getBuildInfoFactories(),
+ ImmutableSet.<Path>of(),
+ ImmutableList.<DiffAwareness.Factory>of(),
+ Predicates.<PathFragment>alwaysFalse(),
+ getPreprocessorFactorySupplier(),
+ ImmutableMap.<SkyFunctionName, SkyFunction>of(),
+ getPrecomputedValues(),
+ ImmutableList.<SkyValueDirtinessChecker>of());
skyframeExecutor.preparePackageLoading(
new PathPackageLocator(rootDirectory), ConstantRuleVisibility.PUBLIC, true, 7, "",
UUID.randomUUID());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index 4222a91..34b967e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -92,16 +93,21 @@
pkgFactory = new PackageFactory(ruleClassProvider);
AnalysisTestUtil.DummyWorkspaceStatusActionFactory workspaceStatusActionFactory =
new AnalysisTestUtil.DummyWorkspaceStatusActionFactory(directories);
- skyframeExecutor = SequencedSkyframeExecutor.create(reporter, pkgFactory,
- new TimestampGranularityMonitor(BlazeClock.instance()), directories,
- workspaceStatusActionFactory,
- ruleClassProvider.getBuildInfoFactories(), ImmutableSet.<Path>of(),
- ImmutableList.<DiffAwareness.Factory>of(),
- Predicates.<PathFragment>alwaysFalse(),
- Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
- ImmutableMap.<SkyFunctionName, SkyFunction>of(),
- ImmutableList.<PrecomputedValue.Injected>of()
- );
+ skyframeExecutor =
+ SequencedSkyframeExecutor.create(
+ reporter,
+ pkgFactory,
+ new TimestampGranularityMonitor(BlazeClock.instance()),
+ directories,
+ workspaceStatusActionFactory,
+ ruleClassProvider.getBuildInfoFactories(),
+ ImmutableSet.<Path>of(),
+ ImmutableList.<DiffAwareness.Factory>of(),
+ Predicates.<PathFragment>alwaysFalse(),
+ Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
+ ImmutableMap.<SkyFunctionName, SkyFunction>of(),
+ ImmutableList.<PrecomputedValue.Injected>of(),
+ ImmutableList.<SkyValueDirtinessChecker>of());
skyframeExecutor.preparePackageLoading(pkgLocator,
Options.getDefaults(PackageCacheOptions.class).defaultVisibility, true,
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
index 133e76d..dd44efc 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.Label.SyntaxException;
@@ -69,19 +70,21 @@
super.setUp();
ruleClassProvider = TestRuleClassProvider.getRuleClassProvider();
- skyframeExecutor = SequencedSkyframeExecutor.create(reporter,
- new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
- new TimestampGranularityMonitor(BlazeClock.instance()),
- new BlazeDirectories(outputBase, outputBase, rootDirectory),
- null, /* workspaceStatusActionFactory */
- ruleClassProvider.getBuildInfoFactories(),
- ImmutableSet.<Path>of(),
- ImmutableList.<DiffAwareness.Factory>of(),
- Predicates.<PathFragment>alwaysFalse(),
- Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
- ImmutableMap.<SkyFunctionName, SkyFunction>of(),
- ImmutableList.<PrecomputedValue.Injected>of()
- );
+ skyframeExecutor =
+ SequencedSkyframeExecutor.create(
+ reporter,
+ new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
+ new TimestampGranularityMonitor(BlazeClock.instance()),
+ new BlazeDirectories(outputBase, outputBase, rootDirectory),
+ null, /* workspaceStatusActionFactory */
+ ruleClassProvider.getBuildInfoFactories(),
+ ImmutableSet.<Path>of(),
+ ImmutableList.<DiffAwareness.Factory>of(),
+ Predicates.<PathFragment>alwaysFalse(),
+ Preprocessor.Factory.Supplier.NullSupplier.INSTANCE,
+ ImmutableMap.<SkyFunctionName, SkyFunction>of(),
+ ImmutableList.<PrecomputedValue.Injected>of(),
+ ImmutableList.<SkyValueDirtinessChecker>of());
skyframeExecutor.preparePackageLoading(
new PathPackageLocator(rootDirectory), ConstantRuleVisibility.PUBLIC, true, 7, "",
UUID.randomUUID());
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index ecae37c..e799302 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -70,7 +70,8 @@
EOF
cat $1 >> $http_response
- nc_port=$(pick_random_unused_tcp_port) || exit 1
+ # Assign random_port to nc_port if not already set.
+ echo ${nc_port:=$(pick_random_unused_tcp_port)} > /dev/null
nc_log=$TEST_TMPDIR/nc.log
nc_l $nc_port < $http_response >& $nc_log &
nc_pid=$!
@@ -138,36 +139,41 @@
# male
function http_archive_helper() {
zipper=$1
+ local write_workspace
+ [[ $# -gt 1 ]] && [[ "$2" = "nowrite" ]] && write_workspace=1 || write_workspace=0
- # Create a zipped-up repository HTTP response.
- repo2=$TEST_TMPDIR/repo2
- rm -rf $repo2
- mkdir -p $repo2/fox
- cd $repo2
- touch WORKSPACE
- cat > fox/BUILD <<EOF
+ if [[ $write_workspace = 0 ]]; then
+ # Create a zipped-up repository HTTP response.
+ repo2=$TEST_TMPDIR/repo2
+ rm -rf $repo2
+ mkdir -p $repo2/fox
+ cd $repo2
+ touch WORKSPACE
+ cat > fox/BUILD <<EOF
filegroup(
name = "fox",
srcs = ["male"],
visibility = ["//visibility:public"],
)
EOF
- what_does_the_fox_say="Fraka-kaka-kaka-kaka-kow"
- cat > fox/male <<EOF
+ what_does_the_fox_say="Fraka-kaka-kaka-kaka-kow"
+ cat > fox/male <<EOF
#!/bin/bash
echo $what_does_the_fox_say
EOF
- chmod +x fox/male
- # Add some padding to the .zip to test that Bazel's download logic can
- # handle breaking a response into chunks.
- dd if=/dev/zero of=fox/padding bs=1024 count=10240
- $zipper
- repo2_name=$(basename $repo2_zip)
- sha256=$(sha256sum $repo2_zip | cut -f 1 -d ' ')
+ chmod +x fox/male
+ # Add some padding to the .zip to test that Bazel's download logic can
+ # handle breaking a response into chunks.
+ dd if=/dev/zero of=fox/padding bs=1024 count=10240 >& $TEST_log
+ $zipper >& $TEST_log
+ repo2_name=$(basename $repo2_zip)
+ sha256=$(sha256sum $repo2_zip | cut -f 1 -d ' ')
+ fi
serve_file $repo2_zip
cd ${WORKSPACE_DIR}
- cat > WORKSPACE <<EOF
+ if [[ $write_workspace = 0 ]]; then
+ cat > WORKSPACE <<EOF
http_archive(
name = 'endangered',
url = 'http://localhost:$nc_port/$repo2_name',
@@ -175,7 +181,7 @@
)
EOF
- cat > zoo/BUILD <<EOF
+ cat > zoo/BUILD <<EOF
sh_binary(
name = "breeding-program",
srcs = ["female.sh"],
@@ -183,13 +189,14 @@
)
EOF
- cat > zoo/female.sh <<EOF
+ cat > zoo/female.sh <<EOF
#!/bin/bash
./external/endangered/fox/male
EOF
- chmod +x zoo/female.sh
+ chmod +x zoo/female.sh
+fi
- bazel run //zoo:breeding-program >& $TEST_log \
+ bazel run //zoo:breeding-program >& $TEST_log --show_progress_rate_limit=0 \
|| echo "Expected build/run to succeed"
kill_nc
expect_log $what_does_the_fox_say
@@ -311,6 +318,21 @@
expect_log $what_does_the_fox_say
}
+function test_changed_zip() {
+ nc_port=$(pick_random_unused_tcp_port) || fail "Couldn't get TCP port"
+ http_archive_helper zip_up
+ http_archive_helper zip_up "nowrite"
+ expect_not_log "Downloading from"
+ local readonly output_base=$(bazel info output_base)
+ local readonly repo_zip=$output_base/external/endangered/fox.zip
+ rm $repo_zip || fail "Couldn't delete $repo_zip"
+ touch $repo_zip || fail "Couldn't touch $repo_zip"
+ [[ -s $repo_zip ]] && fail "File size not 0"
+ http_archive_helper zip_up "nowrite"
+ expect_log "Downloading from"
+ [[ -s $repo_zip ]] || fail "File size was 0"
+}
+
# Tests downloading a jar and using it as a Java dependency.
function test_jar_download() {
serve_jar
