Skip to content

Commit

Permalink
Include all necessary sources in prebuild test data.
Browse files Browse the repository at this point in the history
For cc compilation, we need access to a number of sources from the workspace, from both the target dependencies and from the cc toolchain. Add these to the `sources` directory created by the `test_data` rule.

To achieve this, add some explicit `test_mode` support to the existing cc rules in `build_dependencies.bzl` and export all the required sourcs to `test_projects.bzl` which can then add them to the `sources` directory.

Also add the cc related output groups to the `OutputInfo` returned by `PrebuiltProjectFakeDependencyBuilder` so that it behaves more like the real implementation.

PiperOrigin-RevId: 576799685
  • Loading branch information
Googler authored and copybara-github committed Oct 26, 2023
1 parent 8cb2baf commit ef1a33c
Show file tree
Hide file tree
Showing 16 changed files with 114 additions and 93 deletions.
77 changes: 49 additions & 28 deletions aspect/build_dependencies.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def _package_dependencies_impl(target, ctx):
artifact_info_file = java_info_file,
qsync_aars = target[DependenciesInfo].aars.to_list(),
qsync_gensrcs = target[DependenciesInfo].gensrcs.to_list(),
cc_headers = target[DependenciesInfo].cc_headers,
cc_headers = target[DependenciesInfo].cc_headers.to_list(),
cc_info_file = cc_info_file + [target[DependenciesInfo].cc_toolchain_info.file] if target[DependenciesInfo].cc_toolchain_info else [],
)]

Expand Down Expand Up @@ -60,10 +60,34 @@ DependenciesInfo = provider(
"test_mode_own_files": "a structure describing artifacts required when the target is requested within the project scope",
"cc_info": "a structure containing info required to compile cc sources",
"cc_headers": "a list of generated headers required to compile cc sources",
"cc_srcs_for_test": "a list of sources (e.g. headers) required to compile cc sources in integratrion tests",
"cc_toolchain_info": "struct containing cc toolchain info, with keys file (the output file) and id (unique ID for the toolchain info, referred to from elsewhere)",
},
)

def create_dependencies_info(
compile_time_jars = depset(),
target_to_artifacts = {},
aars = depset(),
gensrcs = depset(),
test_mode_own_files = None,
cc_info = None,
cc_headers = depset(),
cc_srcs_for_test = depset(),
cc_toolchain_info = None):
"""A helper function to create a DependenciesInfo provider instance."""
return DependenciesInfo(
compile_time_jars = compile_time_jars,
target_to_artifacts = target_to_artifacts,
aars = aars,
gensrcs = gensrcs,
test_mode_own_files = test_mode_own_files,
cc_info = cc_info,
cc_headers = cc_headers,
cc_srcs_for_test = cc_srcs_for_test,
cc_toolchain_info = cc_toolchain_info,
)

def _encode_target_info_proto(target_to_artifacts):
contents = []
for label, target_info in target_to_artifacts.items():
Expand All @@ -89,7 +113,7 @@ def _encode_cc_info_proto(label, cc_info):
quote_include_directories = cc_info.transitive_quote_include_directory,
system_include_directories = cc_info.transitive_system_include_directory,
framework_include_directories = cc_info.framework_include_directory,
gen_hdrs = cc_info.gen_headers,
gen_hdrs = cc_info.gen_headers.to_list(),
toolchain_id = cc_info.toolchain_id,
),
]),
Expand Down Expand Up @@ -346,29 +370,39 @@ def _get_followed_cc_dependency_info(rule):
return cc_toolchain_target[DependenciesInfo]
return None

def _collect_own_and_dependency_cc_info(target, dependency_info):
def _collect_own_and_dependency_cc_info(target, dependency_info, test_mode):
compilation_context = target[CcInfo].compilation_context
cc_toolchain_info = None
src_headers = depset()
if dependency_info:
cc_toolchain_info = dependency_info.cc_toolchain_info
if test_mode:
src_headers = dependency_info.cc_srcs_for_test

gen_headers = []
gen_headers = depset()
compilation_info = None
if compilation_context:
gen_headers = [f for f in compilation_context.headers.to_list() if not f.is_source]
gen_headers = depset([f for f in compilation_context.headers.to_list() if not f.is_source])

