Skip to content

Conversation

LeFrosch
Copy link
Collaborator

The PR adds a cache for generated headers and _virtual_include directories of a target to avoid placing header search paths into the execution root.

Header search paths in the execution root can cause unnecessary files to be loaded into the VFS, since indexing code might recursively iterate over all files in these directories.

@LeFrosch LeFrosch force-pushed the include-cache branch 4 times, most recently from 4163454 to fc050f2 Compare August 14, 2025 14:00
@LeFrosch LeFrosch marked this pull request as ready for review September 4, 2025 07:48
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Sep 4, 2025
@LeFrosch LeFrosch linked an issue Sep 26, 2025 that may be closed by this pull request
This was referenced Sep 26, 2025
@LeFrosch LeFrosch linked an issue Sep 26, 2025 that may be closed by this pull request
@LeFrosch
Copy link
Collaborator Author

LeFrosch commented Oct 1, 2025

Windows failure caused by:

.\pkg.c: fatal error C1083: Cannot open include file: 'msvc_recommended_pragmas.?
h': No such file or directory?

Maybe separate rules foreign cc tests, seem to be more restrictive then the general virtual includes tests.

this.sources.addAll(cIdeInfo.ruleContext().headers());
this.sources.addAll(cIdeInfo.ruleContext().textualHeaders());

// TODO: add compilation context direct headers here too? (they can contain duplicates, filter?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we update this comment with the testonly usage and drop todo then?


private const val MODULES_DIRECTORY = "modules"

abstract class AspectModuleWriter : AspectWriter {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add documentation here

addAllowedVfsRoots(roots);

Assertions.assertVfsLoads(myBazelInfo.executionRoot(), roots);
HeavyPlatformTestCase.cleanupApplicationCaches(myProject);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's leave a comment why do we want to have it here or maybe remove it if we don't need


http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's consider switching from sqlite to our own simple library built with cmake, I guess we can store it right in this repo and drop BUILD.sqlite.bazel and directly use configure_cmake?

.map(TargetKey::toString)
.collect(joining("\n"));

IssueOutput.warn(String.format("cc target depends on %d apple cc toolchains", entry.getKey()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's preserve the old message as it looks to be more descriptive

fun resolve(target: TargetKey, artifact: ArtifactLocation): Path? {
if (!artifact.isGenerated) return null

val path = target.cacheDirectory().resolve(stripVirtualPrefix(artifact.relativePath))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npe on resolve

var size = 0L

try {
for (item in Files.walk(path)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if (Files.exists(cacheDirectory)) {
// I have a feeling that this will be really slow on Windows, so let's try to avoid this
NioFiles.deleteRecursively(cacheDirectory)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's leave a comment that we'd like to later replace it with rename and delete async

if (importSettings != null) {
BlazeDataStorage.getProjectDataDir(importSettings).toPath().resolve(CC_INCLUDES_CACHE_DIR)
} else {
Path.of(project.basePath, CC_INCLUDES_CACHE_DIR)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npe from basepath


private fun findVirtualIncludesDirectory(bazelBin: Path, label: Label): Path? {
val path = bazelBin
.applyIf(label.externalWorkspaceName() != null) { resolve("external").resolve(label.externalWorkspaceName()) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXTERNAL_BAZEL_DIR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Awaiting review from Bazel team on PRs product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
Status: Untriaged
Development

Successfully merging this pull request may close these issues.

Bazel protobuf generated pb.h files are not indexed auto-complete etc support for protobuf generated code
4 participants