diff --git a/.bazelrc b/.bazelrc index 3d4d5d4e56a..a617c51c458 100644 --- a/.bazelrc +++ b/.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 diff --git a/base/src/META-INF/blaze-base.xml b/base/src/META-INF/blaze-base.xml index 6e8c748b56f..34ad207f953 100644 --- a/base/src/META-INF/blaze-base.xml +++ b/base/src/META-INF/blaze-base.xml @@ -387,16 +387,9 @@ - - + diff --git a/base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java b/base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java index 4b456246b57..12fc7f38e3a 100644 --- a/base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java +++ b/base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java @@ -119,8 +119,11 @@ public ImmutableList resolveToIncludeDirectories(ExecutionRootPath path) { // Resolve virtual_include from execution root either to local or external workspace for correct code insight ImmutableList 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); } diff --git a/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java b/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java index 259c2f53a15..8cf8b574370 100644 --- a/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java +++ b/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java @@ -1,33 +1,21 @@ 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 org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -49,168 +37,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 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 doCollectIncludeHints( - Path root, - TargetKey targetKey, - BlazeProjectData projectData, - ExecutionRootPathResolver resolver, - ProgressIndicator indicator) { - Stack frontier = new Stack<>(); - frontier.push(targetKey); - - Set explored = new HashSet<>(); - - ImmutableList.Builder 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 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 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 splitExecutionPath(ExecutionRootPath executionRootPath) { @@ -229,7 +62,8 @@ static boolean containsVirtualInclude(ExecutionRootPath executionRootPath) { * target data or empty list if resolution has failed */ @NotNull - static ImmutableList resolveVirtualInclude(ExecutionRootPath executionRootPath, + static ImmutableList resolveVirtualInclude( + ExecutionRootPath executionRootPath, File externalWorkspacePath, WorkspacePathResolver workspacePathResolver, TargetMap targetMap) { @@ -247,46 +81,56 @@ static ImmutableList 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)); } /** diff --git a/clwb/BUILD b/clwb/BUILD index b53663fea49..d6d9099df2f 100644 --- a/clwb/BUILD +++ b/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", + ], ) \ No newline at end of file diff --git a/clwb/tests/integrationtests/com/google/idea/blaze/clwb/VirtualIncludesTest.java b/clwb/tests/integrationtests/com/google/idea/blaze/clwb/VirtualIncludesTest.java new file mode 100644 index 00000000000..f8cd59e6b7e --- /dev/null +++ b/clwb/tests/integrationtests/com/google/idea/blaze/clwb/VirtualIncludesTest.java @@ -0,0 +1,60 @@ +package com.google.idea.blaze.clwb; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.idea.blaze.clwb.base.Assertions.assertContainsHeader; + +import com.google.idea.blaze.clwb.base.ClwbIntegrationTestCase; +import com.intellij.openapi.util.SystemInfo; +import com.intellij.openapi.vfs.VirtualFile; +import com.jetbrains.cidr.lang.workspace.compiler.ClangCompilerKind; +import com.jetbrains.cidr.lang.workspace.compiler.GCCCompilerKind; +import com.jetbrains.cidr.lang.workspace.compiler.MSVCCompilerKind; +import com.jetbrains.cidr.lang.workspace.headerRoots.HeadersSearchRoot; +import java.util.List; +import org.jetbrains.annotations.Nullable; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class VirtualIncludesTest extends ClwbIntegrationTestCase { + + @Test + public void testClwb() { + final var errors = runSync(defaultSyncParams().build()); + errors.assertNoErrors(); + + checkIncludes(); + } + + private @Nullable VirtualFile findHeader(String fileName, List roots) { + for (final var root : roots) { + final var rootFile = root.getVirtualFile(); + if (rootFile == null) continue; + + final var headerFile = rootFile.findFileByRelativePath(fileName); + if (headerFile == null) continue; + + return headerFile; + } + + return null; + } + + private void checkIncludes() { + final var compilerSettings = findFileCompilerSettings("main/hello-world.cc"); + + final var headersSearchRoots = compilerSettings.getHeadersSearchRoots().getAllRoots(); + assertThat(headersSearchRoots).isNotEmpty(); + + assertContainsHeader("strip_absolut/strip_absolut.h", headersSearchRoots); + assertContainsHeader("strip_absolut/generated.h", headersSearchRoots); + assertContainsHeader("strip_relative.h", headersSearchRoots); + + assertThat(findProjectFile("lib/strip_absolut/strip_absolut.h")) + .isEqualTo(findHeader("strip_absolut/strip_absolut.h", headersSearchRoots)); + + assertThat(findProjectFile("lib/strip_relative/include/strip_relative.h")) + .isEqualTo(findHeader("strip_relative.h", headersSearchRoots)); + } +} diff --git a/clwb/tests/integrationtests/com/google/idea/blaze/clwb/base/Assertions.java b/clwb/tests/integrationtests/com/google/idea/blaze/clwb/base/Assertions.java index 8735271a693..bece3f491ab 100644 --- a/clwb/tests/integrationtests/com/google/idea/blaze/clwb/base/Assertions.java +++ b/clwb/tests/integrationtests/com/google/idea/blaze/clwb/base/Assertions.java @@ -25,25 +25,19 @@ public static void assertDoesntContainHeader(String fileName, List roots) { final var found = new Ref(); - final var foundIn = new Ref(); + final var foundIn = new Ref(); for (final var root : roots) { - root.processChildren(new HeadersSearchRootProcessor() { - @Override - public boolean process(@NotNull VirtualFile file) { - if (file.isDirectory() || !FileUtil.namesEqual(file.getName(), fileName)) { - return true; - } - - found.set(file); - foundIn.set(root); - return false; - } - }); - - if (!found.isNull()) { - break; - } + final var rootFile = root.getVirtualFile(); + if (rootFile == null) continue; + + final var headerFile = rootFile.findFileByRelativePath(fileName); + if (headerFile == null || !headerFile.exists()) continue; + + found.set(headerFile); + foundIn.set(root); + + break; } if (shouldContain) { diff --git a/clwb/tests/projects/virtual_includes/WORKSPACE b/clwb/tests/projects/virtual_includes/WORKSPACE new file mode 100644 index 00000000000..e69de29bb2d diff --git a/clwb/tests/projects/virtual_includes/lib/strip_absolut/BUILD.bazel b/clwb/tests/projects/virtual_includes/lib/strip_absolut/BUILD.bazel new file mode 100644 index 00000000000..b845386b37a --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_absolut/BUILD.bazel @@ -0,0 +1,24 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +cc_library( + name = "lib", + srcs = ["strip_absolut.cc"], + hdrs = ["strip_absolut.h"], + strip_include_prefix = "/lib", + visibility = ["//visibility:public"] +) + +genrule( + name = "generate", + outs = ["generated.h"], + srcs = [], + cmd = r"""echo '#define GENERATED_MACRO 0' > $@""" +) + +cc_library( + name = "gen", + srcs = [], + hdrs = ["generated.h"], + strip_include_prefix = "/lib", + visibility = ["//visibility:public"] +) diff --git a/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.cc b/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.cc new file mode 100644 index 00000000000..f6459277edd --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.cc @@ -0,0 +1,3 @@ +#include "strip_absolut.h" + +void strip_absolut_function() { } diff --git a/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.h b/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.h new file mode 100644 index 00000000000..16dca55bd13 --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_absolut/strip_absolut.h @@ -0,0 +1 @@ +void strip_absolut_function(); diff --git a/clwb/tests/projects/virtual_includes/lib/strip_relative/BUILD.bazel b/clwb/tests/projects/virtual_includes/lib/strip_relative/BUILD.bazel new file mode 100644 index 00000000000..56e27363806 --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_relative/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +cc_library( + name = "lib", + srcs = ["strip_relative.cc"], + hdrs = ["include/strip_relative.h"], + strip_include_prefix = "include", + visibility = ["//visibility:public"] +) diff --git a/clwb/tests/projects/virtual_includes/lib/strip_relative/include/strip_relative.h b/clwb/tests/projects/virtual_includes/lib/strip_relative/include/strip_relative.h new file mode 100644 index 00000000000..3da8fd6ea9b --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_relative/include/strip_relative.h @@ -0,0 +1 @@ +void strip_relative_function(); diff --git a/clwb/tests/projects/virtual_includes/lib/strip_relative/strip_relative.cc b/clwb/tests/projects/virtual_includes/lib/strip_relative/strip_relative.cc new file mode 100644 index 00000000000..40a122c54c4 --- /dev/null +++ b/clwb/tests/projects/virtual_includes/lib/strip_relative/strip_relative.cc @@ -0,0 +1,3 @@ +#include "include/strip_relative.h" + +void strip_relative_function() { } diff --git a/clwb/tests/projects/virtual_includes/main/BUILD b/clwb/tests/projects/virtual_includes/main/BUILD new file mode 100644 index 00000000000..0cc9c388aa8 --- /dev/null +++ b/clwb/tests/projects/virtual_includes/main/BUILD @@ -0,0 +1,12 @@ +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello-world", + srcs = ["hello-world.cc"], + visibility = ["//visibility:public"], + deps = [ + "//lib/strip_absolut:lib", + "//lib/strip_absolut:gen", + "//lib/strip_relative:lib", + ] +) diff --git a/clwb/tests/projects/virtual_includes/main/hello-world.cc b/clwb/tests/projects/virtual_includes/main/hello-world.cc new file mode 100644 index 00000000000..c2429a20794 --- /dev/null +++ b/clwb/tests/projects/virtual_includes/main/hello-world.cc @@ -0,0 +1,10 @@ +#include "strip_absolut/strip_absolut.h" +#include "strip_absolut/generated.h" +#include "strip_relative.h" + +int main() { + strip_absolut_function(); + strip_relative_function(); + + return GENERATED_MACRO; +} diff --git a/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java b/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java index 86ec8add5ba..f01870455f2 100644 --- a/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java +++ b/cpp/src/com/google/idea/blaze/cpp/BlazeCWorkspace.java @@ -316,15 +316,6 @@ private WorkspaceModel calculateConfigurations( .map(File::getAbsolutePath) .forEach(compilerSwitchesBuilder::withSystemIncludePath); - if (VirtualIncludesHandler.useHints()) { - compilerSwitchesBuilder.withSwitches(VirtualIncludesHandler.collectIncludeHints( - workspaceRoot.directory().toPath(), - targetKey, - blazeProjectData, - executionRootPathResolver, - indicator)); - } - final var cCompilerSwitches = buildSwitchBuilder(compilerSettings, compilerSwitchesBuilder, CLanguageKind.C); final var cppCompilerSwitches =