if test_mode:
src_headers = depset(
[f for f in compilation_context.headers.to_list() if f.is_source],
transitive = [src_headers],
)

compilation_info = struct(
transitive_defines = compilation_context.defines.to_list(),
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() + compilation_context.external_includes.to_list(),
framework_include_directory = compilation_context.framework_includes.to_list(),
gen_headers = [f.path for f in gen_headers],
gen_headers = gen_headers,
toolchain_id = cc_toolchain_info.id if cc_toolchain_info else None,
)
return struct(
compilation_info = compilation_info,
gen_headers = gen_headers,
src_headers = src_headers,
cc_toolchain_info = cc_toolchain_info,
)

Expand All @@ -390,7 +424,7 @@ def _collect_dependencies_core_impl(
test_mode,
)
if CcInfo in target:
dep_infos.append(_collect_cc_dependencies_core_impl(target, ctx))
dep_infos.append(_collect_cc_dependencies_core_impl(target, ctx, test_mode))
if cc_common.CcToolchainInfo in target:
dep_infos.append(_collect_cc_toolchain_info(target, ctx))
return dep_infos
Expand Down Expand Up @@ -432,31 +466,24 @@ def _collect_java_dependencies_core_impl(
)

return [
DependenciesInfo(
create_dependencies_info(
target_to_artifacts = target_to_artifacts,
compile_time_jars = compile_jars,
aars = aars,
gensrcs = gensrcs,
test_mode_own_files = test_mode_own_files,
cc_info = None,
cc_headers = depset(),
cc_toolchain_info = None,
),
]

def _collect_cc_dependencies_core_impl(target, ctx):
def _collect_cc_dependencies_core_impl(target, ctx, test_mode):
dependency_info = _get_followed_cc_dependency_info(ctx.rule)

cc_info = _collect_own_and_dependency_cc_info(target, dependency_info)
cc_info = _collect_own_and_dependency_cc_info(target, dependency_info, test_mode)

return DependenciesInfo(
target_to_artifacts = {},
compile_time_jars = depset(),
aars = depset(),
gensrcs = depset(),
test_mode_own_files = None,
return create_dependencies_info(
cc_info = cc_info.compilation_info,
cc_headers = cc_info.gen_headers,
cc_srcs_for_test = cc_info.src_headers,
cc_toolchain_info = cc_info.cc_toolchain_info,
)

Expand All @@ -465,7 +492,7 @@ def _collect_cc_toolchain_info(target, ctx):

cpp_fragment = ctx.fragments.cpp

# TODO(b/301235884): This logis is not quite right. `ctx` here is the context for for
# TODO(b/301235884): This logic is not quite right. `ctx` here is the context for the
# cc_toolchain target itself, so the `features` and `disabled_features` were using here are
# for the cc_toolchain, not the individual targets that this information will ultimately be
# used for. Instead, we should attach `toolchain_info` itself to the `DependenciesInfo`
Expand Down Expand Up @@ -525,14 +552,8 @@ def _collect_cc_toolchain_info(target, ctx):
),
)

