Skip to content

Commit 942f7cf

Browse files
oquenchilcopybara-github
authored andcommitted
C++: Fixes bug in C++ API with external repo aspects
Reported in: bazelbuild#8254 Added test testApiWithAspectsOnTargetsInExternalRepos() RELNOTES:none PiperOrigin-RevId: 247412486
1 parent 1df1635 commit 942f7cf

File tree

2 files changed

+105
-43
lines changed

2 files changed

+105
-43
lines changed

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,11 @@ protected Label getCallerLabel(Location location, SkylarkActionFactory actions,
14421442
try {
14431443
label =
14441444
Label.create(
1445-
actions.getActionConstructionContext().getActionOwner().getLabel().getPackageName(),
1445+
actions
1446+
.getActionConstructionContext()
1447+
.getActionOwner()
1448+
.getLabel()
1449+
.getPackageIdentifier(),
14461450
name);
14471451
} catch (LabelSyntaxException e) {
14481452
throw new EvalException(location, e);

src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java

+100-42
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.google.devtools.build.lib.testutil.Scratch;
5757
import com.google.devtools.build.lib.testutil.TestConstants;
5858
import com.google.devtools.build.lib.util.Pair;
59+
import com.google.devtools.build.lib.vfs.FileSystemUtils;
5960
import com.google.devtools.build.lib.vfs.PathFragment;
6061
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
6162
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.MakeVariable;
@@ -5197,6 +5198,71 @@ private static void createFiles(Scratch scratch, String bzlFilePath) throws Exce
51975198
createFiles(scratch, bzlFilePath, "", "");
51985199
}
51995200

5201+
@Test
5202+
public void testTransitiveLinkWithDeps() throws Exception {
5203+
setupTestTransitiveLink(scratch, " linking_contexts = dep_linking_contexts");
5204+
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5205+
assertThat(target).isNotNull();
5206+
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5207+
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
5208+
.containsAllOf("bin foo/libdep1.a", "bin foo/libdep2.a");
5209+
}
5210+
5211+
@Test
5212+
public void testTransitiveLinkForDynamicLibrary() throws Exception {
5213+
setupTestTransitiveLink(scratch, "output_type = 'dynamic_library'");
5214+
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5215+
assertThat(target).isNotNull();
5216+
LibraryToLink library = (LibraryToLink) getMyInfoFromTarget(target).getValue("library");
5217+
assertThat(library).isNotNull();
5218+
Object executable = getMyInfoFromTarget(target).getValue("executable");
5219+
assertThat(EvalUtils.isNullOrNone(executable)).isTrue();
5220+
}
5221+
5222+
@Test
5223+
public void testTransitiveLinkForExecutable() throws Exception {
5224+
setupTestTransitiveLink(scratch, "output_type = 'executable'");
5225+
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5226+
assertThat(target).isNotNull();
5227+
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5228+
assertThat(executable).isNotNull();
5229+
Object library = getMyInfoFromTarget(target).getValue("library");
5230+
assertThat(EvalUtils.isNullOrNone(library)).isTrue();
5231+
}
5232+
5233+
@Test
5234+
public void testTransitiveLinkWithCompilationOutputs() throws Exception {
5235+
setupTestTransitiveLink(scratch, "compilation_outputs=objects");
5236+
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5237+
assertThat(target).isNotNull();
5238+
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5239+
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
5240+
.contains("src foo/file.o");
5241+
}
5242+
5243+
@Test
5244+
public void testApiWithAspectsOnTargetsInExternalRepos() throws Exception {
5245+
if (!AnalysisMock.get().isThisBazel()) {
5246+
return;
5247+
}
5248+
createFilesForTestingCompilation(
5249+
scratch, "tools/build_defs/foo", /* compileProviderLines= */ "");
5250+
FileSystemUtils.appendIsoLatin1(
5251+
scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
5252+
scratch.file("/r/WORKSPACE");
5253+
scratch.file("/r/p/BUILD", "cc_library(", " name = 'a',", " srcs = ['a.cc'],", ")");
5254+
invalidatePackages();
5255+
scratch.file(
5256+
"b/BUILD",
5257+
"load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
5258+
"cc_skylark_library(",
5259+
" name = 'b',",
5260+
" srcs = ['b.cc'],",
5261+
" aspect_deps = ['@r//p:a']",
5262+
")");
5263+
assertThat(getConfiguredTarget("//b:b")).isNotNull();
5264+
}
5265+
52005266
private static void createFiles(
52015267
Scratch scratch, String bzlFilePath, String compileProviderLines, String linkProviderLines)
52025268
throws Exception {
@@ -5208,6 +5274,39 @@ private static void createFiles(
52085274
scratch.file(
52095275
bzlFilePath + "/extension.bzl",
52105276
"load('//myinfo:myinfo.bzl', 'MyInfo')",
5277+
"def _cc_aspect_impl(target, ctx):",
5278+
" toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]",
5279+
" feature_configuration = cc_common.configure_features(",
5280+
" cc_toolchain = toolchain,",
5281+
" requested_features = ctx.features,",
5282+
" unsupported_features = ctx.disabled_features,",
5283+
" )",
5284+
" (compilation_context, compilation_outputs) = cc_common.compile(",
5285+
" actions = ctx.actions,",
5286+
" feature_configuration = feature_configuration,",
5287+
" cc_toolchain = toolchain,",
5288+
" name = ctx.label.name + '_aspect',",
5289+
" srcs = ctx.rule.files.srcs,",
5290+
" public_hdrs = ctx.rule.files.hdrs,",
5291+
" )",
5292+
" (linking_context, linking_outputs) = (",
5293+
" cc_common.create_linking_context_from_compilation_outputs(",
5294+
" actions = ctx.actions,",
5295+
" feature_configuration = feature_configuration,",
5296+
" name = ctx.label.name + '_aspect',",
5297+
" cc_toolchain = toolchain,",
5298+
" compilation_outputs = compilation_outputs,",
5299+
" )",
5300+
" )",
5301+
" return []",
5302+
"_cc_aspect = aspect(",
5303+
" implementation = _cc_aspect_impl,",
5304+
" attrs = {",
5305+
" '_cc_toolchain': attr.label(default ="
5306+
+ " '@bazel_tools//tools/cpp:current_cc_toolchain'),",
5307+
" },",
5308+
fragments,
5309+
")",
52115310
"def _cc_skylark_library_impl(ctx):",
52125311
" dep_compilation_contexts = []",
52135312
" dep_linking_contexts = []",
@@ -5262,6 +5361,7 @@ private static void createFiles(
52625361
" '_additional_inputs': attr.label_list(allow_files=True,"
52635362
+ " default=['//foo:script.lds']),",
52645363
" '_deps': attr.label_list(default=['//foo:dep1', '//foo:dep2']),",
5364+
" 'aspect_deps': attr.label_list(aspects=[_cc_aspect]),",
52655365
" '_cc_toolchain': attr.label(default =",
52665366
" configuration_field(fragment = 'cpp', name = 'cc_toolchain'))",
52675367
" },",
@@ -5399,48 +5499,6 @@ private void setupCcOutputsTest() throws Exception {
53995499
")");
54005500
}
54015501

5402-
@Test
5403-
public void testTransitiveLinkWithDeps() throws Exception {
5404-
setupTestTransitiveLink(scratch, " linking_contexts = dep_linking_contexts");
5405-
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5406-
assertThat(target).isNotNull();
5407-
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5408-
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
5409-
.containsAllOf("bin foo/libdep1.a", "bin foo/libdep2.a");
5410-
}
5411-
5412-
@Test
5413-
public void testTransitiveLinkForDynamicLibrary() throws Exception {
5414-
setupTestTransitiveLink(scratch, "output_type = 'dynamic_library'");
5415-
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5416-
assertThat(target).isNotNull();
5417-
LibraryToLink library = (LibraryToLink) getMyInfoFromTarget(target).getValue("library");
5418-
assertThat(library).isNotNull();
5419-
Object executable = getMyInfoFromTarget(target).getValue("executable");
5420-
assertThat(EvalUtils.isNullOrNone(executable)).isTrue();
5421-
}
5422-
5423-
@Test
5424-
public void testTransitiveLinkForExecutable() throws Exception {
5425-
setupTestTransitiveLink(scratch, "output_type = 'executable'");
5426-
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5427-
assertThat(target).isNotNull();
5428-
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5429-
assertThat(executable).isNotNull();
5430-
Object library = getMyInfoFromTarget(target).getValue("library");
5431-
assertThat(EvalUtils.isNullOrNone(library)).isTrue();
5432-
}
5433-
5434-
@Test
5435-
public void testTransitiveLinkWithCompilationOutputs() throws Exception {
5436-
setupTestTransitiveLink(scratch, "compilation_outputs=objects");
5437-
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
5438-
assertThat(target).isNotNull();
5439-
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
5440-
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
5441-
.contains("src foo/file.o");
5442-
}
5443-
54445502
private static void setupTestTransitiveLink(Scratch scratch, String... additionalLines)
54455503
throws Exception {
54465504
String fragments = " fragments = ['google_cpp', 'cpp'],";

0 commit comments

Comments
 (0)