Refresh WorkspaceFileValue for WORKSPACE header, if listener is regis…

…tered.

- 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

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 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor<BuildDrive
   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 @@ private SequencedSkyframeExecutor(
       List<BuildFileName> buildFilesByPriority,
       ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
       BuildOptions defaultBuildOptions,
-      MutableArtifactFactorySupplier mutableArtifactFactorySupplier) {
+      MutableArtifactFactorySupplier mutableArtifactFactorySupplier,
+      @Nullable WorkspaceFileHeaderListener workspaceFileHeaderListener) {
     super(
         skyframeExecutorConsumerOnInit,
         evaluatorSupplier,
@@ -185,6 +196,7 @@ private SequencedSkyframeExecutor(
         /*nonexistentFileReceiver=*/ null);
     this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
     this.customDirtinessCheckers = customDirtinessCheckers;
+    this.workspaceFileHeaderListener = workspaceFileHeaderListener;
   }
 
   @Override
@@ -301,7 +313,8 @@ public void setDeletedPackages(Iterable<PackageIdentifier> pkgs) {
 
   /** 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 @@ public void handleDiffsForTesting(ExtendedEventHandler eventHandler) throws Inte
 
   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 @@ public void dumpPackages(PrintStream out) {
     }
   }
 
+  /**
+   * 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 @@ public static final class Builder {
     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 @@ public SequencedSkyframeExecutor build() {
               buildFilesByPriority,
               actionOnIOExceptionReadingBuildFile,
               defaultBuildOptions,
-              mutableArtifactFactorySupplier);
+              mutableArtifactFactorySupplier,
+              workspaceFileHeaderListener);
       skyframeExecutor.init();
       return skyframeExecutor;
     }
@@ -975,5 +1057,21 @@ public Builder setSkyframeExecutorConsumerOnInit(
       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);
   }
 }

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 @@ public void initializeSkyframeExecutor(boolean doPackageLoadingChecks) throws Ex
                 DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
             .setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
             .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
+            .setWorkspaceFileHeaderListener(getWorkspaceFileListener())
             .build();
     TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
     skyframeExecutor.injectExtraPrecomputedValues(extraPrecomputedValues);
@@ -319,6 +321,10 @@ protected ConfiguredRuleClassProvider getRuleClassProvider() {
     return getAnalysisMock().createRuleClassProvider();
   }
 
+  protected WorkspaceFileHeaderListener getWorkspaceFileListener() {
+    return null;
+  }
+
   protected PackageFactory getPackageFactory() {
     return pkgFactory;
   }

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 @@ private ModifiedFileSet getModifiedFileSet() {
       return builder.build();
     }
 
-    void sync() throws InterruptedException {
+    void sync() throws InterruptedException, AbruptExitException {
       clock.advanceMillis(1);
 
       modifiedFileSet = getModifiedFileSet();

src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java

@@ -25,13 +25,15 @@
 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;
 import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
 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 @@ public class WorkspaceFileFunctionTest extends BuildViewTestCase {
   private ExternalPackageFunction externalSkyFunc;
   private WorkspaceASTFunction astSkyFunc;
   private FakeFileValue fakeWorkspaceFileValue;
+  private TestWorkspaceFileListener testWorkspaceFileListener;
 
   static class FakeFileValue extends FileValue {
     private boolean exists;
@@ -128,6 +132,12 @@ public final void setUp() throws Exception {
     fakeWorkspaceFileValue = new FakeFileValue();
   }
 
+  @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 @@ public void testListBindFunction() throws Exception {
         .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;
+    }
+  }
 }