Skip to content

Commit

Permalink
Automated rollback of commit f6c4e62.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Blaze crashes when building DriveFS - see b/219077770

*** Original change description ***

Changes necessary for Starlark cc_library

Blaze changes for cc_library.bzl

RELNOTES:none
PiperOrigin-RevId: 428122831
  • Loading branch information
tetromino authored and copybara-github committed Feb 12, 2022
1 parent 152e705 commit eeec121
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcLinkingOutputsApi;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import javax.annotation.Nullable;
Expand All @@ -35,14 +36,17 @@ public class CcLinkingOutputs implements CcLinkingOutputsApi<Artifact, LtoBacken
@Nullable private final Artifact executable;

private final ImmutableList<LtoBackendArtifacts> allLtoArtifacts;
private final ImmutableList<Artifact> linkActionInputs;

private CcLinkingOutputs(
LibraryToLink libraryToLink,
Artifact executable,
ImmutableList<LtoBackendArtifacts> allLtoArtifacts) {
ImmutableList<LtoBackendArtifacts> allLtoArtifacts,
ImmutableList<Artifact> linkActionInputs) {
this.libraryToLink = libraryToLink;
this.executable = executable;
this.allLtoArtifacts = allLtoArtifacts;
this.linkActionInputs = linkActionInputs;
}

@Override
Expand All @@ -68,6 +72,10 @@ public Sequence<LtoBackendArtifacts> getAllLtoArtifactsForStarlark(StarlarkThrea
return StarlarkList.immutableCopyOf(getAllLtoArtifacts());
}

public ImmutableList<Artifact> getLinkActionInputs() {
return linkActionInputs;
}

