diff options
Diffstat (limited to 'src')
15 files changed, 504 insertions, 277 deletions
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 620f24e9d4..76d135e2fb 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.GitCloneFunction; 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.BlazeModule; 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 @@ public class BazelRepositoryModule extends BlazeModule { 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 @@ public class BazelRepositoryModule extends BlazeModule { // 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 640f4f96d2..5c7bf51371 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 @@ import javax.annotation.Nullable; * 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 class HttpDownloadFunction implements SkyFunction { 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 @@ public class HttpDownloadFunction implements SkyFunction { 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 8d430191c9..a0d0affb7e 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.query2.output.OutputFormatter; 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 @@ public abstract class BlazeModule { 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 d8a590fefb..d1793cfe2c 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.server.signal.InterruptSignalHandler; 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 @@ public final class BlazeRuntime { 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 0000000000..c02b8be34a --- /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 d2f4a069ae..941ddbdaec 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.Supplier; 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.actions.Artifact; 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 @@ class FilesystemValueChecker { 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 @@ class FilesystemValueChecker { }); } - 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 @@ class FilesystemValueChecker { 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 @@ class FilesystemValueChecker { 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 @@ class FilesystemValueChecker { 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 @@ class FilesystemValueChecker { } } - 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 c28bcad28d..000f21f015 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.base.Predicates; 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.packages.Preprocessor; 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.util.io.TimestampGranularityMonitor; 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 com.google.devtools.build.skyframe.SkyValue; 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { allowedMissingInputs, preprocessorFactorySupplier, extraSkyFunctions, - extraPrecomputedValues); + extraPrecomputedValues, + customDirtinessCheckers); skyframeExecutor.init(); return skyframeExecutor; } @@ -150,12 +169,20 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { * 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { // 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { } } - /** - * 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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { 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 7e277a7fbc..b39707f3f4 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 @@ import java.util.Set; 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 0000000000..3ebbc29e6c --- /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 a1615cf02e..7710fb0b67 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 @@ public interface SkyframeExecutorFactory { * * @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 @@ public interface SkyframeExecutorFactory { * @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 @@ public interface SkyframeExecutorFactory { 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 94975bb5fe..7c2351aa2b 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.ConfiguredTargetKey; 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 @@ public abstract class AnalysisTestCase extends FoundationTestCase { 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 453815fd63..ab6f345d1d 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.ConfiguredTargetKey; 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 @@ public abstract class BuildViewTestCase extends FoundationTestCase { 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 4222a911f3..34b967ef86 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.pkgcache.PathPackageLocator; 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 @@ public abstract class ConfigurationTestCase extends FoundationTestCase { 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 133e76dc87..dd44efc42e 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.pkgcache.PathPackageLocator; 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 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { 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 ecae37cea0..e799302e9b 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh @@ -70,7 +70,8 @@ HTTP/1.0 200 OK 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 @@ function tar_gz_up() { # male function http_archive_helper() { zipper=$1 - - # 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 + local write_workspace + [[ $# -gt 1 ]] && [[ "$2" = "nowrite" ]] && write_workspace=1 || write_workspace=0 + + 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 @@ http_archive( ) EOF - cat > zoo/BUILD <<EOF + cat > zoo/BUILD <<EOF sh_binary( name = "breeding-program", srcs = ["female.sh"], @@ -183,13 +189,14 @@ sh_binary( ) 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 @@ EOF 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 |