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 2750ddece8aac2..1d0fa393fa55ec 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 @@ -50,10 +50,11 @@ import com.google.devtools.build.lib.pkgcache.PackageCacheOptions; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl; import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryRule; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; -import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; +import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryDirtinessChecker; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; import com.google.devtools.build.lib.runtime.BlazeModule; @@ -67,18 +68,14 @@ import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected; import com.google.devtools.build.lib.skyframe.SkyFunctions; -import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryBootstrap; import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; -import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingResult; import java.io.IOException; @@ -87,7 +84,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; -import javax.annotation.Nullable; /** Adds support for fetching external code. */ public class BazelRepositoryModule extends BlazeModule { @@ -105,10 +101,14 @@ public class BazelRepositoryModule extends BlazeModule { private final MutableSupplier> clientEnvironmentSupplier = new MutableSupplier<>(); private ImmutableMap overrides = ImmutableMap.of(); - private Optional resolvedFile = Optional.absent(); - private Optional resolvedFileReplacingWorkspace = Optional.absent(); - private Set outputVerificationRules = ImmutableSet.of(); + private Optional resolvedFile = Optional.absent(); + private Optional resolvedFileReplacingWorkspace = Optional.absent(); + private Set outputVerificationRules = ImmutableSet.of(); private FileSystem filesystem; + // We hold the precomputed value of the managed directories here, so that the dependency + // on WorkspaceFileValue is not registered for each FileStateValue. + private final ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge = + new ManagedDirectoriesKnowledgeImpl(); public BazelRepositoryModule() { this.skylarkRepositoryFunction = new SkylarkRepositoryFunction(httpDownloader); @@ -128,35 +128,6 @@ public static ImmutableMap repositoryRules( .build(); } - /** - * A dirtiness checker that always dirties {@link RepositoryDirectoryValue}s so that if they were - * produced in a {@code --nofetch} build, they are re-created no subsequent {@code --fetch} - * builds. - * - *

The alternative solution would be to reify the value of the flag as a Skyframe value. - */ - private static final SkyValueDirtinessChecker REPOSITORY_VALUE_CHECKER = - new SkyValueDirtinessChecker() { - @Override - public boolean applies(SkyKey skyKey) { - return skyKey.functionName().equals(SkyFunctions.REPOSITORY_DIRECTORY); - } - - @Override - public SkyValue createNewValue(SkyKey key, @Nullable TimestampGranularityMonitor tsgm) { - throw new UnsupportedOperationException(); - } - - @Override - public DirtyResult check( - SkyKey skyKey, SkyValue skyValue, @Nullable TimestampGranularityMonitor tsgm) { - RepositoryDirectoryValue repositoryValue = (RepositoryDirectoryValue) skyValue; - return repositoryValue.repositoryExists() && repositoryValue.isFetchingDelayed() - ? DirtyResult.dirty(skyValue) - : DirtyResult.notDirty(skyValue); - } - }; - private static class RepositoryCacheInfoItem extends InfoItem { private final RepositoryCache repositoryCache; @@ -182,17 +153,25 @@ public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builde @Override public void workspaceInit( BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { - builder.addCustomDirtinessChecker(REPOSITORY_VALUE_CHECKER); + builder.setWorkspaceFileHeaderListener( + value -> + managedDirectoriesKnowledge.setManagedDirectories( + value != null ? value.getManagedDirectories() : ImmutableMap.of())); + + RepositoryDirectoryDirtinessChecker customDirtinessChecker = + new RepositoryDirectoryDirtinessChecker(managedDirectoriesKnowledge); + builder.addCustomDirtinessChecker(customDirtinessChecker); // Create the repository function everything flows through. builder.addSkyFunction(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction()); - builder.addSkyFunction( - SkyFunctions.REPOSITORY_DIRECTORY, + RepositoryDelegatorFunction repositoryDelegatorFunction = new RepositoryDelegatorFunction( repositoryHandlers, skylarkRepositoryFunction, isFetch, clientEnvironmentSupplier, - directories)); + directories, + managedDirectoriesKnowledge); + builder.addSkyFunction(SkyFunctions.REPOSITORY_DIRECTORY, repositoryDelegatorFunction); builder.addSkyFunction(MavenServerFunction.NAME, new MavenServerFunction(directories)); filesystem = runtime.getFileSystem(); } @@ -233,7 +212,7 @@ public void beforeCommand(CommandEnvironment env) { env.getReporter() .handle( Event.warn( - "Ingoring request to scale timeouts for repositories by a non-positive" + "Ignoring request to scale timeouts for repositories by a non-positive" + " factor")); skylarkRepositoryFunction.setTimeoutScaling(1.0); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java new file mode 100644 index 00000000000000..66ca1569bddb45 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java @@ -0,0 +1,51 @@ +// Copyright 2019 The Bazel Authors. 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.rules.repository; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.RootedPath; +import javax.annotation.Nullable; + +/** + * Interface to access the managed directories holder object. + * + *

Managed directories are user-owned directories, which can be incrementally updated by + * repository rules, so that the updated files are visible for the actions in the same build. + * + *

Having managed directories as a separate component (and not SkyValue) allows to skip recording + * the dependency in Skyframe for each FileStateValue and DirectoryListingStateValue. + */ +public interface ManagedDirectoriesKnowledge { + ManagedDirectoriesKnowledge NO_MANAGED_DIRECTORIES = + new ManagedDirectoriesKnowledge() { + @Nullable + @Override + public RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old) { + return null; + } + + @Override + public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { + return ImmutableSet.of(); + } + }; + + @Nullable + RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old); + + ImmutableSet getManagedDirectories(RepositoryName repositoryName); +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java new file mode 100644 index 00000000000000..08b44fab54a222 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java @@ -0,0 +1,101 @@ +// Copyright 2019 The Bazel Authors. 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.rules.repository; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.RootedPath; +import java.util.Comparator; +import java.util.Map; +import java.util.NavigableMap; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; + +/** Managed directories component. {@link ManagedDirectoriesKnowledge} */ +public class ManagedDirectoriesKnowledgeImpl implements ManagedDirectoriesKnowledge { + private final AtomicReference> + managedDirectoriesRef = new AtomicReference<>(ImmutableSortedMap.of()); + private final AtomicReference>> repoToDirMapRef = + new AtomicReference<>(ImmutableMap.of()); + + /** + * During build commands execution, Skyframe caches the states of files (FileStateValue) and + * directory listings (DirectoryListingStateValue). In the case when the files/directories are + * under a managed directory or inside an external repository, evaluation of file/directory + * listing states requires that the RepositoryDirectoryValue of the owning external repository is + * evaluated beforehand. (So that the repository rule generates the files.) So there is a + * dependency on RepositoryDirectoryValue for files under managed directories and external + * repositories. This dependency is recorded by Skyframe, + * + *

From the other side, by default Skyframe injects the new values of changed files already at + * the stage of checking what files have changed. Only the values without any dependencies can be + * injected into Skyframe. Skyframe can be specifically instructed to not inject new values and + * only register them as changed. + * + *

When the values of managed directories change, some files appear to become files under + * managed directories, or they are no longer files under managed directories. This implies that + * corresponding file/directory listing states gain the dependency (RepositoryDirectoryValue) or + * they lose this dependency. In both cases, we should prevent Skyframe from injecting those new + * values of file/directory listing states at the stage of checking changed files. + * + *

That is why we need to keep track of the previous value of the managed directories. + */ + private final AtomicReference> + oldManagedDirectoriesRef = new AtomicReference<>(ImmutableSortedMap.of()); + + @Override + @Nullable + public RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old) { + PathFragment relativePath = rootedPath.getRootRelativePath(); + NavigableMap map = + old ? oldManagedDirectoriesRef.get() : managedDirectoriesRef.get(); + Map.Entry entry = map.floorEntry(relativePath); + if (entry != null && relativePath.startsWith(entry.getKey())) { + return entry.getValue(); + } + return null; + } + + @Override + public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { + ImmutableSet pathFragments = repoToDirMapRef.get().get(repositoryName); + return pathFragments != null ? pathFragments : ImmutableSet.of(); + } + + public void setManagedDirectories(ImmutableMap map) { + oldManagedDirectoriesRef.set(managedDirectoriesRef.get()); + ImmutableSortedMap pathsMap = ImmutableSortedMap.copyOf(map); + managedDirectoriesRef.set(pathsMap); + + Map> reposMap = Maps.newHashMap(); + for (Map.Entry entry : pathsMap.entrySet()) { + RepositoryName repositoryName = entry.getValue(); + reposMap.computeIfAbsent(repositoryName, name -> Sets.newTreeSet()).add(entry.getKey()); + } + + ImmutableSortedMap.Builder> builder = + new ImmutableSortedMap.Builder<>(Comparator.comparing(RepositoryName::getName)); + for (Map.Entry> entry : reposMap.entrySet()) { + builder.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue())); + } + repoToDirMapRef.set(builder.build()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index a4161cc3637ea9..f0acce2e66a2bd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -18,7 +18,9 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.collect.Ordering; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; @@ -44,11 +46,14 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import java.util.stream.Collectors; import javax.annotation.Nullable; /** @@ -93,6 +98,9 @@ public final class RepositoryDelegatorFunction implements SkyFunction { private final AtomicBoolean isFetch; private final BlazeDirectories directories; + // Managed directories mappings, pre-calculated and injected by SequencedSkyframeExecutor + // before each command. + private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge; private final Supplier> clientEnvironmentSupplier; @@ -101,12 +109,14 @@ public RepositoryDelegatorFunction( @Nullable RepositoryFunction skylarkHandler, AtomicBoolean isFetch, Supplier> clientEnvironmentSupplier, - BlazeDirectories directories) { + BlazeDirectories directories, + ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { this.handlers = handlers; this.skylarkHandler = skylarkHandler; this.isFetch = isFetch; this.clientEnvironmentSupplier = clientEnvironmentSupplier; this.directories = directories; + this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; } private void setupRepositoryRoot(Path repoRoot) throws RepositoryFunctionException { @@ -154,9 +164,15 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } + ImmutableSet managedDirectories = + managedDirectoriesKnowledge.getManagedDirectories(repositoryName); DigestWriter digestWriter = new DigestWriter( - directories, repositoryName, rule, Preconditions.checkNotNull(ruleSpecificData)); + directories, + repositoryName, + rule, + Preconditions.checkNotNull(ruleSpecificData), + managedDirectories); // Local repositories are fetched regardless of the marker file because the operation is // generally fast and they do not depend on non-local data, so it does not make much sense to @@ -179,7 +195,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } - return RepositoryDirectoryValue.builder().setPath(repoRoot).setDigest(markerHash).build(); + return RepositoryDirectoryValue.builder() + .setPath(repoRoot) + .setDigest(markerHash) + .setManagedDirectories(managedDirectories) + .build(); } } } @@ -197,7 +217,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) // restart thus calling the possibly very slow (networking, decompression...) fetch() // operation again. So we write the marker file here immediately. byte[] digest = digestWriter.writeMarkerFile(); - return builder.setDigest(digest).build(); + return builder.setDigest(digest).setManagedDirectories(managedDirectories).build(); } if (!repoRoot.exists()) { @@ -223,7 +243,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) + "run the build without the '--nofetch' command line option.", rule.getName()))); - return RepositoryDirectoryValue.builder().setPath(repoRoot).setFetchingDelayed().build(); + return RepositoryDirectoryValue.builder() + .setPath(repoRoot) + .setFetchingDelayed() + .setManagedDirectories(managedDirectories) + .build(); } private RepositoryFunction getHandler(Rule rule) { @@ -337,6 +361,7 @@ static String unescape(String str) { } private static class DigestWriter { + private static final String MANAGED_DIRECTORIES_MARKER = "$MANAGED"; private final Path markerPath; private final Rule rule; private final Map markerData; @@ -346,11 +371,19 @@ private static class DigestWriter { BlazeDirectories directories, RepositoryName repositoryName, Rule rule, - byte[] ruleSpecificData) { + byte[] ruleSpecificData, + ImmutableSet managedDirectories) { ruleKey = computeRuleKey(rule, ruleSpecificData); markerPath = getMarkerPath(directories, repositoryName.strippedName()); this.rule = rule; markerData = Maps.newHashMap(); + + List directoriesList = Ordering.natural().sortedCopy(managedDirectories); + String directoriesString = + directoriesList.stream() + .map(PathFragment::getPathString) + .collect(Collectors.joining(" ")); + markerData.put(MANAGED_DIRECTORIES_MARKER, directoriesString); } byte[] writeMarkerFile() throws RepositoryFunctionException { @@ -396,7 +429,10 @@ byte[] areRepositoryAndMarkerFileConsistent(RepositoryFunction handler, Environm content = FileSystemUtils.readContent(markerPath, StandardCharsets.UTF_8); String markerRuleKey = readMarkerFile(content, markerData); boolean verified = false; - if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) { + if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey) + && Objects.equals( + markerData.get(MANAGED_DIRECTORIES_MARKER), + this.markerData.get(MANAGED_DIRECTORIES_MARKER))) { verified = handler.verifyMarkerData(rule, markerData, env); if (env.valuesMissing()) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java new file mode 100644 index 00000000000000..bbc0eaf894b15f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java @@ -0,0 +1,76 @@ +// Copyright 2019 The Bazel Authors. 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.rules.repository; + +import com.google.common.annotations.VisibleForTesting; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.skyframe.SkyFunctions; +import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; +import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.Objects; +import javax.annotation.Nullable; + +/** + * A dirtiness checker that: + * + *

    + *
  • Only dirties {@link RepositoryDirectoryValue}s, if they were produced in a {@code + * --nofetch} build, so that they are re-created on subsequent {@code --fetch} builds. The + * alternative solution would be to reify the value of the flag as a Skyframe value. + *
  • Dirties repositories, if their managed directories were changed. + *
+ */ +@VisibleForTesting +public class RepositoryDirectoryDirtinessChecker extends SkyValueDirtinessChecker { + private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge; + + public RepositoryDirectoryDirtinessChecker( + ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { + this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; + } + + @Override + public boolean applies(SkyKey skyKey) { + return skyKey.functionName().equals(SkyFunctions.REPOSITORY_DIRECTORY); + } + + @Override + public SkyValue createNewValue(SkyKey key, @Nullable TimestampGranularityMonitor tsgm) { + throw new UnsupportedOperationException(); + } + + @Override + public DirtyResult check( + SkyKey skyKey, SkyValue skyValue, @Nullable TimestampGranularityMonitor tsgm) { + RepositoryName repositoryName = (RepositoryName) skyKey.argument(); + RepositoryDirectoryValue repositoryValue = (RepositoryDirectoryValue) skyValue; + + if (!repositoryValue.repositoryExists()) { + return DirtyResult.notDirty(skyValue); + } + if (repositoryValue.isFetchingDelayed()) { + return DirtyResult.dirty(skyValue); + } + + if (!Objects.equals( + managedDirectoriesKnowledge.getManagedDirectories(repositoryName), + repositoryValue.getManagedDirectories())) { + return DirtyResult.dirty(skyValue); + } + return DirtyResult.notDirty(skyValue); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java index b48e9c6726a5f1..f7c465be3c278c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java @@ -17,6 +17,7 @@ import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Interner; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.concurrent.BlazeInterners; @@ -24,11 +25,13 @@ import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.AbstractSkyKey; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.Arrays; +import java.util.Collection; import java.util.Map; import javax.annotation.Nullable; @@ -50,6 +53,13 @@ public abstract class RepositoryDirectoryValue implements SkyValue { public abstract boolean isFetchingDelayed(); + /** + * Returns the set of relative (to the workspace root) paths to managed directories for this + * repository. We need to keep this information in a value, since managed directories are part of + * the repository definition. + */ + public abstract ImmutableSet getManagedDirectories(); + /** Represents a successful repository lookup. */ public static final class SuccessfulRepositoryDirectoryValue extends RepositoryDirectoryValue { private final Path path; @@ -57,18 +67,21 @@ public static final class SuccessfulRepositoryDirectoryValue extends RepositoryD @Nullable private final byte[] digest; @Nullable private final DirectoryListingValue sourceDir; private final ImmutableMap fileValues; + private final ImmutableSet managedDirectories; private SuccessfulRepositoryDirectoryValue( Path path, boolean fetchingDelayed, DirectoryListingValue sourceDir, byte[] digest, - ImmutableMap fileValues) { + ImmutableMap fileValues, + ImmutableSet managedDirectories) { this.path = path; this.fetchingDelayed = fetchingDelayed; this.sourceDir = sourceDir; this.digest = digest; this.fileValues = fileValues; + this.managedDirectories = managedDirectories; } @Override @@ -86,6 +99,11 @@ public boolean isFetchingDelayed() { return fetchingDelayed; } + @Override + public ImmutableSet getManagedDirectories() { + return managedDirectories; + } + @Override public boolean equals(Object other) { if (this == other) { @@ -97,14 +115,16 @@ public boolean equals(Object other) { return Objects.equal(path, otherValue.path) && Objects.equal(sourceDir, otherValue.sourceDir) && Arrays.equals(digest, otherValue.digest) - && Objects.equal(fileValues, otherValue.fileValues); + && Objects.equal(fileValues, otherValue.fileValues) + && Objects.equal(managedDirectories, otherValue.managedDirectories); } return false; } @Override public int hashCode() { - return Objects.hashCode(path, sourceDir, Arrays.hashCode(digest), fileValues); + return Objects.hashCode( + path, sourceDir, Arrays.hashCode(digest), fileValues, managedDirectories); } @Override @@ -131,6 +151,11 @@ public Path getPath() { public boolean isFetchingDelayed() { throw new IllegalStateException(); } + + @Override + public ImmutableSet getManagedDirectories() { + throw new IllegalStateException(); + } } public static final NoRepositoryDirectoryValue NO_SUCH_REPOSITORY_VALUE = @@ -173,6 +198,7 @@ public static class Builder { private byte[] digest = null; private DirectoryListingValue sourceDir = null; private Map fileValues = ImmutableMap.of(); + private ImmutableSet managedDirectories = ImmutableSet.of(); private Builder() {} @@ -201,6 +227,11 @@ public Builder setFileValues(Map fileValues) { return this; } + public Builder setManagedDirectories(Collection managedDirectories) { + this.managedDirectories = ImmutableSet.copyOf(managedDirectories); + return this; + } + public SuccessfulRepositoryDirectoryValue build() { Preconditions.checkNotNull(path, "Repository path must be specified!"); // Only if fetching is delayed then we are allowed to have a null digest. @@ -208,7 +239,12 @@ public SuccessfulRepositoryDirectoryValue build() { Preconditions.checkNotNull(digest, "Repository marker digest must be specified!"); } return new SuccessfulRepositoryDirectoryValue( - path, fetchingDelayed, sourceDir, digest, ImmutableMap.copyOf(fileValues)); + path, + fetchingDelayed, + sourceDir, + digest, + ImmutableMap.copyOf(fileValues), + managedDirectories); } } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java index 1e9ee5c7ac3c57..bf102732448697 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.profiler.memory.AllocationTracker; import com.google.devtools.build.lib.skyframe.DiffAwareness; +import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutorFactory; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; @@ -52,6 +53,7 @@ public final class WorkspaceBuilder { private final ImmutableList.Builder customDirtinessCheckers = ImmutableList.builder(); private AllocationTracker allocationTracker; + private WorkspaceFileHeaderListener workspaceFileHeaderListener; WorkspaceBuilder(BlazeDirectories directories, BinTools binTools) { this.directories = directories; @@ -79,7 +81,8 @@ BlazeWorkspace build( ruleClassProvider.getBuildInfoFactories(), diffAwarenessFactories.build(), skyFunctions.build(), - customDirtinessCheckers.build()); + customDirtinessCheckers.build(), + workspaceFileHeaderListener); return new BlazeWorkspace( runtime, directories, @@ -153,4 +156,10 @@ public WorkspaceBuilder addCustomDirtinessChecker( this.customDirtinessCheckers.add(Preconditions.checkNotNull(customDirtinessChecker)); return this; } + + public WorkspaceBuilder setWorkspaceFileHeaderListener( + WorkspaceFileHeaderListener workspaceFileHeaderListener) { + this.workspaceFileHeaderListener = workspaceFileHeaderListener; + return this; + } } 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 aba704e7ca13e0..ca668eabbd903a 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 @@ -21,9 +21,11 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.packages.PackageFactory; +import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; +import javax.annotation.Nullable; /** * A factory of SkyframeExecutors that returns SequencedSkyframeExecutor. @@ -46,7 +48,8 @@ public SkyframeExecutor create( ImmutableList buildInfoFactories, Iterable diffAwarenessFactories, ImmutableMap extraSkyFunctions, - Iterable customDirtinessCheckers) { + Iterable customDirtinessCheckers, + @Nullable WorkspaceFileHeaderListener workspaceFileHeaderListener) { return BazelSkyframeExecutorConstants.newBazelSkyframeExecutorBuilder() .setPkgFactory(pkgFactory) .setFileSystem(fileSystem) @@ -58,6 +61,7 @@ public SkyframeExecutor create( .setDiffAwarenessFactories(diffAwarenessFactories) .setExtraSkyFunctions(extraSkyFunctions) .setCustomDirtinessCheckers(customDirtinessCheckers) + .setWorkspaceFileHeaderListener(workspaceFileHeaderListener) .build(); } } 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 index ad852d626845a5..6f51cf3c459081 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java @@ -88,7 +88,7 @@ private DirtyResult(boolean isDirty, @Nullable SkyValue oldValue, this.newValue = newValue; } - boolean isDirty() { + public boolean isDirty() { return isDirty; } 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 3cb5d1178af46f..504e4a45e67662 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 @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory; import com.google.devtools.build.lib.packages.PackageFactory; +import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.skyframe.SkyFunction; @@ -33,15 +34,11 @@ public interface SkyframeExecutorFactory { /** * Creates an instance of SkyframeExecutor * - * @param tsgm timestamp granularity monitor * @param pkgFactory the package factory * @param fileSystem the Blaze file system * @param directories Blaze directories * @param workspaceStatusActionFactory a factory for creating WorkspaceStatusAction objects * @param buildInfoFactories list of BuildInfoFactories - * @param diffAwarenessFactories - * @param extraSkyFunctions - * @param customDirtinessCheckers * @return an instance of the SkyframeExecutor * @throws AbruptExitException if the executor cannot be created */ @@ -54,6 +51,7 @@ SkyframeExecutor create( ImmutableList buildInfoFactories, Iterable diffAwarenessFactories, ImmutableMap extraSkyFunctions, - Iterable customDirtinessCheckers) + Iterable customDirtinessCheckers, + WorkspaceFileHeaderListener workspaceFileHeaderListener) throws AbruptExitException; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index d0476fb2877531..5ff0d78c867e6a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.bazel.rules.BazelRulesModule; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; @@ -114,7 +115,8 @@ httpDownloader, new MavenDownloader(repositoryCache)), new SkylarkRepositoryFunction(httpDownloader), isFetch, ImmutableMap::of, - directories)) + directories, + ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES)) .put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction()) .put(MavenServerFunction.NAME, new MavenServerFunction(directories)) .build()); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index c7084db72f8c2b..3fc0354fe6b57b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.rules.cpp.CcSkyframeFdoSupportValue; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; @@ -131,7 +132,8 @@ public ImmutableMap getSkyFunctions(BlazeDirectori null, new AtomicBoolean(true), ImmutableMap::of, - directories), + directories, + ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES), SkyFunctions.REPOSITORY, new RepositoryLoaderFunction(), CcSkyframeFdoSupportValue.SKYFUNCTION, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java index 4daa6b830ebdb0..af90185ef4b4f9 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; @@ -84,7 +85,8 @@ public ImmutableMap getSkyFunctions( skylarkRepositoryFunction, new AtomicBoolean(true), ImmutableMap::of, - directories); + directories, + ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES); return ImmutableMap.of( SkyFunctions.REPOSITORY_DIRECTORY, function, diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD index a66ac9b4cc8106..7c484fe62920d6 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD @@ -27,8 +27,10 @@ java_test( "//src/main/java/com/google/devtools/build/lib:unix", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/rules/cpp", + "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 02ec42de699df6..c728520dce7918 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -19,16 +19,27 @@ import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ServerDirectories; +import com.google.devtools.build.lib.analysis.util.AnalysisMock; +import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; +import com.google.devtools.build.lib.bazel.repository.skylark.SkylarkRepositoryFunction; +import com.google.devtools.build.lib.bazel.repository.skylark.SkylarkRepositoryModule; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.packages.PackageFactory; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; +import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.SuccessfulRepositoryDirectoryValue; +import com.google.devtools.build.lib.skyframe.ASTFileLookupFunction; import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; +import com.google.devtools.build.lib.skyframe.BlacklistedPackagePrefixesFunction; +import com.google.devtools.build.lib.skyframe.ContainingPackageLookupFunction; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.ExternalPackageFunction; @@ -38,12 +49,16 @@ import com.google.devtools.build.lib.skyframe.PackageFunction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SkyFunctions; +import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction; import com.google.devtools.build.lib.skyframe.WorkspaceASTFunction; import com.google.devtools.build.lib.skyframe.WorkspaceFileFunction; +import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryBootstrap; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.testutil.FoundationTestCase; +import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -60,15 +75,19 @@ import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; import com.google.devtools.build.skyframe.SequentialBuildDriver; import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mockito; /** * Tests for {@link RepositoryDelegatorFunction} @@ -78,32 +97,60 @@ public class RepositoryDelegatorTest extends FoundationTestCase { private RepositoryDelegatorFunction delegatorFunction; private Path overrideDirectory; private SequentialBuildDriver driver; + private ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge; + private RecordingDifferencer differencer; + private TestSkylarkRepositoryFunction testSkylarkRepositoryFunction; + private Path rootPath; @Before public void setupDelegator() throws Exception { - Path root = scratch.dir("/outputbase"); + rootPath = scratch.dir("/outputbase"); BlazeDirectories directories = new BlazeDirectories( - new ServerDirectories(root, root, root), - root, + new ServerDirectories(rootPath, rootPath, rootPath), + rootPath, /* defaultSystemJavabase= */ null, TestConstants.PRODUCT_NAME); + managedDirectoriesKnowledge = new ManagedDirectoriesKnowledgeImpl(); + HttpDownloader downloader = Mockito.mock(HttpDownloader.class); + RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction(); + testSkylarkRepositoryFunction = new TestSkylarkRepositoryFunction(downloader); + ImmutableMap repositoryHandlers = + ImmutableMap.of(LocalRepositoryRule.NAME, localRepositoryFunction); delegatorFunction = new RepositoryDelegatorFunction( - ImmutableMap.of(), null, new AtomicBoolean(true), ImmutableMap::of, directories); + repositoryHandlers, + testSkylarkRepositoryFunction, + new AtomicBoolean(true), + ImmutableMap::of, + directories, + managedDirectoriesKnowledge); AtomicReference pkgLocator = new AtomicReference<>( new PathPackageLocator( - root, - ImmutableList.of(Root.fromPath(root)), + rootPath, + ImmutableList.of(Root.fromPath(rootPath)), BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY)); ExternalFilesHelper externalFilesHelper = ExternalFilesHelper.createForTesting( pkgLocator, ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, directories); - RecordingDifferencer differencer = new SequencedRecordingDifferencer(); - ConfiguredRuleClassProvider ruleClassProvider = - TestRuleClassProvider.getRuleClassProvider(true); + differencer = new SequencedRecordingDifferencer(); + + ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); + TestRuleClassProvider.addStandardRules(builder); + builder + .clearWorkspaceFileSuffixForTesting() + .addSkylarkBootstrap(new RepositoryBootstrap(new SkylarkRepositoryModule())); + ConfiguredRuleClassProvider ruleClassProvider = builder.build(); + + PackageFactory.BuilderForTesting pkgFactoryBuilder = + AnalysisMock.get().getPackageFactoryBuilderForTesting(directories); + SkylarkImportLookupFunction skylarkImportLookupFunction = + new SkylarkImportLookupFunction( + ruleClassProvider, pkgFactoryBuilder.build(ruleClassProvider, fileSystem)); + skylarkImportLookupFunction.resetCache(); + MemoizingEvaluator evaluator = new InMemoryMemoizingEvaluator( ImmutableMap.builder() @@ -121,7 +168,7 @@ public void setupDelegator() throws Exception { .put( SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( - null, + new AtomicReference<>(ImmutableSet.of()), CrossRepositoryLabelViolationStrategy.ERROR, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY)) .put(SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(ruleClassProvider)) @@ -133,29 +180,44 @@ public void setupDelegator() throws Exception { .builder(directories) .build(ruleClassProvider, fileSystem), directories, - /*skylarkImportLookupFunctionForInlining=*/ null)) + skylarkImportLookupFunction)) + .put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction()) .put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction()) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()) + .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) + .put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(ruleClassProvider)) + .put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()) + .put( + SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, + new BlacklistedPackagePrefixesFunction( + /*hardcodedBlacklistedPackagePrefixes=*/ ImmutableSet.of(), + /*additionalBlacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT)) + .put(SkyFunctions.RESOLVED_HASH_VALUES, new ResolvedHashesFunction()) .build(), differencer); driver = new SequentialBuildDriver(evaluator); overrideDirectory = scratch.dir("/foo"); scratch.file("/foo/WORKSPACE"); - RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set( - differencer, - ImmutableMap.builder() - .put(RepositoryName.createFromValidStrippedName("foo"), overrideDirectory.asFragment()) - .build()); + RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of()); RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set( differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY); PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get()); PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT_SEMANTICS); RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE.set( differencer, Optional.absent()); + PrecomputedValue.REPO_ENV.set(differencer, ImmutableMap.of()); + RepositoryDelegatorFunction.OUTPUT_VERIFICATION_REPOSITORY_RULES.set( + differencer, ImmutableSet.of()); + RepositoryDelegatorFunction.RESOLVED_FILE_FOR_VERIFICATION.set(differencer, Optional.absent()); } @Test public void testOverride() throws Exception { + RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set( + differencer, + ImmutableMap.of( + RepositoryName.createFromValidStrippedName("foo"), overrideDirectory.asFragment())); + StoredEventHandler eventHandler = new StoredEventHandler(); SkyKey key = RepositoryDirectoryValue.key(RepositoryName.createFromValidStrippedName("foo")); EvaluationContext evaluationContext = @@ -174,4 +236,148 @@ public void testOverride() throws Exception { assertThat(actualPath.readSymbolicLink()).isEqualTo(overrideDirectory.asFragment()); } + @Test + public void testRepositoryDirtinessChecker() throws Exception { + TimestampGranularityMonitor tsgm = new TimestampGranularityMonitor(new ManualClock()); + ManagedDirectoriesKnowledgeImpl knowledge = new ManagedDirectoriesKnowledgeImpl(); + + RepositoryDirectoryDirtinessChecker checker = + new RepositoryDirectoryDirtinessChecker(knowledge); + RepositoryName repositoryName = RepositoryName.create("@repo"); + RepositoryDirectoryValue.Key key = RepositoryDirectoryValue.key(repositoryName); + + SuccessfulRepositoryDirectoryValue usual = + RepositoryDirectoryValue.builder() + .setPath(rootDirectory.getRelative("a")) + .setDigest(new byte[] {1}) + .build(); + + assertThat(checker.check(key, usual, tsgm).isDirty()).isFalse(); + + SuccessfulRepositoryDirectoryValue fetchDelayed = + RepositoryDirectoryValue.builder() + .setPath(rootDirectory.getRelative("b")) + .setFetchingDelayed() + .build(); + + assertThat(checker.check(key, fetchDelayed, tsgm).isDirty()).isTrue(); + + SuccessfulRepositoryDirectoryValue withManagedDirectories = + RepositoryDirectoryValue.builder() + .setPath(rootDirectory.getRelative("c")) + .setDigest(new byte[] {1}) + .setManagedDirectories(ImmutableSet.of(PathFragment.create("m"))) + .build(); + + assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isTrue(); + + knowledge.setManagedDirectories( + ImmutableMap.of(PathFragment.create("m"), RepositoryName.create("@other"))); + assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isTrue(); + + knowledge.setManagedDirectories(ImmutableMap.of(PathFragment.create("m"), repositoryName)); + assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isFalse(); + } + + @Test + public void testManagedDirectoriesCauseRepositoryReFetches() throws Exception { + scratch.file(rootPath.getRelative("BUILD").getPathString()); + scratch.file( + rootPath.getRelative("repo_rule.bzl").getPathString(), + "def _impl(rctx):", + " rctx.file('BUILD', '')", + "fictive_repo_rule = repository_rule(implementation = _impl)"); + scratch.overwriteFile( + rootPath.getRelative("WORKSPACE").getPathString(), + "workspace(name = 'abc')", + "load(':repo_rule.bzl', 'fictive_repo_rule')", + "fictive_repo_rule(name = 'repo1')"); + + // Managed directories from workspace() attribute will not be parsed by this test, since + // we are not calling SequencedSkyframeExecutor. + // That's why we will directly fill managed directories value (the corresponding structure + // is passed to RepositoryDelegatorFunction during construction). + managedDirectoriesKnowledge.setManagedDirectories( + ImmutableMap.of(PathFragment.create("dir1"), RepositoryName.create("@repo1"))); + + StarlarkSemantics semantics = + StarlarkSemantics.builderWithDefaults() + .experimentalAllowIncrementalRepositoryUpdates(true) + .build(); + PrecomputedValue.STARLARK_SEMANTICS.set(differencer, semantics); + + loadRepo("repo1"); + + assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isTrue(); + testSkylarkRepositoryFunction.reset(); + + loadRepo("repo1"); + assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isFalse(); + testSkylarkRepositoryFunction.reset(); + + managedDirectoriesKnowledge.setManagedDirectories( + ImmutableMap.of( + PathFragment.create("dir1"), + RepositoryName.create("@repo1"), + PathFragment.create("dir2"), + RepositoryName.create("@repo1"))); + loadRepo("repo1"); + + assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isTrue(); + testSkylarkRepositoryFunction.reset(); + + managedDirectoriesKnowledge.setManagedDirectories(ImmutableMap.of()); + loadRepo("repo1"); + + assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isTrue(); + testSkylarkRepositoryFunction.reset(); + } + + private void loadRepo(String strippedRepoName) throws InterruptedException { + StoredEventHandler eventHandler = new StoredEventHandler(); + SkyKey key = + RepositoryDirectoryValue.key(RepositoryName.createFromValidStrippedName(strippedRepoName)); + // Make it be evaluated every time, as we are testing evaluation. + differencer.invalidate(ImmutableSet.of(key)); + EvaluationContext evaluationContext = + EvaluationContext.newBuilder() + .setKeepGoing(false) + .setNumThreads(8) + .setEventHander(eventHandler) + .build(); + EvaluationResult result = driver.evaluate(ImmutableList.of(key), evaluationContext); + assertThat(result.hasError()).isFalse(); + RepositoryDirectoryValue repositoryDirectoryValue = (RepositoryDirectoryValue) result.get(key); + assertThat(repositoryDirectoryValue.repositoryExists()).isTrue(); + } + + private static class TestSkylarkRepositoryFunction extends SkylarkRepositoryFunction { + private boolean fetchCalled = false; + + private TestSkylarkRepositoryFunction(HttpDownloader downloader) { + super(downloader); + } + + public void reset() { + fetchCalled = false; + } + + private boolean isFetchCalled() { + return fetchCalled; + } + + @Nullable + @Override + public RepositoryDirectoryValue.Builder fetch( + Rule rule, + Path outputDirectory, + BlazeDirectories directories, + Environment env, + Map markerData, + SkyKey key) + throws RepositoryFunctionException, InterruptedException { + fetchCalled = true; + return super.fetch(rule, outputDirectory, directories, env, markerData, key); + } + } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index d560875f19462c..b42bcb6307f1d4 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; @@ -154,7 +155,12 @@ public final void setUp() throws Exception { skyFunctions.put( SkyFunctions.REPOSITORY_DIRECTORY, new RepositoryDelegatorFunction( - repositoryHandlers, null, new AtomicBoolean(true), ImmutableMap::of, directories)); + repositoryHandlers, + null, + new AtomicBoolean(true), + ImmutableMap::of, + directories, + ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES)); skyFunctions.put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction()); differencer = new SequencedRecordingDifferencer(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index 022867aef4a638..ec1121052c7001 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction; @@ -164,7 +165,12 @@ public final void setUp() throws Exception { skyFunctions.put( SkyFunctions.REPOSITORY_DIRECTORY, new RepositoryDelegatorFunction( - repositoryHandlers, null, new AtomicBoolean(true), ImmutableMap::of, directories)); + repositoryHandlers, + null, + new AtomicBoolean(true), + ImmutableMap::of, + directories, + ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES)); skyFunctions.put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction()); differencer = new SequencedRecordingDifferencer();