Switch external path resolution implementation to getFileRootedAt(out…

…putBase)

Based on tpasternak's good feedback.
Please see #2766 and #4063 for context.

base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java

@@ -25,7 +25,6 @@
 import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
 import com.intellij.openapi.project.Project;
 import java.io.File;
-import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 
 /**
@@ -40,15 +39,18 @@ public class ExecutionRootPathResolver {
 
   private final ImmutableList<String> buildArtifactDirectories;
   private final File executionRoot;
+  private final File outputBase;
   private final WorkspacePathResolver workspacePathResolver;
 
   public ExecutionRootPathResolver(
       BuildSystemProvider buildSystemProvider,
       WorkspaceRoot workspaceRoot,
       File executionRoot,
+      File outputBase,
       WorkspacePathResolver workspacePathResolver) {
     this.buildArtifactDirectories = buildArtifactDirectories(buildSystemProvider, workspaceRoot);
     this.executionRoot = executionRoot;
+    this.outputBase = outputBase;
     this.workspacePathResolver = workspacePathResolver;
   }
 
@@ -63,6 +65,7 @@ public static ExecutionRootPathResolver fromProject(Project project) {
         Blaze.getBuildSystemProvider(project),
         WorkspaceRoot.fromProject(project),
         projectData.getBlazeInfo().getExecutionRoot(),
+        projectData.getBlazeInfo().getOutputBase(),
         projectData.getWorkspacePathResolver());
   }
 
@@ -75,10 +78,18 @@ public File resolveExecutionRootPath(ExecutionRootPath path) {
     if (path.isAbsolute()) {
       return path.getAbsoluteOrRelativeFile();
     }
-    if (isInWorkspace(path)) {
-      return workspacePathResolver.resolveToFile(path.getAbsoluteOrRelativeFile().getPath());
+    String firstPathComponent = getFirstPathComponent(path.getAbsoluteOrRelativeFile().getPath());
+    if (buildArtifactDirectories.contains(firstPathComponent)) {
+      // Build artifacts accumulate under the execution root, independent of symlink settings
+      return path.getFileRootedAt(executionRoot);
+    }
+    if (firstPathComponent.equals("external")) { // In external workspace
+      // External workspaces accumulate under the output base.
+      // The symlinks to them under the execution root are unstable, and only linked per build.
+      return path.getFileRootedAt(outputBase);
     }
-    return convertExternalToStable(path.getFileRootedAt(executionRoot));
+    // Else, in main workspace
+    return workspacePathResolver.resolveToFile(path.getAbsoluteOrRelativeFile().getPath());
   }
 
   /**
@@ -90,54 +101,32 @@ public ImmutableList<File> resolveToIncludeDirectories(ExecutionRootPath path) {
     if (path.isAbsolute()) {
       return ImmutableList.of(path.getAbsoluteOrRelativeFile());
     }
-    if (isInWorkspace(path)) {
-      WorkspacePath workspacePath =
-          WorkspacePath.createIfValid(path.getAbsoluteOrRelativeFile().getPath());
-      if (workspacePath != null) {
-        return workspacePathResolver.resolveToIncludeDirectories(workspacePath);
-      } else {
-        return ImmutableList.of();
-      }
+    String firstPathComponent = getFirstPathComponent(path.getAbsoluteOrRelativeFile().getPath());
+    if (buildArtifactDirectories.contains(firstPathComponent)) {
+      // Build artifacts accumulate under the execution root, independent of symlink settings
+      return ImmutableList.of(path.getFileRootedAt(executionRoot));
+    }
+    if (firstPathComponent.equals("external")) { // In external workspace
+      // External workspaces accumulate under the output base.
+      // The symlinks to them under the execution root are unstable, and only linked per build.
+      return ImmutableList.of(path.getFileRootedAt(outputBase));
+    }
+    // Else, in main workspace
+    WorkspacePath workspacePath =
+        WorkspacePath.createIfValid(path.getAbsoluteOrRelativeFile().getPath());
+    if (workspacePath != null) {
+      return workspacePathResolver.resolveToIncludeDirectories(workspacePath);
+    } else {
+      return ImmutableList.of();
     }
-    return ImmutableList.of(convertExternalToStable(path.getFileRootedAt(executionRoot)));
   }
 
   public File getExecutionRoot() {
     return executionRoot;
   }
 
-  private boolean isInWorkspace(ExecutionRootPath path) {
-    String firstPathComponent = getFirstPathComponent(path.getAbsoluteOrRelativeFile().getPath());
-    return !buildArtifactDirectories.contains(firstPathComponent)
-        && !isExternalWorkspacePath(firstPathComponent);
-  }
-
   private static String getFirstPathComponent(String path) {
     int index = path.indexOf(File.separatorChar);
     return index == -1 ? path : path.substring(0, index);
   }
-
-  private static boolean isExternalWorkspacePath(String firstPathComponent) {
-    return firstPathComponent.equals("external");
-  }
-
-  /**
-  * Converts external paths in unstable locations under execroot that may change on the next build
-  * to stable ones under their external workspace.
-  * That is, converts paths under <outputBase>/execroot/<workspace_name>/external/ to paths under
-  * <outputBase>/external/.
-  * Safe to call on all paths; non-external paths are left as they are.
-  * See https://github.com/bazelbuild/intellij/issues/2766 for more details.
-  */
-  private static File convertExternalToStable(File f) {
-    String regexSep = Pattern.quote(f.separator);
-    // Workspace name defaults to __main__ per DEFAULT_REPOSITORY_DIRECTORY in 
-    // https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java
-    // Valid workspace name regex copied from LEGAL_WORKSPACE_NAME in 
-    // https://github.com/bazelbuild/bazel/blob/9302ebd906a2f5e9678f994efb2fbc8abab544c0/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
-    String regexUnstableExecrootSubpath = // ↓ matches workspace name.
-      regexSep + "execroot" + regexSep + "(__main__|\\p{Alpha}[-.\\w]*)" + regexSep + "external" + regexSep;
-    String stableExternalSubpath = f.separator + "external" + f.separator;
-    return new File(f.getAbsolutePath().replaceFirst(regexUnstableExecrootSubpath, stableExternalSubpath));
-  }
 }

