Let blaze modules control source root virtualization.

Similar to how execroot virtualization is already handled. PiperOrigin-RevId: 641902670 Change-Id: I40690b61b9ba52d824bdad4c5b121a10033df78d
bazelbuild · Jun 10, 2024 · 647371a · 647371a
1 parent bc1bc57
commit 647371a
Show file tree

Hide file tree

Showing 10 changed files with 121 additions and 45 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -556,7 +556,6 @@ java_library(
         ":server_directories",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
-        "//src/main/java/com/google/devtools/build/lib/util:string",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//third_party:guava",
         "//third_party:jsr305",

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java
@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.Root;
 import javax.annotation.Nullable;
 
 /**
@@ -215,7 +216,7 @@ public Path getActionTempsDirectory(Path execRoot) {
 
   /** Returns the installed embedded binaries directory, under the shared installBase location. */
   public Path getEmbeddedBinariesRoot() {
-    return serverDirectories.getEmbeddedBinariesRoot();
+    return getInstallBase();
   }
 
   /**
@@ -256,4 +257,10 @@ public static String getRelativeOutputPath(String productName) {
   public String getProductName() {
     return productName;
   }
+
+  /** Convenience method for {@link ServerDirectories#getVirtualSourceRoot}. */
+  @Nullable
+  public Root getVirtualSourceRoot() {
+    return serverDirectories.getVirtualSourceRoot();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java
@@ -14,13 +14,14 @@
 
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.google.common.base.Strings;
 import com.google.common.hash.HashCode;
 import com.google.common.hash.Hashing;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.Root;
 import javax.annotation.Nullable;
 
 /**
@@ -33,6 +34,7 @@ public final class ServerDirectories {
 
   /** Where Bazel gets unpacked. */
   private final Path installBase;
+
   /** The content hash of everything in installBase. */
   @Nullable private final HashCode installMD5;
 
@@ -43,6 +45,7 @@ public final class ServerDirectories {
   private final Path outputUserRoot;
 
   private final Path execRootBase;
+  @Nullable private final Root virtualSourceRoot; // Null if the source root is not virtualized.
 
   // TODO(bazel-team): Use a builder to simplify/unify these constructors. This makes it easier to
   // have sensible defaults, e.g. execRootBase = outputBase + "/execroot". Then reorder the fields
@@ -53,26 +56,36 @@ public ServerDirectories(
       Path outputBase,
       Path outputUserRoot,
       Path execRootBase,
+      @Nullable Root virtualSourceRoot,
       @Nullable String installMD5) {
     this.installBase = installBase;
-    this.installMD5 =
-        Strings.isNullOrEmpty(installMD5) ? null : checkMD5(HashCode.fromString(installMD5));
+    this.installMD5 = toMD5HashCode(installMD5);
     this.outputBase = outputBase;
     this.outputUserRoot = outputUserRoot;
     this.execRootBase = execRootBase;
+    this.virtualSourceRoot = virtualSourceRoot;
   }
 
   public ServerDirectories(Path installBase, Path outputBase, Path outputUserRoot) {
     this(
         // Some tests set installBase to null.
         // TODO(bazel-team): Be more consistent about whether nulls are permitted. (e.g. equals()
         // presently doesn't tolerate them for some fields). We should probably just disallow them.
-        installBase, outputBase, outputUserRoot, outputBase.getRelative(EXECROOT), null);
+        installBase,
+        outputBase,
+        outputUserRoot,
+        outputBase.getRelative(EXECROOT),
+        /* virtualSourceRoot= */ null,
+        /* installMD5= */ null);
   }
 
-  private static HashCode checkMD5(HashCode hash) {
-    Preconditions.checkArgument(
-        hash.bits() == Hashing.md5().bits(), "Hash '%s' has %s bits", hash, hash.bits());
+  @Nullable
+  private static HashCode toMD5HashCode(@Nullable String installMD5) {
+    if (Strings.isNullOrEmpty(installMD5)) {
+      return null;
+    }
+    HashCode hash = HashCode.fromString(installMD5);
+    checkArgument(hash.bits() == Hashing.md5().bits(), "Hash '%s' has %s bits", hash, hash.bits());
     return hash;
   }
 
@@ -107,19 +120,29 @@ public Path getOutputUserRoot() {
   /**
    * Parent of all execution roots.
    *
-   * <p>This is physically, always /outputbase/execroot, but might be virtualized.
+   * <p>By default, this is a folder called {@linkplain #EXECROOT execroot} in {@link
+   * #getOutputBase}. However, some {@link com.google.devtools.build.lib.vfs.FileSystem}
+   * implementations may choose to virtualize the execroot (in other words, it is not a real on-disk
+   * path, but one that the {@link com.google.devtools.build.lib.vfs.FileSystem} recognizes).
+   *
+   * <p>This is virtual if and only if {@link #getVirtualSourceRoot} is present.
    */
   public Path getExecRootBase() {
     return execRootBase;
   }
 
-  /** Returns the installed embedded binaries directory, under the shared installBase location. */
-  public Path getEmbeddedBinariesRoot() {
-    return getEmbeddedBinariesRoot(installBase);
-  }
-
-  @VisibleForTesting
-  public static Path getEmbeddedBinariesRoot(Path installBase) {
-    return installBase;
+  /**
+   * Returns a stable, virtual root that (if present) should be used as the effective package path
+   * for all commands during the server's lifetime.
+   *
+   * <p>If present, the server's {@link com.google.devtools.build.lib.vfs.FileSystem} is responsible
+   * for translating paths under this root to the actual requested {@code --package_path} for a
+   * given command.
+   *
+   * <p>Present if and only if {@link #getExecRootBase} is virtualized.
+   */
+  @Nullable
+  public Root getVirtualSourceRoot() {
+    return virtualSourceRoot;
   }
 }
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
@@ -46,9 +46,11 @@
 import com.google.devtools.build.lib.vfs.OutputService;
 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.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingResult;
 import com.google.devtools.common.options.OptionsProvider;
+import java.util.Optional;
 import java.util.UUID;
 import javax.annotation.Nullable;
 
@@ -107,17 +109,31 @@ public FileSystem getFileSystemForBuildArtifacts(FileSystem fileSystem) {
   public abstract static class ModuleFileSystem {
     public abstract FileSystem fileSystem();
 
-    /** Non-null if this filesystem virtualizes the execroot folder. */
-    @Nullable
-    abstract Path virtualExecRootBase();
+    /**
+     * Present if this filesystem virtualizes the source root. See {@link
+     * ServerDirectories#getVirtualSourceRoot}.
+     */
+    abstract Optional<Root> virtualSourceRoot();
 
-    public static ModuleFileSystem create(
-        FileSystem fileSystem, @Nullable Path virtualExecRootBase) {
-      return new AutoValue_BlazeModule_ModuleFileSystem(fileSystem, virtualExecRootBase);
+    /**
+     * Present if this filesystem virtualizes the execroot folder. See {@link
+     * ServerDirectories#getExecRootBase}.
+     */
+    abstract Optional<Path> virtualExecRootBase();
+
+    public static ModuleFileSystem createWithVirtualization(
+        FileSystem fileSystem, PathFragment virtualSourceRoot, PathFragment virtualExecRootBase) {
+      return new AutoValue_BlazeModule_ModuleFileSystem(
+          fileSystem,
+          Optional.of(Root.fromPath(fileSystem.getPath(virtualSourceRoot))),
+          Optional.of(fileSystem.getPath(virtualExecRootBase)));
     }
 
     public static ModuleFileSystem create(FileSystem fileSystem) {
-      return create(fileSystem, /*virtualExecRootBase=*/ null);
+      return new AutoValue_BlazeModule_ModuleFileSystem(
+          fileSystem,
+          /* virtualSourceRoot= */ Optional.empty(),
+          /* virtualExecRootBase= */ Optional.empty());
     }
   }
 

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
@@ -74,6 +74,7 @@
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
 import com.google.devtools.build.lib.query2.query.output.OutputFormatter;
 import com.google.devtools.build.lib.query2.query.output.OutputFormatters;
+import com.google.devtools.build.lib.runtime.BlazeModule.ModuleFileSystem;
 import com.google.devtools.build.lib.runtime.CommandDispatcher.LockingMode;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.server.CommandProtos.EnvironmentVariable;
@@ -102,6 +103,7 @@
 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.worker.WorkerProcessMetricsCollector;
 import com.google.devtools.common.options.CommandNameCache;
 import com.google.devtools.common.options.InvocationPolicyParser;
@@ -1172,6 +1174,7 @@ private static BlazeRuntime newRuntime(
     PathFragment outputUserRoot = startupOptions.outputUserRoot;
     PathFragment installBase = startupOptions.installBase;
     PathFragment outputBase = startupOptions.outputBase;
+    PathFragment realExecRootBase = outputBase.getRelative(ServerDirectories.EXECROOT);
 
     maybeForceJNIByGettingPid(installBase); // Must be before first use of JNI.
 
@@ -1192,14 +1195,15 @@ private static BlazeRuntime newRuntime(
     }
 
     FileSystem nativeFs = null;
-    Path execRootBasePath = null;
+    Optional<Root> virtualSourceRoot = Optional.empty();
+    Optional<Path> virtualExecRootBase = Optional.empty();
     for (BlazeModule module : blazeModules) {
-      BlazeModule.ModuleFileSystem moduleFs =
-          module.getFileSystem(options, outputBase.getRelative(ServerDirectories.EXECROOT));
+      ModuleFileSystem moduleFs = module.getFileSystem(options, realExecRootBase);
       if (moduleFs != null) {
-        execRootBasePath = moduleFs.virtualExecRootBase();
         Preconditions.checkState(nativeFs == null, "more than one module returns a file system");
         nativeFs = moduleFs.fileSystem();
+        virtualSourceRoot = moduleFs.virtualSourceRoot();
+        virtualExecRootBase = moduleFs.virtualExecRootBase();
       }
     }
 
@@ -1264,9 +1268,7 @@ private static BlazeRuntime newRuntime(
     Path outputUserRootPath = fs.getPath(outputUserRoot);
     Path installBasePath = fs.getPath(installBase);
     Path outputBasePath = fs.getPath(outputBase);
-    if (execRootBasePath == null) {
-      execRootBasePath = outputBasePath.getRelative(ServerDirectories.EXECROOT);
-    }
+    Path execRootBasePath = virtualExecRootBase.orElseGet(() -> fs.getPath(realExecRootBase));
     Path workspaceDirectoryPath = null;
     if (!workspaceDirectory.equals(PathFragment.EMPTY_FRAGMENT)) {
       workspaceDirectoryPath = nativeFs.getPath(workspaceDirectory);
@@ -1282,6 +1284,7 @@ private static BlazeRuntime newRuntime(
             outputBasePath,
             outputUserRootPath,
             execRootBasePath,
+            virtualSourceRoot.orElse(null),
             startupOptions.installMD5);
     Clock clock = BlazeClock.instance();
     BlazeRuntime.Builder runtimeBuilder =

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
@@ -137,6 +137,7 @@
 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.Symlinks;
 import com.google.devtools.build.lib.vfs.util.FileSystems;
 import com.google.devtools.build.lib.worker.WorkerModule;
@@ -265,6 +266,7 @@ public final void createFilesAndMocks() throws Exception {
             /* outputBase= */ outputBase,
             /* outputUserRoot= */ outputBase,
             /* execRootBase= */ getExecRootBase(),
+            /* virtualSourceRoot= */ getVirtualSourceRoot(),
             // Arbitrary install base hash.
             /* installMD5= */ "83bc4458738962b9b77480bac76164a9");
     directories =
@@ -335,8 +337,14 @@ protected UncaughtExceptionHandler createUncaughtExceptionHandler() {
         BugReport.handleCrash(Crash.from(exception), CrashContext.keepAlive());
   }
 
+  @ForOverride
+  @Nullable
+  protected Root getVirtualSourceRoot() {
+    return null;
+  }
+
   protected Path getExecRootBase() {
-    return outputBase.getRelative("execroot");
+    return outputBase.getRelative(ServerDirectories.EXECROOT);
   }
 
   protected void createRuntimeWrapper() throws Exception {

diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java
@@ -78,6 +78,7 @@
 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.SyscallCache;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
@@ -123,10 +124,15 @@ public void setUp() throws Exception {
     analysisMock = AnalysisMock.get();
     rootDirectory = createDir(getRootDirectoryNameForSetup());
     outputBase = createDir(fileSystem.getPath("/output").getPathString());
-    rootDirectory = createDir(getRootDirectoryNameForSetup());
     directories =
         new BlazeDirectories(
-            new ServerDirectories(rootDirectory, outputBase, outputBase),
+            new ServerDirectories(
+                rootDirectory,
+                outputBase,
+                outputBase,
+                outputBase.getRelative(ServerDirectories.EXECROOT),
+                useVirtualSourceRoot() ? Root.fromPath(rootDirectory) : null,
+                /* installMD5= */ null),
             rootDirectory,
             /* defaultSystemJavabase= */ null,
             analysisMock.getProductName());
@@ -156,6 +162,11 @@ public final void cleanUp() {
 
   protected abstract String getRootDirectoryNameForSetup();
 
+  @ForOverride
+  protected boolean useVirtualSourceRoot() {
+    return false;
+  }
+
   protected abstract void performAdditionalClientSetup(MockToolsConfig mockToolsConfig)
       throws IOException;
 

diff --git a/...ava/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java b/...ava/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java
@@ -57,6 +57,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.common.options.Options;
+import com.google.errorprone.annotations.ForOverride;
 import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
@@ -91,10 +92,13 @@ public void setUp() throws IOException {
             new ServerDirectories(
                 fileSystem.getPath("/install"),
                 fileSystem.getPath("/output"),
-                fileSystem.getPath("/user_root")),
+                fileSystem.getPath("/user_root"),
+                fileSystem.getPath("/execroot"),
+                useVirtualSourceRoot() ? root : null,
+                null),
             workingDir,
-            /*defaultSystemJavabase=*/ null,
-            /*productName=*/ "DummyProductNameForUnitTests");
+            /* defaultSystemJavabase= */ null,
+            /* productName= */ "DummyProductNameForUnitTests");
     eventCollector = new EventCollector();
     reporter = new Reporter(new EventBus());
     reporter.addHandler(eventCollector);
@@ -108,6 +112,9 @@ public void setUp() throws IOException {
 
   protected abstract SkyframeExecutorFactory makeSkyframeExecutorFactory();
 
+  @ForOverride
+  protected abstract boolean useVirtualSourceRoot();
+
   @Test
   public void noPackageErrors() throws Exception {
     initEvaluator();

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryTest.java
@@ -26,6 +26,7 @@
 @RunWith(JUnit4.class)
 public final class CollectPackagesUnderDirectoryTest
     extends AbstractCollectPackagesUnderDirectoryTest {
+
   @Override
   protected String getWorkspacePathString() {
     return "/workspace";
@@ -50,4 +51,9 @@ protected ImmutableMap<SkyFunctionName, SkyFunction> getExtraSkyFunctions() {
   protected SkyframeExecutorFactory makeSkyframeExecutorFactory() {
     return new SequencedSkyframeExecutorFactory();
   }
+
+  @Override
+  protected boolean useVirtualSourceRoot() {
+    return false;
+  }
 }