Skip to content

Commit

Permalink
Allow tree artifacts to be source or header inputs to cc_common.compi…
Browse files Browse the repository at this point in the history
…le()

This is already supported in cc_library, and would make the behavior
more consistent.

PiperOrigin-RevId: 362295588
  • Loading branch information
googlewalt authored and copybara-github committed Mar 11, 2021
1 parent d2f93fd commit 596653d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,9 @@ public CcCompilationHelper addPrivateHeaders(Iterable<Pair<Artifact, Label>> pri
}

private CcCompilationHelper addPrivateHeader(Artifact privateHeader, Label label) {
boolean isHeader = CppFileTypes.CPP_HEADER.matches(privateHeader.getExecPath());
boolean isHeader =
CppFileTypes.CPP_HEADER.matches(privateHeader.getExecPath())
|| privateHeader.isTreeArtifact();
boolean isTextualInclude =
CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(privateHeader.getExecPath());
Preconditions.checkState(isHeader || isTextualInclude);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1985,17 +1985,20 @@ protected Tuple compile(
CppFileTypes.CPP_SOURCE,
CppFileTypes.C_SOURCE,
CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR,
CppFileTypes.ASSEMBLER));
CppFileTypes.ASSEMBLER),
/* allowAnyTreeArtifacts= */ true);
validateExtensions(
"public_hdrs",
publicHeaders,
FileTypeSet.of(CppFileTypes.CPP_HEADER),
FileTypeSet.of(CppFileTypes.CPP_HEADER));
FileTypeSet.of(CppFileTypes.CPP_HEADER),
/* allowAnyTreeArtifacts= */ true);
validateExtensions(
"private_hdrs",
privateHeaders,
FileTypeSet.of(CppFileTypes.CPP_HEADER),
FileTypeSet.of(CppFileTypes.CPP_HEADER));
FileTypeSet.of(CppFileTypes.CPP_HEADER),
/* allowAnyTreeArtifacts= */ true);

if (disallowNopicOutputs && disallowPicOutputs) {
throw Starlark.errorf("Either PIC or no PIC actions have to be created.");
Expand Down Expand Up @@ -2244,11 +2247,20 @@ protected CcCompilationOutputs createCompilationOutputsFromStarlark(
Object objectsObject, Object picObjectsObject) throws EvalException {
CcCompilationOutputs.Builder ccCompilationOutputsBuilder = CcCompilationOutputs.builder();
NestedSet<Artifact> objects = convertToNestedSet(objectsObject, Artifact.class, "objects");
validateExtensions("objects", objects.toList(), Link.OBJECT_FILETYPES, Link.OBJECT_FILETYPES);
validateExtensions(
"objects",
objects.toList(),
Link.OBJECT_FILETYPES,
Link.OBJECT_FILETYPES,
/* allowAnyTreeArtifacts= */ false);
NestedSet<Artifact> picObjects =
convertToNestedSet(picObjectsObject, Artifact.class, "pic_objects");
validateExtensions(
"pic_objects", picObjects.toList(), Link.OBJECT_FILETYPES, Link.OBJECT_FILETYPES);
"pic_objects",
picObjects.toList(),
Link.OBJECT_FILETYPES,
Link.OBJECT_FILETYPES,
/* allowAnyTreeArtifacts= */ false);
ccCompilationOutputsBuilder.addObjectFiles(objects.toList());
ccCompilationOutputsBuilder.addPicObjectFiles(picObjects.toList());
return ccCompilationOutputsBuilder.build();
Expand All @@ -2258,9 +2270,13 @@ private static void validateExtensions(
String paramName,
List<Artifact> files,
FileTypeSet validFileTypeSet,
FileTypeSet fileTypeForErrorMessage)
FileTypeSet fileTypeForErrorMessage,
boolean allowAnyTreeArtifacts)
throws EvalException {
for (Artifact file : files) {
if (allowAnyTreeArtifacts && file.isTreeArtifact()) {
continue;
}
if (!validFileTypeSet.matches(file.getFilename())) {
throw Starlark.errorf(
"'%s' has wrong extension. The list of possible extensions for '%s' is: %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5586,6 +5586,21 @@ public void testPossiblePublicHdrExtensions() throws Exception {
doTestPossibleExtensionsOfSrcsAndHdrs("public_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
}

@Test
public void testTreeArtifactSrcs() throws Exception {
doTestTreeAtrifactInSrcsAndHdrs("srcs");
}

@Test
public void testTreeArtifactPrivateHdrs() throws Exception {
doTestTreeAtrifactInSrcsAndHdrs("private_hdrs");
}

@Test
public void testTreeArtifactPublicHdrs() throws Exception {
doTestTreeAtrifactInSrcsAndHdrs("public_hdrs");
}

@Test
public void testWrongSrcsExtensionGivesError() throws Exception {
doTestWrongExtensionOfSrcsAndHdrs("srcs");
Expand Down Expand Up @@ -6120,6 +6135,35 @@ private void doTestPossibleExtensionsOfSrcsAndHdrs(String attrName, List<String>
}
}

private void doTestTreeAtrifactInSrcsAndHdrs(String attrName) throws Exception {
createFiles(scratch, "tools/build_defs/foo");
reporter.removeHandler(failFastHandler);

scratch.file(
"bar/create_tree_artifact.bzl",
"def _impl(ctx):",
" tree = ctx.actions.declare_directory('dir')",
" ctx.actions.run_shell(",
" outputs = [tree],",
" inputs = [],",
" arguments = [tree.path],",
" command = 'mkdir $1',",
" )",
" return [DefaultInfo(files = depset([tree]))]",
"create_tree_artifact = rule(implementation = _impl)");
scratch.file(
"bar/BUILD",
"load('//tools/build_defs/foo:extension.bzl', 'cc_starlark_library')",
"load(':create_tree_artifact.bzl', 'create_tree_artifact')",
"create_tree_artifact(name = 'tree_artifact')",
"cc_starlark_library(",
" name = 'starlark_lib',",
" " + attrName + " = [':tree_artifact'],",
")");
getConfiguredTarget("//bar:starlark_lib");
assertNoEvents();
}

private void doTestCcOutputsWrongExtension(String attrName, String paramName) throws Exception {
setupCcOutputsTest();
scratch.file(
Expand Down

1 comment on commit 596653d

@konste
Copy link

@konste konste commented on 596653d Apr 30, 2021

Choose a reason for hiding this comment

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

Do I get it right that with this change TreeArtifacts are supported for sources and headers in cc_common.compile but still NOT supported in cc_common.link? compilation_outputs returned by cc_common.compile is also a TreeArtifact and when it is fed to cc_common.link it does not understand it. How TreeArtifacts supposed to be used when compile understands them but link does not?

Please sign in to comment.