Skip to content

Commit

Permalink
CLion Virtual Include Fixes (#6759)
Browse files Browse the repository at this point in the history 
Addresses problems with the virtual includes handler:
- Fixes absolute strip prefix
- Fallback to virtual includes for include prefix (no longer an error)
- Fallback to virtual includes if a source file is generated

Fixes  #6648
  • Loading branch information
@LeFrosch
LeFrosch authored Oct 2, 2024
1 parent cd31e7c commit d5e3402
Show file tree
Hide file tree
Showing 17 changed files with 193 additions and 236 deletions.
4 changes: 2 additions & 2 deletions .bazelrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 3 additions & 10 deletions base/src/META-INF/blaze-base.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
7 changes: 5 additions & 2 deletions base/src/com/google/idea/blaze/base/sync/workspace/ExecutionRootPathResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
234 changes: 39 additions & 195 deletions base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<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) {
Expand All @@ -229,7 +62,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) {
Expand All @@ -247,46 +81,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));
}

/**
Expand Down
11 changes: 10 additions & 1 deletion clwb/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Loading

0 comments on commit d5e3402

Please sign in to comment.