Attempt to fix test working directory issue

src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java

@@ -49,6 +49,9 @@ interface RunfilesTree {
 
     /** Returns whether the runfile symlinks should be materialized during the build. */
     boolean isBuildRunfileLinks();
+
+    /** Returns the name of the workspace that the build is occurring in. */
+    PathFragment getWorkspaceName();
   }
 
   /** Returns the runfiles trees to be materialized on the inputs of the action. */

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -343,6 +343,7 @@ public NestedSet<Artifact> getRunfilesArtifacts() {
   }
 
   /** Returns the name of the workspace that the build is occurring in. */
+  @Override
   public PathFragment getWorkspaceName() {
     return runfiles.getSuffix();
   }

src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java

@@ -166,6 +166,11 @@ public boolean isBuildRunfileLinks() {
     return buildRunfileLinks;
   }
 
+  @Override
+  public PathFragment getWorkspaceName() {
+    return runfiles.getSuffix();
+  }
+
   /** Softly caches the result of {@link Runfiles#getRunfilesInputs}. */
   private static final class RunfilesCacher implements Supplier<Map<PathFragment, Artifact>> {
 

src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java

@@ -116,15 +116,11 @@ public synchronized void cancelOthers() {
    * Ensures that all directories used to run test are in the correct state and their content will
    * not result in stale files.
    */
-  protected void prepareFileSystem(
-      TestRunnerAction testAction, Path execRoot, Path tmpDir, Path workingDirectory)
+  protected void prepareFileSystem(TestRunnerAction testAction, Path execRoot, Path tmpDir)
       throws IOException {
     if (tmpDir != null) {
       recreateDirectory(tmpDir);
     }
-    if (workingDirectory != null) {
-      workingDirectory.createDirectoryAndParents();
-    }
 
     ResolvedPaths resolvedPaths = testAction.resolve(execRoot);
     if (testAction.isCoverageMode()) {
@@ -142,7 +138,7 @@ protected void prepareFileSystem(
    * not result in stale files. Only use this if no local tmp and working directory are required.
    */
   protected void prepareFileSystem(TestRunnerAction testAction, Path execRoot) throws IOException {
-    prepareFileSystem(testAction, execRoot, null, null);
+    prepareFileSystem(testAction, execRoot, null);
   }
 
   /** Removes directory if it exists and recreates it. */

src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java

@@ -181,7 +181,10 @@ public class ExecutionTool {
     }
     actionContextRegistryBuilder.register(
         SymlinkTreeActionContext.class,
-        new SymlinkTreeStrategy(env.getOutputService(), env.getBlazeWorkspace().getBinTools()));
+        new SymlinkTreeStrategy(
+            env.getOutputService(),
+            env.getBlazeWorkspace().getBinTools(),
+            PathFragment.create(env.getWorkspaceName())));
     // TODO(philwo) - the ExecutionTool should not add arbitrary dependencies on its own, instead
     // these dependencies should be added to the ActionContextConsumer of the module that actually
     // depends on them.

src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java

@@ -150,11 +150,12 @@ private void updateRunfilesTree(
     }
 
     SymlinkTreeHelper helper =
-        new SymlinkTreeHelper(inputManifest, runfilesDirPath, /* filesetTree= */ false);
+        new SymlinkTreeHelper(
+            inputManifest, runfilesDirPath, /* filesetTree= */ false, tree.getWorkspaceName());
 
     switch (tree.getSymlinksMode()) {
       case SKIP:
-        helper.clearAndLinkManifest();
+        helper.clearRunfilesDirectory();
         break;
       case EXTERNAL:
         helper.createSymlinksUsingCommand(execRoot, binTools, env, outErr);

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -139,8 +139,7 @@ public TestRunnerSpawn createTestRunnerSpawn(
     Path runfilesDir = pathResolver.convertPath(action.getExecutionSettings().getRunfilesDir());
     Path tmpDir = pathResolver.convertPath(tmpDirRoot.getChild(TestStrategy.getTmpDirName(action)));
     Path workingDirectory = runfilesDir.getRelative(action.getRunfilesPrefix());
-    return new StandaloneTestRunnerSpawn(
-        action, actionExecutionContext, spawn, tmpDir, workingDirectory, execRoot);
+    return new StandaloneTestRunnerSpawn(action, actionExecutionContext, spawn, tmpDir, execRoot);
   }
 
   private static ImmutableList<Pair<String, Path>> renameOutputs(
@@ -556,21 +555,18 @@ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn {
     private final ActionExecutionContext actionExecutionContext;
     private final Spawn spawn;
     private final Path tmpDir;
-    private final Path workingDirectory;
     private final Path execRoot;
 
     StandaloneTestRunnerSpawn(
         TestRunnerAction testAction,
         ActionExecutionContext actionExecutionContext,
         Spawn spawn,
         Path tmpDir,
-        Path workingDirectory,
         Path execRoot) {
       this.testAction = testAction;
       this.actionExecutionContext = actionExecutionContext;
       this.spawn = spawn;
       this.tmpDir = tmpDir;
-      this.workingDirectory = workingDirectory;
       this.execRoot = execRoot;
     }
 
@@ -581,7 +577,7 @@ public ActionExecutionContext getActionExecutionContext() {
 
     @Override
     public TestAttemptResult execute() throws InterruptedException, IOException, ExecException {
-      prepareFileSystem(testAction, execRoot, tmpDir, workingDirectory);
+      prepareFileSystem(testAction, execRoot, tmpDir);
       return beginTestAttempt(testAction, spawn, actionExecutionContext, execRoot);
     }
 

src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java

@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.exec;
 
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -57,6 +56,7 @@ public final class SymlinkTreeHelper {
   private final Path inputManifest;
   private final Path symlinkTreeRoot;
   private final boolean filesetTree;
+  private final PathFragment workspaceName;
 
   /**
    * Creates SymlinkTreeHelper instance. Can be used independently of SymlinkTreeAction.
@@ -65,11 +65,14 @@ public final class SymlinkTreeHelper {
    * @param symlinkTreeRoot the root of the symlink tree to be created
    * @param filesetTree true if this is fileset symlink tree, false if this is a runfiles symlink
    *     tree.
+   * @param workspaceName the name of the workspace, used to create the workspace subdirectory
    */
-  public SymlinkTreeHelper(Path inputManifest, Path symlinkTreeRoot, boolean filesetTree) {
+  public SymlinkTreeHelper(
+      Path inputManifest, Path symlinkTreeRoot, boolean filesetTree, PathFragment workspaceName) {
     this.inputManifest = inputManifest;
     this.symlinkTreeRoot = symlinkTreeRoot;
     this.filesetTree = filesetTree;
+    this.workspaceName = workspaceName;
   }
 
   private Path getOutputManifest() {
@@ -118,20 +121,26 @@ public void createSymlinksDirectly(Path symlinkTreeRoot, Map<PathFragment, Artif
         parentDir.addSymlink(entry.getKey().getBaseName(), entry.getValue());
       }
       root.syncTreeRecursively(symlinkTreeRoot);
+      createWorkspaceSubdirectory();
     }
   }
 
   /**
-   * Ensures that the runfiles directory is empty except for the symlinked MANIFEST. This is the
-   * expected state with --noenable_runfiles.
+   * Ensures that the runfiles directory is empty except for the symlinked MANIFEST and the
+   * workspace subdirectory. This is the expected state with --noenable_runfiles.
    */
-  public void clearAndLinkManifest() throws ExecException {
-    clearRunfilesDirectory();
+  public void clearRunfilesDirectory() throws ExecException {
+    deleteRunfilesDirectory();
     linkManifest();
+    try {
+      createWorkspaceSubdirectory();
+    } catch (IOException e) {
+      throw new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION);
+    }
   }
 
   /** Deletes the contents of the runfiles directory. */
-  private void clearRunfilesDirectory() throws ExecException {
+  private void deleteRunfilesDirectory() throws ExecException {
     try (SilentCloseable c = Profiler.instance().profile("Clear symlink tree")) {
       symlinkTreeRoot.deleteTreesBelow();
     } catch (IOException e) {
@@ -152,6 +161,14 @@ private void linkManifest() throws ExecException {
     }
   }
 
+  private void createWorkspaceSubdirectory() throws IOException {
+    // Always create the subdirectory corresponding to the workspace (i.e., the main repository).
+    // This is required by tests as their working directory, even with --noenable_runfiles. But if
+    // the test action creates the directory and then proceeds to execute the test spawn, this logic
+    // would remove it. For the sake of consistency, always create the directory instead.
+    symlinkTreeRoot.getRelative(workspaceName).createDirectory();
+  }
+
   /**
    * Creates a symlink tree using a CommandBuilder. This means that the symlink tree will always be
    * present on the developer's workstation. Useful when running commands locally.
@@ -179,6 +196,11 @@ public void createSymlinksUsingCommand(
                     Execution.newBuilder().setCode(Code.SYMLINK_TREE_CREATION_COMMAND_EXCEPTION))
                 .build());
       }
+      try {
+        createWorkspaceSubdirectory();
+      } catch (IOException e) {
+        throw new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION);
+      }
     }
   }
 

src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java

@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.exec;
 
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
@@ -56,10 +55,13 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext {
 
   private final OutputService outputService;
   private final BinTools binTools;
+  private final PathFragment workspaceName;
 
-  public SymlinkTreeStrategy(OutputService outputService, BinTools binTools) {
+  public SymlinkTreeStrategy(
+      OutputService outputService, BinTools binTools, PathFragment workspaceName) {
     this.outputService = outputService;
     this.binTools = binTools;
+    this.workspaceName = workspaceName;
   }
 
   @Override
@@ -97,15 +99,14 @@ public void createSymlinks(
           }
 
           outputService.createSymlinkTree(
-              symlinks,
-              action.getOutputManifest().getExecPath().getParentDirectory());
+              symlinks, action.getOutputManifest().getExecPath().getParentDirectory());
 
           createOutput(action, actionExecutionContext, inputManifest);
         } else if (action.getRunfileSymlinksMode() == RunfileSymlinksMode.SKIP) {
           // Delete symlinks possibly left over by a previous invocation with a different mode.
           // This is required because only the output manifest is considered an action output, so
           // Skyframe does not clear the directory for us.
-          createSymlinkTreeHelper(action, actionExecutionContext).clearAndLinkManifest();
+          createSymlinkTreeHelper(action, actionExecutionContext).clearRunfilesDirectory();
         } else if (action.getRunfileSymlinksMode() == RunfileSymlinksMode.INTERNAL
             && !action.isFilesetTree()) {
           try {
@@ -161,12 +162,13 @@ private static void createOutput(
     }
   }
 
-  private static SymlinkTreeHelper createSymlinkTreeHelper(
+  private SymlinkTreeHelper createSymlinkTreeHelper(
       SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) {
     return new SymlinkTreeHelper(
         actionExecutionContext.getInputPath(action.getInputManifest()),
         actionExecutionContext.getInputPath(action.getOutputManifest()).getParentDirectory(),
-        action.isFilesetTree());
+        action.isFilesetTree(),
+        workspaceName);
   }
 
   private static EnvironmentalExecException createLinkFailureException(

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java

@@ -43,7 +43,11 @@ public void checkCreatedSpawn() {
     BinTools binTools =
         BinTools.forUnitTesting(execRoot, ImmutableList.of(SymlinkTreeHelper.BUILD_RUNFILES));
     Command command =
-        new SymlinkTreeHelper(inputManifestPath, execRoot.getRelative("output/MANIFEST"), false)
+        new SymlinkTreeHelper(
+                inputManifestPath,
+                execRoot.getRelative("output/MANIFEST"),
+                false,
+                PathFragment.create("__main__"))
             .createCommand(execRoot, binTools, ImmutableMap.of());
     assertThat(command.getEnvironment()).isEmpty();
     assertThat(command.getWorkingDirectory()).isEqualTo(execRoot.getPathFile());

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java

@@ -67,7 +67,7 @@ public void outputServiceInteraction() throws Exception {
     StoredEventHandler eventHandler = new StoredEventHandler();
 
     when(context.getContext(SymlinkTreeActionContext.class))
-        .thenReturn(new SymlinkTreeStrategy(outputService, null));
+        .thenReturn(new SymlinkTreeStrategy(outputService, null, PathFragment.create("__main__")));
     when(context.getInputPath(any())).thenAnswer((i) -> ((Artifact) i.getArgument(0)).getPath());
     when(context.getPathResolver()).thenReturn(ArtifactPathResolver.IDENTITY);
     when(context.getEventHandler()).thenReturn(eventHandler);
@@ -130,7 +130,7 @@ public void inprocessSymlinkCreation() throws Exception {
     StoredEventHandler eventHandler = new StoredEventHandler();
 
     when(context.getContext(SymlinkTreeActionContext.class))
-        .thenReturn(new SymlinkTreeStrategy(outputService, null));
+        .thenReturn(new SymlinkTreeStrategy(outputService, null, PathFragment.create("__main__")));
     when(context.getInputPath(any())).thenAnswer((i) -> ((Artifact) i.getArgument(0)).getPath());
     when(context.getEventHandler()).thenReturn(eventHandler);
     when(outputService.canCreateSymlinkTree()).thenReturn(false);

src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java

@@ -39,6 +39,7 @@
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
@@ -71,7 +72,9 @@ public TestExecutorBuilder(FileSystem fileSystem, Path execRoot, BinTools binToo
     this.execRoot = execRoot;
     addContext(FileWriteActionContext.class, new FileWriteStrategy());
     addContext(TemplateExpansionContext.class, new LocalTemplateExpansionStrategy());
-    addContext(SymlinkTreeActionContext.class, new SymlinkTreeStrategy(null, binTools));
+    addContext(
+        SymlinkTreeActionContext.class,
+        new SymlinkTreeStrategy(null, binTools, PathFragment.create("__main__")));
     addContext(SpawnStrategyResolver.class, new SpawnStrategyResolver());
   }
 

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java

@@ -2500,6 +2500,11 @@ public RunfileSymlinksMode getSymlinksMode() {
           public boolean isBuildRunfileLinks() {
             return false;
           }
+
+          @Override
+          public PathFragment getWorkspaceName() {
+            return PathFragment.create("__main__");
+          }
         };
     return new RunfilesSupplier() {
       @Override

src/test/py/bazel/runfiles_test.py

@@ -427,5 +427,17 @@ def testRunfilesDirectoryIncrementalityNoBuildRunfileLinksEnableRunfilesFlippedO
       ["run", ":test", "--nobuild_runfile_links", "--noenable_runfiles"], allow_failure=True)
     self.assertNotEqual(exit_code, 0)
 
+  def testTestsRunWithNoBuildRunfileLinksAndNoEnableRunfiles(self):
+    self.ScratchFile("MODULE.bazel")
+    self.ScratchFile("BUILD", [
+      "sh_test(",
+      "  name = 'test',",
+      "  srcs = ['test.sh'],",
+      ")",
+    ])
+    self.ScratchFile("test.sh", executable=True)
+    self.ScratchFile(".bazelrc", ["common --spawn_strategy=local"])
+    self.RunBazel(["test", ":test", "--nobuild_runfile_links", "--noenable_runfiles"])
+
 if __name__ == "__main__":
   absltest.main()