return DependenciesInfo(
target_to_artifacts = {},
compile_time_jars = depset(),
aars = depset(),
gensrcs = depset(),
test_mode_own_files = None,
cc_info = None,
cc_headers = depset(),
return create_dependencies_info(
cc_srcs_for_test = depset([f for f in toolchain_info.all_files.to_list() if f.is_source]),
cc_toolchain_info = struct(file = cc_toolchain_file, id = toolchain_id),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.BuildArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaArtifacts;
import com.google.devtools.intellij.qsync.CcCompilationInfoOuterClass.CcCompilationInfo;
import com.google.idea.blaze.base.bazel.BazelExitCodeException;
import com.google.idea.blaze.base.bazel.BazelExitCodeException.ThrowOption;
Expand Down Expand Up @@ -202,7 +202,7 @@ private OutputInfo createOutputInfo(
BlazeBuildOutputs blazeBuildOutputs, Set<OutputGroup> outputGroups) throws BuildException {
GroupedOutputArtifacts allArtifacts =
new GroupedOutputArtifacts(blazeBuildOutputs, outputGroups);
ImmutableSet.Builder<BuildArtifacts> artifactInfoFilesBuilder = ImmutableSet.builder();
ImmutableSet.Builder<JavaArtifacts> artifactInfoFilesBuilder = ImmutableSet.builder();
ImmutableSet.Builder<CcCompilationInfo> ccInfoBuilder = ImmutableSet.builder();

for (OutputArtifact artifactInfoFile : allArtifacts.get(OutputGroup.ARTIFACT_INFO_FILE)) {
Expand All @@ -222,8 +222,8 @@ private OutputInfo createOutputInfo(
blazeBuildOutputs.buildResult.exitCode);
}

private BuildArtifacts readArtifactInfoFile(BlazeArtifact file) throws BuildException {
return readProtoFile(BuildArtifacts.newBuilder(), file).build();
private JavaArtifacts readArtifactInfoFile(BlazeArtifact file) throws BuildException {
return readProtoFile(JavaArtifacts.newBuilder(), file).build();
}

private CcCompilationInfo readCcInfoFile(BlazeArtifact file) throws BuildException {
Expand Down
10 changes: 5 additions & 5 deletions base/src/com/google/idea/blaze/base/qsync/OutputInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.BuildArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaArtifacts;
import com.google.devtools.intellij.qsync.CcCompilationInfoOuterClass.CcCompilationInfo;
import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
import com.google.idea.blaze.common.Label;
Expand All @@ -34,7 +34,7 @@ public abstract class OutputInfo {
GroupedOutputArtifacts.EMPTY, ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), 0);

/** Returns the proto containing details of artifacts per target produced by the aspect. */
public abstract ImmutableSet<BuildArtifacts> getArtifactInfo();
public abstract ImmutableSet<JavaArtifacts> getArtifactInfo();

public abstract ImmutableSet<CcCompilationInfo> getCcCompilationInfo();

Expand Down Expand Up @@ -74,7 +74,7 @@ public static Builder builder() {

public static OutputInfo create(
GroupedOutputArtifacts allArtifacts,
ImmutableSet<BuildArtifacts> artifacts,
ImmutableSet<JavaArtifacts> artifacts,
ImmutableSet<CcCompilationInfo> ccInfo,
ImmutableSet<Label> targetsWithErrors,
int exitCode) {
Expand All @@ -92,9 +92,9 @@ public static OutputInfo create(
@AutoValue.Builder
public abstract static class Builder {

public abstract Builder setArtifactInfo(ImmutableSet<BuildArtifacts> value);
public abstract Builder setArtifactInfo(ImmutableSet<JavaArtifacts> value);

public abstract Builder setArtifactInfo(BuildArtifacts... values);
public abstract Builder setArtifactInfo(JavaArtifacts... values);

public abstract Builder setCcCompilationInfo(ImmutableSet<CcCompilationInfo> value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.TargetArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaTargetArtifacts;
import com.google.idea.blaze.common.Label;
import java.nio.file.Path;
import java.util.stream.Stream;
Expand Down Expand Up @@ -57,7 +57,7 @@ public abstract class ArtifactInfo {

public abstract ImmutableSet<Path> srcJars();

public static ArtifactInfo create(TargetArtifacts proto) {
public static ArtifactInfo create(JavaTargetArtifacts proto) {
// Note, the proto contains a list of sources, we take the parent as we want directories instead
return new AutoValue_ArtifactInfo(
Label.of(proto.getTarget()),
Expand All @@ -68,8 +68,8 @@ public static ArtifactInfo create(TargetArtifacts proto) {
proto.getSrcjarsList().stream().map(Path::of).collect(toImmutableSet()));
}

public TargetArtifacts toProto() {
return TargetArtifacts.newBuilder()
public JavaTargetArtifacts toProto() {
return JavaTargetArtifacts.newBuilder()
.setTarget(label().toString())
.addAllJars(jars().stream().map(Path::toString).collect(toImmutableList()))
.addAllIdeAars(ideAars().stream().map(Path::toString).collect(toImmutableList()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.ArtifactTrackerState;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.BuildArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.CachedArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.TargetArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaTargetArtifacts;
import com.google.devtools.intellij.qsync.CcCompilationInfoOuterClass.CcCompilationInfo;
import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStats;
Expand Down Expand Up @@ -220,7 +220,7 @@ public void clear() throws IOException {
}

private void saveState() throws IOException {
BuildArtifacts.Builder builder = BuildArtifacts.newBuilder();
JavaArtifacts.Builder builder = JavaArtifacts.newBuilder();
javaArtifacts.values().stream().map(ArtifactInfo::toProto).forEach(builder::addArtifacts);
CcCompilationInfo ccCompilationInfo = ccDepencenciesInfo.toProto();
CachedArtifacts.Builder cachedArtifactsBuilder = CachedArtifacts.newBuilder();
Expand Down Expand Up @@ -258,7 +258,7 @@ private void loadFromDisk() {
saved.getArtifactInfo().getArtifactsList().stream()
.map(ArtifactInfo::create)
.collect(toImmutableMap(ArtifactInfo::label, Function.identity())));
for (TargetArtifacts targetArtifact : saved.getArtifactInfo().getArtifactsList()) {
for (JavaTargetArtifacts targetArtifact : saved.getArtifactInfo().getArtifactsList()) {
ArtifactInfo artifactInfo = ArtifactInfo.create(targetArtifact);
javaArtifacts.put(artifactInfo.label(), artifactInfo);
}
Expand Down Expand Up @@ -409,7 +409,7 @@ public UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeConte
ImmutableMap<Path, Path> updated = cache(context, artifactMap);

this.cachePathToArtifactKeyMap.putAll(updated);
for (BuildArtifacts artifacts : outputInfo.getArtifactInfo()) {
for (JavaArtifacts artifacts : outputInfo.getArtifactInfo()) {
updateMaps(targets, artifacts);
}
CcDependenciesInfo.Builder ccDepsBuilder = ccDepencenciesInfo.toBuilder();
Expand Down Expand Up @@ -503,7 +503,7 @@ private ImmutableListMultimap<Boolean, OutputArtifact> getGensrcsByInclusion(
// files produced by the aspect.
SetMultimap<Label, String> genSrcsByTarget =
outputInfo.getArtifactInfo().stream()
.map(BuildArtifacts::getArtifactsList)
.map(JavaArtifacts::getArtifactsList)
.flatMap(List::stream)
.collect(
Multimaps.flatteningToMultimap(
Expand All @@ -530,8 +530,8 @@ private ImmutableListMultimap<Boolean, OutputArtifact> getGensrcsByInclusion(
* @param targets the list of targets that were expected to be built. (From blaze query)
* @param newArtifacts the artifacts that were actually built. From (blaze build)
*/
private void updateMaps(Set<Label> targets, BuildArtifacts newArtifacts) {
for (TargetArtifacts targetArtifacts : newArtifacts.getArtifactsList()) {
private void updateMaps(Set<Label> targets, JavaArtifacts newArtifacts) {
for (JavaTargetArtifacts targetArtifacts : newArtifacts.getArtifactsList()) {
ArtifactInfo artifactInfo = ArtifactInfo.create(targetArtifacts);
javaArtifacts.put(artifactInfo.label(), artifactInfo);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.BuildArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.TargetArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaArtifacts;
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaTargetArtifacts;
import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
import com.google.idea.blaze.base.filecache.ArtifactState;
import com.google.idea.blaze.base.qsync.ArtifactTracker.UpdateResult;
Expand Down Expand Up @@ -239,15 +239,15 @@ public void library_sources() throws Throwable {
.build())
.build())
.setArtifactInfo(
BuildArtifacts.newBuilder()
JavaArtifacts.newBuilder()
.addArtifacts(
TargetArtifacts.newBuilder()
JavaTargetArtifacts.newBuilder()
.setTarget("//test:test")
.addJars("out/test.jar")
.addSrcs("test/Test.java")
.build())
.addArtifacts(
TargetArtifacts.newBuilder()
JavaTargetArtifacts.newBuilder()
.setTarget("//test:anothertest")
.addJars("out/anothertest.jar")
.addSrcs("test/AnotherTest.java")
Expand Down Expand Up @@ -290,9 +290,9 @@ public void library_sources_unknown_lib() throws Throwable {
.build())
.build())
.setArtifactInfo(
BuildArtifacts.newBuilder()
JavaArtifacts.newBuilder()
.addArtifacts(
TargetArtifacts.newBuilder()
JavaTargetArtifacts.newBuilder()
.setTarget("//test:test")
.addJars("out/test.jar")
.addSrcs("test/Test.java")
Expand Down
Loading

0 comments on commit ef1a33c

Please sign in to comment.