CLion Virtual Include Fixes

.bazelrc

@@ -2,8 +2,8 @@ build --java_language_version=17 --java_runtime_version=17
 
 # Delete test data packages, needed for bazel integration tests. Update by running the following command:
 # bazel run @rules_bazel_integration_test//tools:update_deleted_packages
-build --deleted_packages=aspect/testing/tests/src/com/google/idea/blaze/aspect/integration/testdata,clwb/tests/projects/simple/main
-query --deleted_packages=aspect/testing/tests/src/com/google/idea/blaze/aspect/integration/testdata,clwb/tests/projects/simple/main
+build --deleted_packages=aspect/testing/tests/src/com/google/idea/blaze/aspect/integration/testdata,clwb/tests/projects/simple/main,clwb/tests/projects/virtual_includes/lib/strip_absolut,clwb/tests/projects/virtual_includes/lib/strip_relative,clwb/tests/projects/virtual_includes/main
+query --deleted_packages=aspect/testing/tests/src/com/google/idea/blaze/aspect/integration/testdata,clwb/tests/projects/simple/main,clwb/tests/projects/virtual_includes/lib/strip_absolut,clwb/tests/projects/virtual_includes/lib/strip_relative,clwb/tests/projects/virtual_includes/main
 
 common --enable_bzlmod
 common --enable_workspace # to load rules_scala from WORKSPACE.bzlmod

base/src/META-INF/blaze-base.xml

@@ -387,16 +387,9 @@
     <notificationGroup displayType="BALLOON" id="QuerySyncBuild" />
     <registryKey defaultValue="false" description="Disable auto import of bazel projects" key="bazel.auto.import.disabled"/>
     <registryKey defaultValue="false" description="Allow to have optional imports in project view" key="bazel.projectview.optional.imports"/>
-    <registryKey
-      key="bazel.sync.resolve.virtual.includes"
-      description="Apply heuristics to detect correct location of headers which are accessed from _virtual_includes during build. (requires re-sync)"
-      defaultValue="true"
-    />
-    <registryKey
-      key="bazel.sync.collect.virtual.includes.hints"
-      description="CAN BREAK SYNC. Collect the actual location of includes during sync. Currently, only available with the legacy engine. (requires re-sync)"
-      defaultValue="false"
-    />
+    <registryKey key="bazel.sync.resolve.virtual.includes"
+                 description="Apply heuristics to detect correct location of headers which are accessed from _virtual_includes during build. (requires re-sync)"
+                 defaultValue="true"/>
     <registryKey defaultValue="true"
                  description="Enables opening the project view file the first time the project is imported"
                  key="bazel.project.import.open_project_view"/>

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

