Skip to content

Commit

Permalink
Resolve _virtual_includes to local or external workspace (bazelbuild#…
Browse files Browse the repository at this point in the history
…5059)

* Resolve _virtual_includes to local or external workspace

Bazel generates _virtual_includes directory in the execution root
for targets with strip_include_prefix attribute. However, the headers
there are just linked to either external or local workspace.
Unfortunately, CLion can not follow symbolic link and displays
non-project file warning on a project file

 * bazelbuild#510
 * https://youtrack.jetbrains.com/issue/CPP-17658

In some cases symbolic links in execroot could disappear after other
target build resulting in non-resolved or incorrectly suggested include
directive (bazelbuild#145).

The proposal is to identify workspace and target from the include path
and then apply strip_include_prefix attribute to point to the proper
include directory.

* fixup! Resolve _virtual_includes to local or external workspace

Signed-off-by: Evgenii Novozhilov <[email protected]>

* fixup! Resolve _virtual_includes to local or external workspace

Signed-off-by: Evgenii Novozhilov <[email protected]>

* fixup! Resolve _virtual_includes to local or external workspace

Signed-off-by: Evgenii Novozhilov <[email protected]>

* fixup! Resolve _virtual_includes to local or external workspace

Signed-off-by: Evgenii Novozhilov <[email protected]>

* fixup! Resolve _virtual_includes to local or external workspace

Signed-off-by: Evgenii Novozhilov <[email protected]>

---------

Signed-off-by: Evgenii Novozhilov <[email protected]>
  • Loading branch information
ujohnny authored Jul 14, 2023
1 parent f0ea421 commit 0630fee
Show file tree
Hide file tree
Showing 12 changed files with 391 additions and 16 deletions.
2 changes: 2 additions & 0 deletions aspect/intellij_info_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,8 @@ def collect_cpp_info(target, ctx, semantics, ide_info, ide_info_file, output_gro
transitive_include_directory = compilation_context.includes.to_list(),
transitive_quote_include_directory = compilation_context.quote_includes.to_list(),
transitive_system_include_directory = compilation_context.system_includes.to_list(),
include_prefix = getattr(ctx.rule.attr, "include_prefix", None),
strip_include_prefix = getattr(ctx.rule.attr, "strip_include_prefix", None),
)
ide_info["c_ide_info"] = c_info
resolve_files = compilation_context.headers
Expand Down
5 changes: 5 additions & 0 deletions base/src/META-INF/blaze-base.xml
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@
<actionConfigurationCustomizer implementation="com.google.idea.blaze.base.plugin.BlazeHideMakeActions"/>
<notificationGroup displayType="BALLOON" id="BuildifierBinaryMissing"/>
<registryKey defaultValue="false" description="Disable auto import of bazel projects" key="bazel.auto.import.disabled"/>
<registryKey
key="bazel.sync.resolve.virtual.includes"
description="Apply heuristics to detect correct location of headers which are accessed from _virtual_includes during build"
defaultValue="true"
/>
</extensions>

<extensions defaultExtensionNs="com.intellij">
Expand Down
40 changes: 37 additions & 3 deletions base/src/com/google/idea/blaze/base/ideinfo/CIdeInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public final class CIdeInfo implements ProtoWrapper<IntellijIdeInfo.CIdeInfo> {
private final ImmutableList<String> transitiveDefines;
private final ImmutableList<ExecutionRootPath> transitiveSystemIncludeDirectories;

private final String includePrefix;
private final String stripIncludePrefix;

private CIdeInfo(
ImmutableList<ArtifactLocation> sources,
ImmutableList<ArtifactLocation> headers,
Expand All @@ -43,7 +46,9 @@ private CIdeInfo(
ImmutableList<ExecutionRootPath> transitiveIncludeDirectories,
ImmutableList<ExecutionRootPath> transitiveQuoteIncludeDirectories,
ImmutableList<String> transitiveDefines,
ImmutableList<ExecutionRootPath> transitiveSystemIncludeDirectories) {
ImmutableList<ExecutionRootPath> transitiveSystemIncludeDirectories,
String includePrefix,
String stripIncludePrefix) {
this.sources = sources;
this.headers = headers;
this.textualHeaders = textualHeaders;
Expand All @@ -52,6 +57,8 @@ private CIdeInfo(
this.transitiveQuoteIncludeDirectories = transitiveQuoteIncludeDirectories;
this.transitiveDefines = transitiveDefines;
this.transitiveSystemIncludeDirectories = transitiveSystemIncludeDirectories;
this.includePrefix = includePrefix;
this.stripIncludePrefix = stripIncludePrefix;
}

static CIdeInfo fromProto(IntellijIdeInfo.CIdeInfo proto) {
Expand All @@ -65,7 +72,9 @@ static CIdeInfo fromProto(IntellijIdeInfo.CIdeInfo proto) {
proto.getTransitiveQuoteIncludeDirectoryList(), ExecutionRootPath::fromProto),
ProtoWrapper.internStrings(proto.getTransitiveDefineList()),
ProtoWrapper.map(
proto.getTransitiveSystemIncludeDirectoryList(), ExecutionRootPath::fromProto));
proto.getTransitiveSystemIncludeDirectoryList(), ExecutionRootPath::fromProto),
proto.getIncludePrefix(),
proto.getStripIncludePrefix());
}

@Override
Expand Down Expand Up @@ -116,6 +125,14 @@ public ImmutableList<ExecutionRootPath> getTransitiveSystemIncludeDirectories()
return transitiveSystemIncludeDirectories;
}

