Skip to content

Commit

Permalink
Pass target name instead of a label
Browse files Browse the repository at this point in the history 
This makes it clearer what's happening with the name

PiperOrigin-RevId: 609311005
Change-Id: I063f4d45c5096136e21913deb95b3fe32fc8d09d
  • Loading branch information
@comius @copybara-github
comius authored and copybara-github committed Feb 22, 2024
1 parent 6377fc3 commit 7bc54e2
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 31 deletions.
60 changes: 38 additions & 22 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -114,7 +115,7 @@ public CcLinkingOutputs getCcLinkingOutputs() {
private String linkedDLLNameSuffix = "";

private final LinkActionConstruction linkActionConstruction;
private final Label label;
private final String targetName;
private final SymbolGenerator<?> symbolGenerator;
private final ImmutableMap<String, String> executionInfo;

Expand All @@ -124,7 +125,7 @@ public CcLinkingOutputs getCcLinkingOutputs() {
/**
* Creates a CcLinkingHelper that outputs artifacts in a given configuration.
*
* @param label the Label of the rule being built
* @param targetName the name of the rule being built
* @param linkActionConstruction the ActionConstructionContext of the rule being built
* @param semantics CppSemantics for the build
* @param featureConfiguration activated features and action configs for the build
Expand All @@ -133,7 +134,7 @@ public CcLinkingOutputs getCcLinkingOutputs() {
* @param executionInfo the execution info data associated with a rule
*/
public CcLinkingHelper(
Label label,
String targetName,
LinkActionConstruction linkActionConstruction,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
Expand All @@ -145,7 +146,7 @@ public CcLinkingHelper(
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.ccToolchain = Preconditions.checkNotNull(ccToolchain);
this.fdoContext = Preconditions.checkNotNull(fdoContext);
this.label = label;
this.targetName = targetName;
this.linkActionConstruction = linkActionConstruction;
this.symbolGenerator = symbolGenerator;
this.executionInfo = executionInfo;
Expand Down Expand Up @@ -357,7 +358,7 @@ public CcLinkingOutputs link(CcCompilationOutputs ccOutputs)

public CcLinkingContext buildCcLinkingContextFromLibrariesToLink(
List<LibraryToLink> librariesToLink, CcCompilationContext ccCompilationContext)
throws InterruptedException {
throws EvalException, InterruptedException {
ImmutableList.Builder<Linkstamp> linkstampBuilder = ImmutableList.builder();
for (Artifact linkstamp : linkstamps.build().toList()) {
try {
Expand All @@ -374,20 +375,31 @@ public CcLinkingContext buildCcLinkingContextFromLibrariesToLink(
if (!neverlink) {
// We want an empty set if there are no link options. We have to make sure we don't
// create a LinkOptions instance that contains an empty list.
ccLinkingContext =
CcLinkingContext.builder()
.setOwner(label)
.addUserLinkFlags(
linkopts.isEmpty()
? ImmutableList.of()
: ImmutableList.of(
CcLinkingContext.LinkOptions.of(
ImmutableList.copyOf(linkopts), symbolGenerator)))
.addLibraries(librariesToLink)
// additionalLinkerInputsBuilder not expected to be a big list for now.
.addNonCodeInputs(additionalLinkerInputsBuilder.build().toList())
.addLinkstamps(linkstampBuilder.build())
.build();
try {
ccLinkingContext =
CcLinkingContext.builder()
.setOwner(
Label.create(
linkActionConstruction
.getContext()
.getActionOwner()
.getLabel()
.getPackageIdentifier(),
targetName))
.addUserLinkFlags(
linkopts.isEmpty()
? ImmutableList.of()
: ImmutableList.of(
CcLinkingContext.LinkOptions.of(
ImmutableList.copyOf(linkopts), symbolGenerator)))
.addLibraries(librariesToLink)
// additionalLinkerInputsBuilder not expected to be a big list for now.
.addNonCodeInputs(additionalLinkerInputsBuilder.build().toList())
.addLinkstamps(linkstampBuilder.build())
.build();
} catch (LabelSyntaxException e) {
throw new EvalException(e);
}
}
ImmutableList.Builder<CcLinkingContext> mergedCcLinkingContexts = ImmutableList.builder();
mergedCcLinkingContexts.add(ccLinkingContext);
Expand Down Expand Up @@ -422,7 +434,7 @@ private CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs)
CcToolchainProvider.usePicForDynamicLibraries(
ccToolchain.getCppConfiguration(), featureConfiguration);

PathFragment labelName = PathFragment.create(label.getName());
PathFragment labelName = PathFragment.create(targetName);
String libraryIdentifier =
linkActionConstruction
.getContext()
Expand Down Expand Up @@ -888,7 +900,7 @@ private CppLinkActionBuilder newLinkActionBuilder(
* not present. TODO(b/30393154): Assert that the given link action has an action_config.
*/
private Artifact getLinkedArtifact(LinkTargetType linkTargetType) throws RuleErrorException {
String maybePicName = label.getName() + linkedArtifactNameSuffix;
String maybePicName = targetName + linkedArtifactNameSuffix;
if (linkTargetType.picness() == Picness.PIC) {
maybePicName =
CppHelper.getArtifactNameForCategory(
Expand All @@ -915,7 +927,11 @@ private Artifact getLinkedArtifact(LinkTargetType linkTargetType) throws RuleErr
}

return CppHelper.getLinkedArtifact(
label, linkActionConstruction, linkTargetType, linkedArtifactNameSuffix, artifactFragment);
targetName,
linkActionConstruction,
linkTargetType,
linkedArtifactNameSuffix,
artifactFragment);
}

private static List<LinkerInputs.LibraryToLink> convertLibraryToLinkListToLinkerInputList(
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1932,7 +1932,6 @@ public Tuple createLinkingContextFromCompilationOutputs(
CcToolchainProvider.PROVIDER.wrapOrThrowEvalException(starlarkCcToolchainProvider);
FeatureConfigurationForStarlark featureConfiguration =
convertFromNoneable(starlarkFeatureConfiguration, null);
Label label = getCallerLabel(actions, name);
FdoContext fdoContext = ccToolchainProvider.getFdoContext();
LinkTargetType staticLinkTargetType = null;
if (alwayslink && !actions.getRuleContext().getRule().getRuleClass().equals("swift_library")) {
Expand All @@ -1946,7 +1945,7 @@ public Tuple createLinkingContextFromCompilationOutputs(
Sequence.cast(linkingContextsObjects, CcLinkingContext.class, "linking_contexts");
CcLinkingHelper helper =
new CcLinkingHelper(
label,
name,
CppLinkActionBuilder.newActionConstruction(actions.getRuleContext()),
getSemantics(Language.CPP),
featureConfiguration.getFeatureConfiguration(),
Expand Down Expand Up @@ -2678,7 +2677,6 @@ public CcLinkingOutputs link(
FeatureConfigurationForStarlark featureConfiguration =
convertFromNoneable(starlarkFeatureConfiguration, null);
Artifact mainOutput = convertFromNoneable(mainOutputObject, null);
Label label = getCallerLabel(actions, name);
FdoContext fdoContext = ccToolchainProvider.getFdoContext();
LinkTargetType dynamicLinkTargetType = null;
LinkTargetType staticLinkTargetType = null;
Expand Down Expand Up @@ -2719,7 +2717,7 @@ public CcLinkingOutputs link(

CcLinkingHelper helper =
new CcLinkingHelper(
label,
name,
CppLinkActionBuilder.newActionConstruction(
actions.getRuleContext(), buildConfiguration, shareableArtifacts),
getSemantics(language),
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public static PathFragment getThinLtoNativeObjectDirectoryFromLtoOutputRoot(
}

public static Artifact getLinkedArtifact(
Label label,
String targetName,
LinkActionConstruction linkActionConstruction,
LinkTargetType linkType,
String linkedArtifactNameSuffix,
Expand All @@ -193,7 +193,8 @@ public static Artifact getLinkedArtifact(
// linux default to satisfy the requirements of any implicit outputs.
// TODO(b/30132703): Remove the implicit outputs of cc_library.
Artifact linuxDefault =
getLinuxLinkedArtifact(label, linkActionConstruction, linkType, linkedArtifactNameSuffix);
getLinuxLinkedArtifact(
targetName, linkActionConstruction, linkType, linkedArtifactNameSuffix);
if (!result.equals(linuxDefault)) {
linkActionConstruction
.getContext()
Expand All @@ -211,11 +212,11 @@ public static Artifact getLinkedArtifact(
}

private static Artifact getLinuxLinkedArtifact(
Label label,
String targetName,
LinkActionConstruction linkActionConstruction,
LinkTargetType linkType,
String linkedArtifactNameSuffix) {
PathFragment name = PathFragment.create(label.getName());
PathFragment name = PathFragment.create(targetName);
if (linkType != LinkTargetType.EXECUTABLE) {
name =
name.replaceName(
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public static NativeDepsRunfiles createNativeDepsAction(
FdoContext fdoContext = toolchain.getFdoContext();

new CcLinkingHelper(
ruleContext.getLabel(),
ruleContext.getLabel().getName(),
CppLinkActionBuilder.newActionConstruction(
ruleContext, configuration, /* shareableArtifacts= */ true),
cppSemantics,
Expand Down

0 comments on commit 7bc54e2

Please sign in to comment.