@@ -119,8 +119,11 @@ public ImmutableList<File> resolveToIncludeDirectories(ExecutionRootPath path) {
         // Resolve virtual_include from execution root either to local or external workspace for correct code insight
         ImmutableList<File> resolved = ImmutableList.of();
         try {
-          resolved = VirtualIncludesHandler.resolveVirtualInclude(path, outputBase,
-                  workspacePathResolver, targetMap);
+          resolved = VirtualIncludesHandler.resolveVirtualInclude(
+              path,
+              outputBase,
+              workspacePathResolver,
+              targetMap);
         } catch (Throwable throwable) {
           LOG.error("Failed to resolve virtual includes for " + path, throwable);
         }

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

@@ -1,34 +1,24 @@
 package com.google.idea.blaze.base.sync.workspace;
 
-import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
-import com.google.idea.blaze.base.ideinfo.ArtifactLocation;
-import com.google.idea.blaze.base.ideinfo.CIdeInfo;
-import com.google.idea.blaze.base.ideinfo.Dependency;
-import com.google.idea.blaze.base.ideinfo.TargetIdeInfo;
 import com.google.idea.blaze.base.ideinfo.TargetKey;
 import com.google.idea.blaze.base.ideinfo.TargetMap;
-import com.google.idea.blaze.base.model.BlazeProjectData;
 import com.google.idea.blaze.base.model.primitives.ExecutionRootPath;
 import com.google.idea.blaze.base.model.primitives.Label;
 import com.google.idea.blaze.base.model.primitives.TargetName;
 import com.google.idea.blaze.base.model.primitives.WorkspacePath;
 import com.intellij.openapi.diagnostic.Logger;
-import com.intellij.openapi.progress.ProgressIndicator;
 import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.openapi.util.registry.Registry;
 
 import java.io.File;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
 import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.Stack;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
+import kotlinx.coroutines.scheduling.WorkQueueKt;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -49,168 +39,13 @@ public class VirtualIncludesHandler {
   static final Path VIRTUAL_INCLUDES_DIRECTORY = Path.of("_virtual_includes");
 
   private static final Logger LOG = Logger.getInstance(VirtualIncludesHandler.class);
-  private static final String ABSOLUTE_LABEL_PREFIX = "//";
   private static final int EXTERNAL_DIRECTORY_IDX = 3;
   private static final int EXTERNAL_WORKSPACE_NAME_IDX = 4;
   private static final int WORKSPACE_PATH_START_FOR_EXTERNAL_WORKSPACE = 5;
   private static final int WORKSPACE_PATH_START_FOR_LOCAL_WORKSPACE = 3;
-  private static final int ABSOLUTE_LABEL_PREFIX_LENGTH = ABSOLUTE_LABEL_PREFIX.length();
 
-  public static boolean useHeuristic() {
-    return Registry.is("bazel.sync.resolve.virtual.includes") && !useHints();
-  }
-
-  public static boolean useHints() {
-    return Registry.is("bazel.sync.collect.virtual.includes.hints");
-  }
-
-  private static Path trimStart(Path value, @Nullable Path prefix) {
-    if (prefix == null || !value.startsWith(prefix)) {
-      return value;
-    }
-
-    return value.subpath(prefix.getNameCount(), value.getNameCount());
-  }
-
-  @Nullable
-  private static Path pathOf(@Nullable String value) {
-    if (value == null || value.isEmpty()) {
-      return null;
-    }
-
-    // turns a windows absolut path into a normal absolut path
-    if (value.startsWith(ABSOLUTE_LABEL_PREFIX)) {
-      value = value.substring(1);
-    }
-
-    try {
-      return Path.of(value);
-    } catch (InvalidPathException e) {
-      LOG.warn("invalid path: " + value);
-      return null;
-    }
-  }
-
-  private static void collectTargetIncludeHints(
-      Path root,
-      TargetKey targetKey,
-      TargetIdeInfo targetIdeInfo,
-      ExecutionRootPathResolver resolver,
-      ProgressIndicator indicator,
-      ImmutableList.Builder<String> includes) {
-    CIdeInfo cIdeInfo = targetIdeInfo.getcIdeInfo();
-    if (cIdeInfo == null) {
-      return;
-    }
-
-    Path includePrefix = pathOf(cIdeInfo.getIncludePrefix());
-    Path stripPrefix = pathOf(cIdeInfo.getStripIncludePrefix());
-    Path packagePath = targetKey.getLabel().blazePackage().asPath();
-
-    String externalWorkspaceName = targetKey.getLabel().externalWorkspaceName();
-
-    for (ArtifactLocation header : cIdeInfo.getHeaders()) {
-      indicator.checkCanceled();
-
-      Path realPath;
-      if (externalWorkspaceName != null) {
-        Path externalWorkspace = ExecutionRootPathResolver.externalPath.toPath()
-            .resolve(externalWorkspaceName);
-
-        Path resolvedPath = resolver.resolveToExternalWorkspaceWithSymbolicLinkResolution(
-            new ExecutionRootPath(externalWorkspace.toString())).get(0).toPath();
-
-        realPath = resolvedPath.resolve(header.getRelativePath());
-      } else {
-        realPath = root.resolve(header.getExecutionRootRelativePath());
-      }
-
-      Path codePath = pathOf(header.getRelativePath());
-      if (codePath == null) {
-        continue;
-      }
-
-      // if the path is absolut, strip prefix is a repository-relative path
-      if (stripPrefix != null && stripPrefix.isAbsolute()) {
-        codePath = trimStart(codePath, stripPrefix.subpath(0, stripPrefix.getNameCount()));
-      }
-
-      codePath = trimStart(codePath, packagePath);
-
-      // if the path is not absolut, strip prefix is a package-relative path
-      if (stripPrefix != null && !stripPrefix.isAbsolute()) {
-        codePath = trimStart(codePath, stripPrefix);
-      }
-
-      if (includePrefix != null) {
-        codePath = includePrefix.resolve(codePath);
-      }
-
-      includes.add(String.format("-ibazel%s=%s", codePath, realPath));
-    }
-  }
-
-  private static ImmutableList<String> doCollectIncludeHints(
-      Path root,
-      TargetKey targetKey,
-      BlazeProjectData projectData,
-      ExecutionRootPathResolver resolver,
-      ProgressIndicator indicator) {
-    Stack<TargetKey> frontier = new Stack<>();
-    frontier.push(targetKey);
-
-    Set<TargetKey> explored = new HashSet<>();
-
-    ImmutableList.Builder<String> includes = ImmutableList.builder();
-    while (!frontier.isEmpty()) {
-      indicator.checkCanceled();
-
-      TargetKey currentKey = frontier.pop();
-      if (!explored.add(currentKey)) {
-        continue;
-      }
-
-      TargetIdeInfo currentIdeInfo = projectData.getTargetMap().get(currentKey);
-      if (currentIdeInfo == null) {
-        continue;
-      }
-
-      collectTargetIncludeHints(root, currentKey, currentIdeInfo, resolver, indicator,
-          includes);
-
-      for (Dependency dep : currentIdeInfo.getDependencies()) {
-        frontier.push(dep.getTargetKey());
-      }
-    }
-
-    return includes.build();
-  }
-
-  /**
-   * Collects all header files form the targetKey and its dependencies to create the '-ibazel'
-   * mappings. The mappings are used to resolve headers which use an 'include_prefix' or a
-   * 'strip_include_prefix'.
-   */
-  public static ImmutableList<String> collectIncludeHints(
-      Path projectRoot,
-      TargetKey targetKey,
-      BlazeProjectData projectData,
-      ExecutionRootPathResolver resolver,
-      ProgressIndicator indicator) {
-    indicator.pushState();
-    indicator.setIndeterminate(true);
-    indicator.setText2("Collecting include hints...");
-
-    Stopwatch stopwatch = Stopwatch.createStarted();
-    ImmutableList<String> result = doCollectIncludeHints(projectRoot, targetKey, projectData,
-        resolver, indicator);
-
-    long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
-    LOG.info(String.format("Collecting include hints took %dms", elapsed));
-
-    indicator.popState();
-
-    return result;
+  static boolean useHeuristic() {
+    return Registry.is("bazel.sync.resolve.virtual.includes");
   }
 
   private static List<Path> splitExecutionPath(ExecutionRootPath executionRootPath) {
@@ -229,7 +64,8 @@ static boolean containsVirtualInclude(ExecutionRootPath executionRootPath) {
    * target data or empty list if resolution has failed
    */
   @NotNull
-  static ImmutableList<File> resolveVirtualInclude(ExecutionRootPath executionRootPath,
+  static ImmutableList<File> resolveVirtualInclude(
+      ExecutionRootPath executionRootPath,
       File externalWorkspacePath,
       WorkspacePathResolver workspacePathResolver,
       TargetMap targetMap) {
@@ -247,46 +83,56 @@ static ImmutableList<File> resolveVirtualInclude(ExecutionRootPath executionRoot
       return ImmutableList.of();
     }
 
-    TargetIdeInfo info = targetMap.get(key);
+    final var info = targetMap.get(key);
     if (info == null) {
       return ImmutableList.of();
     }
 
-    CIdeInfo cIdeInfo = info.getcIdeInfo();
+    if (info.getSources().stream().anyMatch((it) -> !it.isSource())) {
+      // target contains generated sources which cannot be found in the project root, fallback to virtual include directory
+      return ImmutableList.of();
+    }
+
+    final var cIdeInfo = info.getcIdeInfo();
     if (cIdeInfo == null) {
       return ImmutableList.of();
     }
 
-    if (!info.getcIdeInfo().getIncludePrefix().isEmpty()) {
-      LOG.debug(
-          "_virtual_includes cannot be handled for targets with include_prefix attribute");
+    if (!cIdeInfo.getIncludePrefix().isEmpty()) {
+      // it is not possible to handle include prefixes here, fallback to virtual include directory
       return ImmutableList.of();
     }
 
-    String stripPrefix = info.getcIdeInfo().getStripIncludePrefix();
-    if (!stripPrefix.isEmpty()) {
-      if (stripPrefix.endsWith("/")) {
-        stripPrefix = stripPrefix.substring(0, stripPrefix.length() - 1);
-      }
-      String externalWorkspaceName = key.getLabel().externalWorkspaceName();
-      WorkspacePath stripPrefixWorkspacePath = stripPrefix.startsWith(ABSOLUTE_LABEL_PREFIX) ?
-          new WorkspacePath(stripPrefix.substring(ABSOLUTE_LABEL_PREFIX_LENGTH)) :
-          new WorkspacePath(key.getLabel().blazePackage(), stripPrefix);
-      if (externalWorkspaceName != null) {
-        ExecutionRootPath external = new ExecutionRootPath(
-            ExecutionRootPathResolver.externalPath.toPath()
-                .resolve(externalWorkspaceName)
-                .resolve(stripPrefixWorkspacePath.toString()).toString());
+    var stripPrefix = cIdeInfo.getStripIncludePrefix();
+    if (stripPrefix == null || stripPrefix.isBlank()) {
+      return ImmutableList.of();
+    }
 
-        return ImmutableList.of(external.getFileRootedAt(externalWorkspacePath));
-      } else {
-        return workspacePathResolver.resolveToIncludeDirectories(
-            stripPrefixWorkspacePath);
-      }
+    // strip prefix is a path not a label, `//something` is invalid
+    stripPrefix = stripPrefix.replaceAll("/+", "/");
+
+    // remove trailing slash
+    if (stripPrefix.endsWith("/")) {
+      stripPrefix = stripPrefix.substring(0, stripPrefix.length() - 1);
+    }
+
+    final WorkspacePath workspacePath;
+    if (stripPrefix.startsWith("/")) {
+      workspacePath = new WorkspacePath(stripPrefix.substring(1));
     } else {
-      return ImmutableList.of();
+      workspacePath = new WorkspacePath(key.getLabel().blazePackage(), stripPrefix);
+    }
+
+    final var externalWorkspace = key.getLabel().externalWorkspaceName();
+    if (externalWorkspace == null) {
+      return workspacePathResolver.resolveToIncludeDirectories(workspacePath);
     }
 
+    final var externalRoot = ExecutionRootPathResolver.externalPath.toPath()
+        .resolve(externalWorkspace)
+        .resolve(workspacePath.toString()).toString();
+
+    return ImmutableList.of(new ExecutionRootPath(externalRoot).getFileRootedAt(externalWorkspacePath));
   }
 
   /**

clwb/BUILD

@@ -156,7 +156,16 @@ clwb_integration_test(
     srcs = ["tests/integrationtests/com/google/idea/blaze/clwb/SimpleTest.java"],
 )
 
+clwb_integration_test(
+    name = "virtual_includes_integration_test",
+    project = "virtual_includes",
+    srcs = ["tests/integrationtests/com/google/idea/blaze/clwb/VirtualIncludesTest.java"],
+)
+
 test_suite(
     name = "integration_tests",
-    tests = [":simple_integration_test"],
+    tests = [
+        ":simple_integration_test",
+        ":virtual_includes_integration_test",
+    ],
 )