Address review comments in bazelbuild#5301 pull request

- added registry key - added progress indicator - using path instead of string transformations
LeFrosch · Feb 15, 2024 · d58d4ca · d58d4ca
1 parent 9f90f33
commit d58d4ca
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 25 deletions.
diff --git a/cpp/src/META-INF/blaze-cpp.xml b/cpp/src/META-INF/blaze-cpp.xml
@@ -59,6 +59,7 @@
     <registryKey defaultValue="false" description="Disable absolute path trimming in debug clang builds" key="bazel.trim.absolute.path.disabled"/>
     <registryKey defaultValue="true" description="Allow external targets from source directories be imported in" key="bazel.cpp.sync.external.targets.from.directories"/>
     <registryKey defaultValue="false" description="Filter out some incompatible compiler flags (-include)" key="bazel.cpp.sync.workspace.filter.out.incompatible.flags"/>
+    <registryKey defaultValue="false" description="Disable collection of includes during sync" key="bazel.cpp.sync.workspace.collect.includes.disabled"/>
     <applicationService serviceInterface="com.google.idea.blaze.cpp.CompilerVersionChecker"
                         serviceImplementation="com.google.idea.blaze.cpp.CompilerVersionCheckerImpl"/>
     <applicationService serviceInterface="com.google.idea.blaze.cpp.CompilerWrapperProvider"

diff --git a/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java b/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java
@@ -71,9 +71,11 @@
 import com.jetbrains.cidr.lang.workspace.compiler.CompilerInfoCache.Session;
 import com.jetbrains.cidr.lang.workspace.compiler.OCCompilerKind;
 import com.jetbrains.cidr.lang.workspace.compiler.TempFilesPool;
-import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.File;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
@@ -170,45 +172,97 @@ public void run(ProgressIndicator indicator) {
             });
   }
 
-  private List<String> collectIncludes(String rootPath, TargetKey targetKey, BlazeProjectData blazeProjectData) {
+  private Path trimStart(Path value, @Nullable Path prefix) {
+    if (prefix == null || !value.startsWith(prefix)) {
+        return value;
+    }
+
+    return value.subpath(prefix.getNameCount(), value.getNameCount());
+  }
+
+  private @Nullable Path pathOf(@Nullable String value) {
+    if (value == null || value.isEmpty()) {
+        return null;
+    }
+
+    try {
+      return Path.of(value);
+    } catch (InvalidPathException e) {
+      return null;
+    }
+  }
+
+  private List<String> collectIncludes(Path root, TargetKey targetKey, BlazeProjectData blazeProjectData) {
     TargetIdeInfo targetIdeInfo = blazeProjectData.getTargetMap().get(targetKey);
     if (targetIdeInfo == null || targetIdeInfo.getcIdeInfo() == null) {
       return Collections.emptyList();
     }
 
-    ArrayList<String> includes = new ArrayList<>();
     CIdeInfo cIdeInfo = targetIdeInfo.getcIdeInfo();
-    String includePrefix = cIdeInfo.getIncludePrefix();
-    String stripPrefix = cIdeInfo.getStripIncludePrefix();
+    Path includePrefix = pathOf(cIdeInfo.getIncludePrefix());
+    Path stripPrefix = pathOf(cIdeInfo.getStripIncludePrefix());
+
+    Path packagePath = targetKey.getLabel().blazePackage().asPath();
+
+    ArrayList<String> includes = new ArrayList<>();
     for (ArtifactLocation header : cIdeInfo.getHeaders()) {
-      String realPath = rootPath + "/" + header.getExecutionRootRelativePath();
-      String libPath = targetKey.getLabel().blazePackage().asPath().toString();
-      String pathUsedInSourceCode = header.getRelativePath();
-      if (pathUsedInSourceCode != null) {
-        if (libPath != null && !libPath.isEmpty() && pathUsedInSourceCode.startsWith(libPath)) {
-          pathUsedInSourceCode = pathUsedInSourceCode.substring(libPath.length() + 1);
-        }
-        if (stripPrefix != null && !stripPrefix.isEmpty() && pathUsedInSourceCode.startsWith(stripPrefix)) {
-          pathUsedInSourceCode = pathUsedInSourceCode.substring(stripPrefix.length() +
-                  (stripPrefix.endsWith("/") ? 0 : 1));
-          System.out.println("updated path = " + pathUsedInSourceCode);
-        }
-        if (includePrefix != null && !includePrefix.isEmpty()) {
-          pathUsedInSourceCode = includePrefix + "/" + pathUsedInSourceCode;
-          System.out.println("updated path 2 = " + pathUsedInSourceCode);
-        }
+      Path realPath = root.resolve(header.getExecutionRootRelativePath());
 
-        includes.add("-ibazel" + pathUsedInSourceCode + "=" + realPath);
+      Path codePath = pathOf(header.getRelativePath());
+      if (codePath == null) {
+          continue;
       }
+
+      // if 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 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("-ibazel" + codePath + "=" + realPath);
     }
 
     for (Dependency dep : targetIdeInfo.getDependencies()) {
-      includes.addAll(collectIncludes(rootPath, dep.getTargetKey(), blazeProjectData));
+      includes.addAll(collectIncludes(root, dep.getTargetKey(), blazeProjectData));
     }
 
     return includes;
   }
 
+  private List<String> collectIncludesWithProgress(
+      Path root,
+      TargetKey targetKey,
+      BlazeProjectData blazeProjectData,
+      ProgressIndicator indicator) {
+    if (Registry.is("bazel.cpp.sync.workspace.collect.includes.disabled")) {
+      return Collections.emptyList();
+    }
+
+    indicator.pushState();
+    indicator.setIndeterminate(true);
+    indicator.setText2("Collecting includes..");
+
+    Stopwatch stopwatch = Stopwatch.createStarted();
+    List<String> result = collectIncludes(root, targetKey, blazeProjectData);
+
+    long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
+    logger.info(String.format("Collecting includes took %dms", elapsed));
+
+    indicator.popState();
+
+    return result;
+  }
+
   private OCWorkspaceImpl.ModifiableModel calculateConfigurations(
       BlazeProjectData blazeProjectData,
       WorkspaceRoot workspaceRoot,
@@ -301,8 +355,8 @@ private OCWorkspaceImpl.ModifiableModel calculateConfigurations(
                 .map(file -> "-I" + file.getAbsolutePath())
                 .collect(toImmutableList());
 
-        String rootPath = workspaceRoot.directory().getAbsolutePath();
-        ImmutableList<String> includes = ImmutableList.copyOf(collectIncludes(rootPath, targetKey, blazeProjectData));
+        Path rootPath = workspaceRoot.directory().toPath();
+        List<String> includes = collectIncludesWithProgress(rootPath, targetKey, blazeProjectData, indicator);
 
         for (VirtualFile vf : resolveConfiguration.getSources(targetKey)) {
           OCLanguageKind kind = resolveConfiguration.getDeclaredLanguageKind(vf);