public boolean isEmpty() {
return libraryToLink == null;
}
Expand Down Expand Up @@ -121,9 +129,11 @@ private Builder() {
// same list return the .pdb file for Windows.
private final ImmutableList.Builder<LtoBackendArtifacts> allLtoArtifacts =
ImmutableList.builder();
private final ImmutableList.Builder<Artifact> linkActionInputs = ImmutableList.builder();

public CcLinkingOutputs build() {
return new CcLinkingOutputs(libraryToLink, executable, allLtoArtifacts.build());
return new CcLinkingOutputs(
libraryToLink, executable, allLtoArtifacts.build(), linkActionInputs.build());
}

public Builder setLibraryToLink(LibraryToLink libraryToLink) {
Expand All @@ -140,5 +150,10 @@ public Builder addAllLtoArtifacts(Iterable<LtoBackendArtifacts> allLtoArtifacts)
this.allLtoArtifacts.addAll(allLtoArtifacts);
return this;
}

public Builder addLinkActionInputs(NestedSet<Artifact> linkActionInputs) {
this.linkActionInputs.addAll(linkActionInputs.toList());
return this;
}
}
}
61 changes: 32 additions & 29 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,12 @@ public LibraryToLink createLibraryLinkerInput(
"If you pass '%s_library', you must also pass a 'feature_configuration'", library);
}
}
if (nopicObjects != null && staticLibrary == null) {
throw Starlark.errorf("If you pass 'objects' you must also pass a 'static_library'");
}
if (picObjects != null && picStaticLibrary == null) {
throw Starlark.errorf("If you pass 'pic_objects' you must also pass a 'pic_static_library'");
}
if (notNullArtifactForIdentifier == null) {
throw Starlark.errorf("Must pass at least one artifact");
}
Expand Down Expand Up @@ -1798,7 +1804,7 @@ public Tuple createLinkingContextFromCompilationOutputs(
CcToolchainProvider starlarkCcToolchainProvider,
CcCompilationOutputs compilationOutputs,
Sequence<?> userLinkFlags, // <String> expected
Sequence<?> linkingContextsObjects, // <CcLinkingContext> expected
Sequence<?> linkingContexts, // <CcLinkingContext> expected
String name,
String languageString,
boolean alwayslink,
Expand All @@ -1808,11 +1814,9 @@ public Tuple createLinkingContextFromCompilationOutputs(
Object grepIncludes,
Object variablesExtension,
Object stamp,
Object linkedDllNameSuffix,
Object winDefFile,
StarlarkThread thread)
throws InterruptedException, EvalException {
if (checkObjectsBound(stamp, linkedDllNameSuffix, winDefFile)) {
if (checkObjectsBound(stamp)) {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
}
Language language = parseLanguage(languageString);
Expand Down Expand Up @@ -1841,8 +1845,6 @@ public Tuple createLinkingContextFromCompilationOutputs(
} else {
staticLinkTargetType = LinkTargetType.OBJC_ARCHIVE;
}
List<CcLinkingContext> ccLinkingContexts =
Sequence.cast(linkingContextsObjects, CcLinkingContext.class, "linking_contexts");
CcLinkingHelper helper =
new CcLinkingHelper(
actions.getActionConstructionContext().getRuleErrorConsumer(),
Expand All @@ -1866,42 +1868,43 @@ public Tuple createLinkingContextFromCompilationOutputs(
.addNonCodeLinkerInputs(
Sequence.cast(additionalInputs, Artifact.class, "additional_inputs"))
.setShouldCreateStaticLibraries(!disallowStaticLibraries)
.addCcLinkingContexts(ccLinkingContexts)
.setShouldCreateDynamicLibrary(
!disallowDynamicLibraries
&& (!featureConfiguration
.getFeatureConfiguration()
.isEnabled(CppRuleClasses.TARGETS_WINDOWS)
|| winDefFile != null))
&& !featureConfiguration
.getFeatureConfiguration()
.isEnabled(CppRuleClasses.TARGETS_WINDOWS))
.setStaticLinkType(staticLinkTargetType)
.setDynamicLinkType(LinkTargetType.NODEPS_DYNAMIC_LIBRARY)
.emitInterfaceSharedLibraries(true)
.setLinkedDLLNameSuffix(
convertFromNoneable(linkedDllNameSuffix, /* defaultValue= */ ""))
.setDefFile(convertFromNoneable(winDefFile, /* defaultValue= */ null))
.setIsStampingEnabled(isStampingEnabled)
.addLinkopts(Sequence.cast(userLinkFlags, String.class, "user_link_flags"));
if (!asDict(variablesExtension).isEmpty()) {
helper.addVariableExtension(new UserVariablesExtension(asDict(variablesExtension)));
}
try {
CcLinkingOutputs ccLinkingOutputs = CcLinkingOutputs.EMPTY;
ImmutableList<LibraryToLink> libraryToLink = ImmutableList.of();
CcLinkingOutputs ccLinkingOutputs = helper.link(compilationOutputs);
if (!ccLinkingOutputs.isEmpty()) {
LibraryToLink rewrappedForAlwaysLink =
ccLinkingOutputs.getLibraryToLink().toBuilder().setAlwayslink(alwayslink).build();
ccLinkingOutputs =
CcLinkingOutputs.builder()
.setExecutable(ccLinkingOutputs.getExecutable())
.setLibraryToLink(rewrappedForAlwaysLink)
.addAllLtoArtifacts(ccLinkingOutputs.getAllLtoArtifacts())
.build();
libraryToLink = ImmutableList.of(rewrappedForAlwaysLink);
if (!compilationOutputs.isEmpty()) {
ccLinkingOutputs = helper.link(compilationOutputs);
if (!ccLinkingOutputs.isEmpty()) {
libraryToLink =
ImmutableList.of(
ccLinkingOutputs.getLibraryToLink().toBuilder()
.setAlwayslink(alwayslink)
.build());
}
}
CcLinkingContext linkingContext =
helper.buildCcLinkingContextFromLibrariesToLink(
libraryToLink, CcCompilationContext.EMPTY);
return Tuple.of(linkingContext, ccLinkingOutputs);
return Tuple.of(
CcLinkingContext.merge(
ImmutableList.<CcLinkingContext>builder()
.add(linkingContext)
.addAll(
Sequence.cast(linkingContexts, CcLinkingContext.class, "linking_contexts"))
.build()),
ccLinkingOutputs);
} catch (RuleErrorException e) {
throw Starlark.errorf("%s", e.getMessage());
}
Expand Down Expand Up @@ -1999,7 +2002,7 @@ protected Tuple compile(
Object textualHeadersStarlarkObject,
Object additionalExportedHeadersObject,
Sequence<?> includes, // <String> expected
Object starlarkLooseIncludes,
Object starlarkIncludes,
Sequence<?> quoteIncludes, // <String> expected
Sequence<?> systemIncludes, // <String> expected
Sequence<?> frameworkIncludes, // <String> expected
Expand Down Expand Up @@ -2039,15 +2042,15 @@ protected Tuple compile(
hdrsCheckingModeObject,
implementationCcCompilationContextsObject,
coptsFilterObject,
starlarkLooseIncludes)) {
starlarkIncludes)) {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
}

StarlarkActionFactory actions = starlarkActionFactoryApi;
CcToolchainProvider ccToolchainProvider =
convertFromNoneable(starlarkCcToolchainProvider, null);

ImmutableList<String> looseIncludes = asClassImmutableList(starlarkLooseIncludes);
ImmutableList<String> looseIncludes = asClassImmutableList(starlarkIncludes);
CppModuleMap moduleMap = convertFromNoneable(moduleMapNoneable, /* defaultValue= */ null);
ImmutableList<CppModuleMap> additionalModuleMaps =
asClassImmutableList(additionalModuleMapsNoneable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,18 @@ public abstract static class Builder implements LibraryToLink.Builder {
public final LibraryToLink build() {
LibraryToLink result = autoBuild();
Preconditions.checkNotNull(result.getLibraryIdentifier(), result);
Preconditions.checkState(
(result.getObjectFiles() == null
&& result.getLtoCompilationContext() == null
&& result.getSharedNonLtoBackends() == null)
|| result.getStaticLibrary() != null,
result);
Preconditions.checkState(
(result.getPicObjectFiles() == null
&& result.getPicLtoCompilationContext() == null
&& result.getPicSharedNonLtoBackends() == null)
|| result.getPicStaticLibrary() != null,
result);
Preconditions.checkState(
result.getResolvedSymlinkDynamicLibrary() == null || result.getDynamicLibrary() != null,
result);
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcModuleApi.java
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -1236,18 +1236,6 @@ CcToolchainConfigInfoT ccToolchainConfigInfoFromStarlark(
named = true,
documented = false,
defaultValue = "unbound"),
@Param(
name = "linked_dll_name_suffix",
positional = false,
named = true,
documented = false,
defaultValue = "unbound"),
@Param(
name = "win_def_file",
documented = false,
positional = false,
named = true,
defaultValue = "unbound"),
})
Tuple createLinkingContextFromCompilationOutputs(
StarlarkActionFactoryT starlarkActionFactoryApi,
Expand All @@ -1265,8 +1253,6 @@ Tuple createLinkingContextFromCompilationOutputs(
Object grepIncludes,
Object variablesExtension,
Object stamp,
Object linkedDllNameSuffix,
Object winDefFile,
StarlarkThread thread)
throws InterruptedException, EvalException;

Expand Down
29 changes: 29 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -6745,6 +6745,35 @@ public void testObjectFilesInCreateLibrary() throws Exception {
.containsExactly("object.pic.o");
}

@Test
public void testObjectFilesInCreateLibraryWithoutStaticLibrary() throws Exception {
setUpCcLinkingContextTest(true);
scratch.file(
"b/BUILD",
"load('//tools/build_defs/cc:rule.bzl', 'crule')",
"crule(name='import_objects_no_lib',",
" objects = ['object.o'],",
")");

checkError(
"//b:import_objects_no_lib", "If you pass 'objects' you must also pass a 'static_library'");
}

@Test
public void testObjectFilesInCreateLibraryWithoutPicStaticLibrary() throws Exception {
setUpCcLinkingContextTest(true);
scratch.file(
"b/BUILD",
"load('//tools/build_defs/cc:rule.bzl', 'crule')",
"crule(name='import_objects_no_pic_lib',",
" pic_objects = ['object.pic.o'],",
")");

checkError(
"//b:import_objects_no_pic_lib",
"If you pass 'pic_objects' you must also pass a 'pic_static_library'");
}

private void setupDebugPackageProviderTest(String fission) throws Exception {
getAnalysisMock()
.ccSupport()
Expand Down

0 comments on commit eeec121

Please sign in to comment.