Refresh WorkspaceFileValue for WORKSPACE header, if listener is registered.
- Define WorkspaceFileHeaderListener as interface with one method, accepting old and new values of WorkspaceFileValue for the WORKSPACE header.
- In SequencedSkyframeExecutor, have WorkspaceFileHeaderListener field and the setter method for it. (In future, set it using WorkspaceBuilder, and expose a method in WorkspaceBuilder for setting.)
- If there is a listener, compute the new value of the WorkspaceFileValue for the WORKSPACE header, if the WORKSPACE file has changed since the previous invocation, and call the listener if the old and new values are different.
- In practice, every time the text of the WORKSPACE file is changed, the WorkspaceFileValue will be the different object, as the evaluation is called; so the listener should additionally check the interesting fields itself.
PiperOrigin-RevId: 244810086
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 8fb6870..08f5fa1 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
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.AnalysisProtos.ActionGraphContainer;
@@ -38,6 +39,7 @@
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.concurrent.Uninterruptibles;
import com.google.devtools.build.lib.events.Event;
@@ -52,6 +54,8 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions;
+import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.profiler.Profiler;
@@ -70,6 +74,7 @@
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.actiongraph.ActionGraphDump;
import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.ResourceUsage;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -78,6 +83,7 @@
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
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.BuildDriver;
import com.google.devtools.build.skyframe.Differencer;
import com.google.devtools.build.skyframe.EvaluationContext;
@@ -93,6 +99,7 @@
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.io.PrintStream;
import java.time.Duration;
import java.util.ArrayList;
@@ -102,6 +109,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
@@ -141,6 +149,8 @@
private Duration sourceDiffCheckingDuration = Duration.ofSeconds(-1L);
private Duration outputTreeDiffCheckingDuration = Duration.ofSeconds(-1L);
+ @Nullable private final WorkspaceFileHeaderListener workspaceFileHeaderListener;
+
private SequencedSkyframeExecutor(
Consumer<SkyframeExecutor> skyframeExecutorConsumerOnInit,
EvaluatorSupplier evaluatorSupplier,
@@ -159,7 +169,8 @@
List<BuildFileName> buildFilesByPriority,
ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
BuildOptions defaultBuildOptions,
- MutableArtifactFactorySupplier mutableArtifactFactorySupplier) {
+ MutableArtifactFactorySupplier mutableArtifactFactorySupplier,
+ @Nullable WorkspaceFileHeaderListener workspaceFileHeaderListener) {
super(
skyframeExecutorConsumerOnInit,
evaluatorSupplier,
@@ -185,6 +196,7 @@
/*nonexistentFileReceiver=*/ null);
this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
this.customDirtinessCheckers = customDirtinessCheckers;
+ this.workspaceFileHeaderListener = workspaceFileHeaderListener;
}
@Override
@@ -301,7 +313,8 @@
/** Uses diff awareness on all the package paths to invalidate changed files. */
@VisibleForTesting
- public void handleDiffsForTesting(ExtendedEventHandler eventHandler) throws InterruptedException {
+ public void handleDiffsForTesting(ExtendedEventHandler eventHandler)
+ throws InterruptedException, AbruptExitException {
if (super.lastAnalysisDiscarded) {
// Values were cleared last build, but they couldn't be deleted because they were needed for
// the execution phase. We can delete them now.
@@ -313,9 +326,14 @@
private void handleDiffs(
ExtendedEventHandler eventHandler, boolean checkOutputFiles, OptionsProvider options)
- throws InterruptedException {
+ throws InterruptedException, AbruptExitException {
TimestampGranularityMonitor tsgm = this.tsgm.get();
modifiedFiles = 0;
+
+ if (workspaceFileHeaderListener != null) {
+ refreshWorkspaceHeader(eventHandler);
+ }
+
Map<Root, DiffAwarenessManager.ProcessableModifiedFileSet> modifiedFilesByPathEntry =
Maps.newHashMap();
Set<Pair<Root, DiffAwarenessManager.ProcessableModifiedFileSet>>
@@ -803,6 +821,68 @@
}
}
+ /**
+ * Calculate the new value of the WORKSPACE file header (WorkspaceFileValue with the index = 0),
+ * and call the listener, if the value has changed. Needed for incremental update of user-owned
+ * directories by repository rules.
+ */
+ private void refreshWorkspaceHeader(ExtendedEventHandler eventHandler)
+ throws InterruptedException, AbruptExitException {
+ Preconditions.checkNotNull(workspaceFileHeaderListener);
+
+ Root workspaceRoot = Root.fromPath(directories.getWorkspace());
+ RootedPath workspacePath =
+ RootedPath.toRootedPath(workspaceRoot, LabelConstants.WORKSPACE_FILE_NAME);
+ WorkspaceFileKey workspaceFileKey = WorkspaceFileValue.key(workspacePath);
+
+ WorkspaceFileValue oldValue =
+ (WorkspaceFileValue) memoizingEvaluator.getExistingValue(workspaceFileKey);
+ maybeInvalidateWorkspaceFileStateValue(workspacePath);
+ WorkspaceFileValue newValue =
+ (WorkspaceFileValue) evaluateSingleValue(workspaceFileKey, eventHandler);
+ if (!Objects.equals(newValue, oldValue)) {
+ workspaceFileHeaderListener.workspaceHeaderChanged(newValue);
+ }
+ }
+
+ // We only check the FileStateValue of the WORKSPACE file; we do not support the case
+ // when the WORKSPACE file is a symlink.
+ private void maybeInvalidateWorkspaceFileStateValue(RootedPath workspacePath)
+ throws InterruptedException, AbruptExitException {
+ SkyKey workspaceFileStateKey = FileStateValue.key(workspacePath);
+ SkyValue oldWorkspaceFileState = memoizingEvaluator.getExistingValue(workspaceFileStateKey);
+ if (oldWorkspaceFileState == null) {
+ // no need to invalidate if not cached
+ return;
+ }
+ FileStateValue newWorkspaceFileState;
+ try {
+ newWorkspaceFileState = FileStateValue.create(workspacePath, tsgm.get());
+ if (FileStateType.SYMLINK.equals(newWorkspaceFileState.getType())) {
+ throw new AbruptExitException(
+ "WORKSPACE file can not be a symlink if incrementally"
+ + " updated directories feature is enabled.",
+ ExitCode.PARSING_FAILURE);
+ }
+ } catch (IOException e) {
+ throw new AbruptExitException("Can not read WORKSPACE file.", ExitCode.PARSING_FAILURE, e);
+ }
+ if (!oldWorkspaceFileState.equals(newWorkspaceFileState)) {
+ recordingDiffer.invalidate(ImmutableSet.of(workspaceFileStateKey));
+ }
+ }
+
+ private SkyValue evaluateSingleValue(SkyKey key, ExtendedEventHandler eventHandler)
+ throws InterruptedException {
+ EvaluationContext evaluationContext =
+ EvaluationContext.newBuilder()
+ .setKeepGoing(false)
+ .setNumThreads(DEFAULT_THREAD_COUNT)
+ .setEventHander(eventHandler)
+ .build();
+ return buildDriver.evaluate(ImmutableSet.of(key), evaluationContext).get(key);
+ }
+
public static Builder builder() {
return new Builder();
}
@@ -827,6 +907,7 @@
private CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy;
private List<BuildFileName> buildFilesByPriority;
private ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile;
+ private WorkspaceFileHeaderListener workspaceFileHeaderListener;
// Fields with default values.
private ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions = ImmutableMap.of();
@@ -872,7 +953,8 @@
buildFilesByPriority,
actionOnIOExceptionReadingBuildFile,
defaultBuildOptions,
- mutableArtifactFactorySupplier);
+ mutableArtifactFactorySupplier,
+ workspaceFileHeaderListener);
skyframeExecutor.init();
return skyframeExecutor;
}
@@ -975,5 +1057,21 @@
this.skyframeExecutorConsumerOnInit = skyframeExecutorConsumerOnInit;
return this;
}
+
+ public Builder setWorkspaceFileHeaderListener(
+ WorkspaceFileHeaderListener workspaceFileHeaderListener) {
+ this.workspaceFileHeaderListener = workspaceFileHeaderListener;
+ return this;
+ }
+ }
+
+ /**
+ * Listener class to subscribe for WORKSPACE file header changes.
+ *
+ * <p>Changes to WORKSPACE file header are computed before the files difference is computed in
+ * {@link #handleDiffs(ExtendedEventHandler, boolean, OptionsProvider)}
+ */
+ public interface WorkspaceFileHeaderListener {
+ void workspaceHeaderChanged(@Nullable WorkspaceFileValue newValue);
}
}
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 a8ee477..9df4bda 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
@@ -140,6 +140,7 @@
import com.google.devtools.build.lib.skyframe.PackageRootsNoSymlinkCreation;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
@@ -279,6 +280,7 @@
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
+ .setWorkspaceFileHeaderListener(getWorkspaceFileListener())
.build();
TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
skyframeExecutor.injectExtraPrecomputedValues(extraPrecomputedValues);
@@ -319,6 +321,10 @@
return getAnalysisMock().createRuleClassProvider();
}
+ protected WorkspaceFileHeaderListener getWorkspaceFileListener() {
+ return null;
+ }
+
protected PackageFactory getPackageFactory() {
return pkgFactory;
}
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
index 1e392e2..620a77e00 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
@@ -48,6 +48,7 @@
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
@@ -581,7 +582,7 @@
return builder.build();
}
- void sync() throws InterruptedException {
+ void sync() throws InterruptedException, AbruptExitException {
clock.advanceMillis(1);
modifiedFileSet = getModifiedFileSet();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 540f760..e8754f7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
@@ -32,6 +33,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.testutil.MoreAsserts;
@@ -47,6 +49,7 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
+import javax.annotation.Nullable;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.junit.Before;
@@ -68,6 +71,7 @@
private ExternalPackageFunction externalSkyFunc;
private WorkspaceASTFunction astSkyFunc;
private FakeFileValue fakeWorkspaceFileValue;
+ private TestWorkspaceFileListener testWorkspaceFileListener;
static class FakeFileValue extends FileValue {
private boolean exists;
@@ -129,6 +133,12 @@
}
@Override
+ protected WorkspaceFileHeaderListener getWorkspaceFileListener() {
+ testWorkspaceFileListener = new TestWorkspaceFileListener();
+ return testWorkspaceFileListener;
+ }
+
+ @Override
protected Iterable<EnvironmentExtension> getEnvironmentExtensions() {
return ImmutableList.<EnvironmentExtension>of(new PackageFactory.EmptyEnvironmentExtension());
}
@@ -389,4 +399,42 @@
.isEqualTo(Label.parseAbsolute("//foo:bar", ImmutableMap.of()));
MoreAsserts.assertNoEvents(pkg.getEvents());
}
+
+ @Test
+ public void testWorkspaceFileValueListener() throws Exception {
+ // Normally, syscalls cache is reset in the sync() method of the SkyframeExecutor, before
+ // diffing.
+ // But here we are calling only actual diffing part, exposed for testing:
+ // handleDiffsForTesting(), so we better turn off the syscalls cache.
+ skyframeExecutor.turnOffSyscallCacheForTesting();
+
+ createWorkspaceFile("workspace(name = 'old')");
+ skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE);
+ assertThat(testWorkspaceFileListener.getLastWorkspaceName()).isEqualTo("old");
+ assertThat(testWorkspaceFileListener.getCnt()).isEqualTo(1);
+
+ createWorkspaceFile("workspace(name = 'changed')");
+ skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE);
+ assertThat(testWorkspaceFileListener.getLastWorkspaceName()).isEqualTo("changed");
+ assertThat(testWorkspaceFileListener.getCnt()).isEqualTo(2);
+ }
+
+ private static class TestWorkspaceFileListener implements WorkspaceFileHeaderListener {
+ private String lastWorkspaceName;
+ private int cnt = 0;
+
+ @Override
+ public void workspaceHeaderChanged(@Nullable WorkspaceFileValue newValue) {
+ ++cnt;
+ lastWorkspaceName = newValue != null ? newValue.getPackage().getWorkspaceName() : null;
+ }
+
+ private String getLastWorkspaceName() {
+ return lastWorkspaceName;
+ }
+
+ private int getCnt() {
+ return cnt;
+ }
+ }
}