base/tests/unittests/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolverTest.java

@@ -45,6 +45,7 @@ protected void initTest(Container applicationServices, Container projectServices
             new BazelBuildSystemProvider(),
             WORKSPACE_ROOT,
             new File(EXECUTION_ROOT),
+            new File(OUTPUT_BASE),
             new WorkspacePathResolverImpl(WORKSPACE_ROOT));
   }
 

cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java

@@ -173,6 +173,7 @@ private OCWorkspaceImpl.ModifiableModel calculateConfigurations(
             Blaze.getBuildSystemProvider(project),
             workspaceRoot,
             blazeProjectData.getBlazeInfo().getExecutionRoot(),
+            blazeProjectData.getBlazeInfo().getOutputBase(),
             blazeProjectData.getWorkspacePathResolver());
 
     int progress = 0;

cpp/src/com/google/idea/blaze/cpp/BlazeConfigurationResolver.java

@@ -75,6 +75,7 @@ public BlazeConfigurationResolverResult update(
             Blaze.getBuildSystemProvider(project),
             WorkspaceRoot.fromProject(project),
             blazeProjectData.getBlazeInfo().getExecutionRoot(),
+            blazeProjectData.getBlazeInfo().getOutputBase(),
             blazeProjectData.getWorkspacePathResolver());
     ImmutableMap<TargetKey, CToolchainIdeInfo> toolchainLookupMap =
         BlazeConfigurationToolchainResolver.buildToolchainLookupMap(

cpp/src/com/google/idea/blaze/cpp/IncludeRootFlagsProcessor.java

@@ -47,6 +47,7 @@ public Optional<BlazeCompilerFlagsProcessor> getProcessor(Project project) {
               Blaze.getBuildSystemProvider(project),
               workspaceRoot,
               projectData.getBlazeInfo().getExecutionRoot(),
+              projectData.getBlazeInfo().getOutputBase(),
               projectData.getWorkspacePathResolver());
       return Optional.of(new IncludeRootFlagsProcessor(executionRootPathResolver));
     }

cpp/tests/integrationtests/com/google/idea/blaze/cpp/BlazeCppResolvingTestCase.java

@@ -94,6 +94,7 @@ private ExecutionRootPathResolver executionRootPathResolver(BlazeProjectData pro
         new BazelBuildSystemProvider(),
         workspaceRoot,
         projectData.getBlazeInfo().getExecutionRoot(),
+        projectData.getBlazeInfo().getOutputBase(),
         projectData.getWorkspacePathResolver());
   }
 

cpp/tests/unittests/com/google/idea/blaze/cpp/BlazeCompilerSettingsTest.java

@@ -86,7 +86,7 @@ public void testCompilerSwitchesSimple() {
   }
 
   @Test
-  public void relativeSysroot_makesAbsolutePathInWorkspace() {
+  public void relativeSysroot_makesAbsolutePathInMainWorkspace() {
     ImmutableList<String> cFlags = ImmutableList.of("--sysroot=third_party/toolchain/");
     BlazeCompilerSettings settings =
         new BlazeCompilerSettings(
@@ -118,7 +118,7 @@ public void absoluteSysroot_doesNotChange() {
   }
 
   @Test
-  public void relativeIsystem_makesAbsolutePathInExecRoot() {
+  public void relativeIsystem_makesAbsolutePathInWorkspaces() {
     ImmutableList<String> cFlags =
         ImmutableList.of("-isystem", "external/arm_gcc/include", "-DFOO=1", "-Ithird_party/stl");
     BlazeCompilerSettings settings =
@@ -130,16 +130,36 @@ public void relativeIsystem_makesAbsolutePathInExecRoot() {
             cFlags,
             "cc version (trunk r123456)");
 
-    String execRoot = blazeProjectData.getBlazeInfo().getExecutionRoot().toString();
+    String outputBase = blazeProjectData.getBlazeInfo().getOutputBase().toString();
     assertThat(settings.getCompilerSwitches(CLanguageKind.C, null))
         .containsExactly(
             "-isystem",
-            execRoot + "/external/arm_gcc/include",
+            outputBase + "/external/arm_gcc/include",
             "-DFOO=1",
             "-I",
             workspaceRoot + "/third_party/stl");
   }
 
+  @Test
+  public void relativeIquote_makesAbsolutePathInExecRoot() {
+    ImmutableList<String> cFlags =
+        ImmutableList.of("-iquote", "bazel-out/android-arm64-v8a-opt/bin/external/boringssl");
+    BlazeCompilerSettings settings =
+        new BlazeCompilerSettings(
+            getProject(),
+            new File("bin/c"),
+            new File("bin/c++"),
+            cFlags,
+            cFlags,
+            "cc version (trunk r123456)");
+
+    String execRoot = blazeProjectData.getBlazeInfo().getExecutionRoot().toString();
+    assertThat(settings.getCompilerSwitches(CLanguageKind.C, null))
+        .containsExactly(
+            "-iquote",
+            execRoot + "/bazel-out/android-arm64-v8a-opt/bin/external/boringssl");
+  }
+
   @Test
   public void absoluteISystem_doesNotChange() {
     ImmutableList<String> cFlags = ImmutableList.of("-isystem", "/usr/include");