public String getIncludePrefix() {
return includePrefix;
}

public String getStripIncludePrefix() {
return stripIncludePrefix;
}

public static Builder builder() {
return new Builder();
}
Expand All @@ -135,6 +152,9 @@ public static class Builder {
private final ImmutableList.Builder<ExecutionRootPath> transitiveSystemIncludeDirectories =
ImmutableList.builder();

private String includePrefix = "";
private String stripIncludePrefix = "";

@CanIgnoreReturnValue
public Builder addSources(Iterable<ArtifactLocation> sources) {
this.sources.addAll(sources);
Expand Down Expand Up @@ -204,6 +224,18 @@ public Builder addTransitiveSystemIncludeDirectories(
return this;
}

@CanIgnoreReturnValue
public Builder setIncludePrefix(String includePrefix) {
this.includePrefix = includePrefix;
return this;
}

@CanIgnoreReturnValue
public Builder setStripIncludePrefix(String stripIncludePrefix) {
this.stripIncludePrefix = stripIncludePrefix;
return this;
}

public CIdeInfo build() {
return new CIdeInfo(
sources.build(),
Expand All @@ -213,7 +245,9 @@ public CIdeInfo build() {
transitiveIncludeDirectories.build(),
transitiveQuoteIncludeDirectories.build(),
transitiveDefines.build(),
transitiveSystemIncludeDirectories.build());
transitiveSystemIncludeDirectories.build(),
includePrefix,
stripIncludePrefix);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@

import com.google.common.collect.ImmutableList;
import com.google.idea.blaze.base.bazel.BuildSystemProvider;
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.WorkspacePath;
import com.google.idea.blaze.base.model.primitives.WorkspaceRoot;
import com.google.idea.blaze.base.settings.Blaze;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.registry.Registry;
import java.io.File;
import javax.annotation.Nullable;

Expand All @@ -36,22 +39,27 @@
* target is built.
*/
public class ExecutionRootPathResolver {

private static final Logger LOG = Logger.getInstance(ExecutionRootPathResolver.class);
private final static String externalPrefix = "external";
final static File externalPath = new File(externalPrefix);
private final ImmutableList<String> buildArtifactDirectories;
private final File executionRoot;
private final File outputBase;
private final WorkspacePathResolver workspacePathResolver;
private final TargetMap targetMap;

public ExecutionRootPathResolver(
BuildSystemProvider buildSystemProvider,
WorkspaceRoot workspaceRoot,
File executionRoot,
File outputBase,
WorkspacePathResolver workspacePathResolver) {
WorkspacePathResolver workspacePathResolver,
TargetMap targetMap) {
this.buildArtifactDirectories = buildArtifactDirectories(buildSystemProvider, workspaceRoot);
this.executionRoot = executionRoot;
this.outputBase = outputBase;
this.workspacePathResolver = workspacePathResolver;
this.targetMap = targetMap;
}

@Nullable
Expand All @@ -66,7 +74,8 @@ public static ExecutionRootPathResolver fromProject(Project project) {
WorkspaceRoot.fromProject(project),
projectData.getBlazeInfo().getExecutionRoot(),
projectData.getBlazeInfo().getOutputBase(),
projectData.getWorkspacePathResolver());
projectData.getWorkspacePathResolver(),
projectData.getTargetMap());
}

private static ImmutableList<String> buildArtifactDirectories(
Expand Down Expand Up @@ -104,9 +113,26 @@ public ImmutableList<File> resolveToIncludeDirectories(ExecutionRootPath path) {
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(Registry.is("bazel.sync.resolve.virtual.includes") &&
VirtualIncludesHandler.containsVirtualInclude(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);
} catch (Throwable throwable) {
LOG.error("Failed to resolve virtual includes for " + path, throwable);
}

return resolved.isEmpty()
? ImmutableList.of(path.getFileRootedAt(executionRoot))
: resolved;
} else {
return ImmutableList.of(path.getFileRootedAt(executionRoot));
}
}
if (firstPathComponent.equals("external")) { // In external workspace
if (firstPathComponent.equals(externalPrefix)) { // 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));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package com.google.idea.blaze.base.sync.workspace;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.idea.blaze.base.ideinfo.CIdeInfo;
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.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.util.io.FileUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.io.File;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;

/**
* Utility class designed to convert execroot {@code _virtual_includes} references to
* either external or local workspace.
* <p>
* Virtual includes are generated for targets with strip_include_prefix attribute
* and are stored for external workspaces in
* <p>
* {@code bazel-out/.../bin/external/.../_virtual_includes/...}
* <p>
* or for local workspace in
* <p>
* {@code bazel-out/.../bin/.../_virtual_includes/...}
*/
class VirtualIncludesHandler {
private static final Logger LOG = Logger.getInstance(VirtualIncludesHandler.class);
final static Path VIRTUAL_INCLUDES_DIRECTORY = Path.of("_virtual_includes");
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 List<Path> splitExecutionPath(ExecutionRootPath executionRootPath) {
return Lists.newArrayList(executionRootPath.getAbsoluteOrRelativeFile().toPath().iterator());
}

static boolean containsVirtualInclude(ExecutionRootPath executionRootPath) {
return splitExecutionPath(executionRootPath).contains(VIRTUAL_INCLUDES_DIRECTORY);
}

/**
* Resolves execution root path to {@code _virtual_includes} directory to the matching workspace location
*
* @return list of resolved paths if required information is obtained from execution root path and target data
* or empty list if resolution has failed
*/
@NotNull
static ImmutableList<File> resolveVirtualInclude(ExecutionRootPath executionRootPath,
File externalWorkspacePath,
WorkspacePathResolver workspacePathResolver,
TargetMap targetMap)
{
TargetKey key = null;
try {
key = guessTargetKey(executionRootPath);
} catch (IndexOutOfBoundsException exception) {
// report to intellij EA
LOG.error(
"Failed to detect target from execution root path: " + executionRootPath,
exception);
}

if (key == null) {
return ImmutableList.of();
}

TargetIdeInfo info = targetMap.get(key);
if (info == null) {
return ImmutableList.of();
}

CIdeInfo 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");
return ImmutableList.of();
}

String stripPrefix = info.getcIdeInfo().getStripIncludePrefix();
if (!stripPrefix.isEmpty()) {
String extrenalWorkspaceName = key.getLabel().externalWorkspaceName();
WorkspacePath workspacePath = key.getLabel().blazePackage();
if (key.getLabel().externalWorkspaceName() != null) {
ExecutionRootPath external = new ExecutionRootPath(
ExecutionRootPathResolver.externalPath.toPath()
.resolve(extrenalWorkspaceName)
.resolve(workspacePath.toString())
.resolve(stripPrefix).toString());

return ImmutableList.of(external.getFileRootedAt(externalWorkspacePath));
} else {
return workspacePathResolver.resolveToIncludeDirectories(
new WorkspacePath(workspacePath, stripPrefix));
}
} else {
return ImmutableList.of();
}

}

/**
* @throws IndexOutOfBoundsException if executionRootPath has _virtual_includes but
* its content is unexpected
*/
@Nullable
private static TargetKey guessTargetKey(ExecutionRootPath executionRootPath) {
List<Path> split = splitExecutionPath(executionRootPath);
int virtualIncludesIdx = split.indexOf(VIRTUAL_INCLUDES_DIRECTORY);

if (virtualIncludesIdx > -1) {
String externalWorkspaceName =
split.get(EXTERNAL_DIRECTORY_IDX)
.equals(ExecutionRootPathResolver.externalPath.toPath())
? split.get(EXTERNAL_WORKSPACE_NAME_IDX).toString()
: null;

int workspacePathStart = externalWorkspaceName != null
? WORKSPACE_PATH_START_FOR_EXTERNAL_WORKSPACE
: WORKSPACE_PATH_START_FOR_LOCAL_WORKSPACE;

List<Path> workspacePaths = (workspacePathStart <= virtualIncludesIdx)
? split.subList(workspacePathStart, virtualIncludesIdx)
: Collections.emptyList();

String workspacePathString =
FileUtil.toSystemIndependentName(
workspacePaths.stream().reduce(Path.of(""), Path::resolve).toString());

TargetName target = TargetName.create(split.get(virtualIncludesIdx + 1).toString());
WorkspacePath workspacePath = WorkspacePath.createIfValid(workspacePathString);
if (workspacePath == null) {
return null;
}

return TargetKey.forPlainTarget(
Label.create(externalWorkspaceName, workspacePath, target));
} else {
return null;
}
}
}
Loading

0 comments on commit 0630fee

Please sign in to comment.