Skip to content

Commit

Permalink
Add and propagate CcInfo in binary providers that carry linking info
Browse files Browse the repository at this point in the history
The info in the providers is used to support avoid deps, and we need to add
CcInfo to them in preparation for migrating linking info to CcInfo.

In the starlark constructor of those providers, add an extra cc_info field, and
allow it to be empty while we migrate all the starlark usage.

PiperOrigin-RevId: 482523687
Change-Id: Ibae9c29aadb6819d22d55f207166037352ede0a0
  • Loading branch information
googlewalt authored and Copybara-Service committed Oct 20, 2022
1 parent 0232dd7 commit 9feeb1d
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 21 deletions.
Expand Up @@ -108,11 +108,14 @@ public static AppleLinkingOutputs linkMultiArchBinary(

ObjcProvider.Builder objcProviderBuilder =
new ObjcProvider.Builder(ruleContext.getAnalysisEnvironment().getStarlarkSemantics());
ImmutableList.Builder<CcInfo> ccInfos = new ImmutableList.Builder<>();
for (DependencySpecificConfiguration dependencySpecificConfiguration :
dependencySpecificConfigurations.values()) {
objcProviderBuilder.addTransitiveAndPropagate(
dependencySpecificConfiguration.objcProviderWithAvoidDepsSymbols());
ccInfos.add(dependencySpecificConfiguration.ccInfoWithAvoidDepsSymbols());
}
CcInfo ccInfo = CcInfo.merge(ccInfos.build());

AppleDebugOutputsInfo.Builder legacyDebugOutputsBuilder =
AppleDebugOutputsInfo.Builder.create();
Expand Down Expand Up @@ -173,6 +176,7 @@ public static AppleLinkingOutputs linkMultiArchBinary(

return builder
.setDepsObjcProvider(objcProviderBuilder.build())
.setDepsCcInfo(ccInfo)
.setLegacyDebugOutputsProvider(legacyDebugOutputsBuilder.build())
.build();
}
Expand Down
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.starlarkbuildapi.objc.AppleDynamicFrameworkInfoApi;
import javax.annotation.Nullable;

Expand All @@ -32,6 +33,8 @@
* <li>'framework_files': The full set of artifacts that should be included as inputs to link
* against the dynamic framework
* <li>'binary': The dylib binary artifact of the dynamic framework
* <li>'cc_info': A {@link CcInfo} which contains information about the transitive dependencies
* linked into the binary.
* <li>'objc': An {@link ObjcProvider} which contains information about the transitive
* dependencies linked into the binary, (intended so that bundle loaders depending on this
* executable may avoid relinking symbols included in the loadable binary
Expand All @@ -49,26 +52,20 @@ public final class AppleDynamicFrameworkInfo extends NativeInfo
new BuiltinProvider<AppleDynamicFrameworkInfo>(
STARLARK_NAME, AppleDynamicFrameworkInfo.class) {};

/** Field name for the dylib binary artifact of the dynamic framework. */
public static final String DYLIB_BINARY_FIELD_NAME = "binary";
/** Field name for the framework path names of the dynamic framework. */
public static final String FRAMEWORK_DIRS_FIELD_NAME = "framework_dirs";
/** Field name for the framework link-input artifacts of the dynamic framework. */
public static final String FRAMEWORK_FILES_FIELD_NAME = "framework_files";
/** Field name for the {@link ObjcProvider} containing dependency information. */
public static final String OBJC_PROVIDER_FIELD_NAME = "objc";

private final NestedSet<String> dynamicFrameworkDirs;
private final NestedSet<Artifact> dynamicFrameworkFiles;
@Nullable private final Artifact dylibBinary;
private final CcInfo depsCcInfo;
private final ObjcProvider depsObjcProvider;

public AppleDynamicFrameworkInfo(
@Nullable Artifact dylibBinary,
CcInfo depsCcInfo,
ObjcProvider depsObjcProvider,
NestedSet<String> dynamicFrameworkDirs,
NestedSet<Artifact> dynamicFrameworkFiles) {
this.dylibBinary = dylibBinary;
this.depsCcInfo = depsCcInfo;
this.depsObjcProvider = depsObjcProvider;
this.dynamicFrameworkDirs = dynamicFrameworkDirs;
this.dynamicFrameworkFiles = dynamicFrameworkFiles;
Expand All @@ -94,6 +91,11 @@ public Artifact getAppleDylibBinary() {
return dylibBinary;
}

@Override
public CcInfo getDepsCcInfo() {
return depsCcInfo;
}

@Override
public ObjcProvider getDepsObjcProvider() {
return depsObjcProvider;
Expand Down
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.starlarkbuildapi.objc.AppleExecutableBinaryApi;

/**
Expand All @@ -26,6 +27,8 @@
*
* <ul>
* <li>'binary': The executable binary artifact output by apple_binary
* <li>'cc_info': A {@link CcInfo} which contains information about the transitive dependencies
* linked into the binary.
* <li>'objc': An {@link ObjcProvider} which contains information about the transitive
* dependencies linked into the binary, (intended so that bundle loaders depending on this
* executable may avoid relinking symbols included in the loadable binary
Expand All @@ -44,15 +47,17 @@ public final class AppleExecutableBinaryInfo extends NativeInfo
STARLARK_NAME, AppleExecutableBinaryInfo.class) {};

private final Artifact appleExecutableBinary;
private final CcInfo depsCcInfo;
private final ObjcProvider depsObjcProvider;

/**
* Creates a new AppleExecutableBinaryInfo provider that propagates the given apple_binary
* configured as an executable.
*/
public AppleExecutableBinaryInfo(Artifact appleExecutableBinary,
ObjcProvider depsObjcProvider) {
public AppleExecutableBinaryInfo(
Artifact appleExecutableBinary, CcInfo depsCcInfo, ObjcProvider depsObjcProvider) {
this.appleExecutableBinary = appleExecutableBinary;
this.depsCcInfo = depsCcInfo;
this.depsObjcProvider = depsObjcProvider;
}

Expand All @@ -69,6 +74,15 @@ public Artifact getAppleExecutableBinary() {
return appleExecutableBinary;
}

/**
* Returns the {@link CcInfo} which contains information about the transitive dependencies linked
* into the dylib.
*/
@Override
public CcInfo getDepsCcInfo() {
return depsCcInfo;
}

/**
* Returns the {@link ObjcProvider} which contains information about the transitive dependencies
* linked into the dylib.
Expand Down
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.StarlarkInfo;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -101,17 +102,20 @@ public abstract static class Builder {
}
}

private final CcInfo depsCcInfo;
private final ObjcProvider depsObjcProvider;
private final ImmutableList<LinkingOutput> outputs;
private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;

private final AppleDebugOutputsInfo legacyDebugOutputsProvider;

AppleLinkingOutputs(
CcInfo depsCcInfo,
ObjcProvider depsObjcProvider,
ImmutableList<LinkingOutput> outputs,
ImmutableMap<String, NestedSet<Artifact>> outputGroups,
AppleDebugOutputsInfo legacyDebugOutputsProvider) {
this.depsCcInfo = depsCcInfo;
this.depsObjcProvider = depsObjcProvider;
this.outputs = outputs;
this.outputGroups = outputGroups;
Expand All @@ -127,6 +131,14 @@ public ObjcProvider getDepsObjcProvider() {
return depsObjcProvider;
}

/**
* Returns an {@link CcInfo} containing information about the transitive dependencies linked into
* the binary.
*/
public CcInfo getDepsCcInfo() {
return depsCcInfo;
}

/** Returns the list of single-architecture/platform outputs. */
public ImmutableList<LinkingOutput> getOutputs() {
return outputs;
Expand All @@ -153,6 +165,7 @@ public static class Builder {
private final ImmutableList.Builder<LinkingOutput> outputs;
private final ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups;
private ObjcProvider depsObjcProvider;
private CcInfo depsCcInfo;

private AppleDebugOutputsInfo legacyDebugOutputsProvider;

Expand Down Expand Up @@ -182,6 +195,16 @@ public Builder setLegacyDebugOutputsProvider(AppleDebugOutputsInfo debugOutputsP
return this;
}

/**
* Sets the {@link CcInfo} that contains information about transitive dependencies linked into
* the binary.
*/
@CanIgnoreReturnValue
public Builder setDepsCcInfo(CcInfo depsCcInfo) {
this.depsCcInfo = depsCcInfo;
return this;
}

/**
* Sets the {@link ObjcProvider} that contains information about transitive dependencies linked
* into the binary.
Expand All @@ -194,6 +217,7 @@ public Builder setDepsObjcProvider(ObjcProvider depsObjcProvider) {

public AppleLinkingOutputs build() {
return new AppleLinkingOutputs(
depsCcInfo,
depsObjcProvider,
outputs.build(),
outputGroups.buildOrThrow(),
Expand Down
Expand Up @@ -40,6 +40,7 @@
import com.google.devtools.build.lib.rules.apple.DottedVersion;
import com.google.devtools.build.lib.rules.apple.XcodeConfigInfo;
import com.google.devtools.build.lib.rules.apple.XcodeVersionProperties;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcModule;
import com.google.devtools.build.lib.rules.cpp.CppSemantics;
import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag;
Expand All @@ -64,6 +65,7 @@ public class AppleStarlarkCommon
Artifact,
ConstraintValueInfo,
StarlarkRuleContext,
CcInfo,
ObjcProvider,
XcodeConfigInfo,
ApplePlatform> {
Expand Down Expand Up @@ -211,6 +213,7 @@ public ObjcProvider newObjcProvider(Dict<String, Object> kwargs, StarlarkThread
@Override
public AppleDynamicFrameworkInfo newDynamicFrameworkProvider(
Object dylibBinary,
Object depsCcInfo,
ObjcProvider depsObjcProvider,
Object dynamicFrameworkDirs,
Object dynamicFrameworkFiles)
Expand All @@ -220,15 +223,21 @@ public AppleDynamicFrameworkInfo newDynamicFrameworkProvider(
NestedSet<Artifact> frameworkFiles =
Depset.noneableCast(dynamicFrameworkFiles, Artifact.class, "framework_files");
Artifact binary = (dylibBinary != Starlark.NONE) ? (Artifact) dylibBinary : null;
// TODO(b/252909384): Disallow Starlark.NONE once rules have been migrated to supply CcInfo.
CcInfo ccInfo = (depsCcInfo != Starlark.NONE) ? (CcInfo) depsCcInfo : CcInfo.EMPTY;

return new AppleDynamicFrameworkInfo(binary, depsObjcProvider, frameworkDirs, frameworkFiles);
return new AppleDynamicFrameworkInfo(
binary, ccInfo, depsObjcProvider, frameworkDirs, frameworkFiles);
}

@Override
public AppleExecutableBinaryInfo newExecutableBinaryProvider(
Object executableBinary, ObjcProvider depsObjcProvider) throws EvalException {
Object executableBinary, Object depsCcInfo, ObjcProvider depsObjcProvider)
throws EvalException {
Artifact binary = (executableBinary != Starlark.NONE) ? (Artifact) executableBinary : null;
return new AppleExecutableBinaryInfo(binary, depsObjcProvider);
// TODO(b/252909384): Disallow Starlark.NONE once rules have been migrated to supply CcInfo.
CcInfo ccInfo = (depsCcInfo != Starlark.NONE) ? (CcInfo) depsCcInfo : CcInfo.EMPTY;
return new AppleExecutableBinaryInfo(binary, ccInfo, depsObjcProvider);
}

@Override
Expand Down Expand Up @@ -345,6 +354,7 @@ private StructImpl createStarlarkLinkingOutputs(

ImmutableMap.Builder<String, Object> fields = ImmutableMap.builder();
fields.put("objc", linkingOutputs.getDepsObjcProvider());
fields.put("cc_info", linkingOutputs.getDepsCcInfo());
fields.put("output_groups", Dict.copyOf(thread.mutability(), outputGroups));
fields.put("outputs", StarlarkList.copyOf(thread.mutability(), outputStructs.build()));

Expand Down
Expand Up @@ -67,9 +67,14 @@ static DependencySpecificConfiguration create(
BuildConfigurationValue config,
CcToolchainProvider toolchain,
ObjcProvider objcLinkProvider,
ObjcProvider objcProviderWithAvoidDepsSymbols) {
ObjcProvider objcProviderWithAvoidDepsSymbols,
CcInfo ccInfoWithAvoidDepsSymbols) {
return new AutoValue_MultiArchBinarySupport_DependencySpecificConfiguration(
config, toolchain, objcLinkProvider, objcProviderWithAvoidDepsSymbols);
config,
toolchain,
objcLinkProvider,
objcProviderWithAvoidDepsSymbols,
ccInfoWithAvoidDepsSymbols);
}

/** Returns the child configuration for this tuple. */
Expand All @@ -89,6 +94,12 @@ static DependencySpecificConfiguration create(
* symbols subtracted, thus signaling that this target is still responsible for those symbols.
*/
abstract ObjcProvider objcProviderWithAvoidDepsSymbols();

/**
* Returns the {@link CcInfo} to propagate up to dependers; this will not have avoid deps
* symbols subtracted, thus signaling that this target is still responsible for those symbols.
*/
abstract CcInfo ccInfoWithAvoidDepsSymbols();
}

/** @param ruleContext the current rule context */
Expand Down Expand Up @@ -211,6 +222,8 @@ public Artifact registerConfigurationSpecificLinkActions(
objcProviderWithAvoidDepsSymbols.subtractSubtrees(
avoidDepsObjcProviders, avoidDepsCcLinkingContexts);

CcInfo ccInfoWithAvoidDepsSymbols = common.createCcInfo();

CcToolchainProvider toolchainProvider =
ctad.getConfiguredTarget().get(CcToolchainProvider.PROVIDER);

Expand All @@ -220,7 +233,8 @@ public Artifact registerConfigurationSpecificLinkActions(
childToolchainConfig,
toolchainProvider,
objcProvider,
objcProviderWithAvoidDepsSymbols));
objcProviderWithAvoidDepsSymbols,
ccInfoWithAvoidDepsSymbols));
}

return childInfoBuilder.buildOrThrow();
Expand Down
Expand Up @@ -31,9 +31,9 @@ public class AppleBootstrap implements Bootstrap {
PackageIdentifier.createUnchecked("rules_apple", ""),
PackageIdentifier.createUnchecked("", "tools/build_defs/apple"));

private final AppleCommonApi<?, ?, ?, ?, ?, ?> appleCommon;
private final AppleCommonApi<?, ?, ?, ?, ?, ?, ?> appleCommon;

public AppleBootstrap(AppleCommonApi<?, ?, ?, ?, ?, ?> appleCommon) {
public AppleBootstrap(AppleCommonApi<?, ?, ?, ?, ?, ?, ?> appleCommon) {
this.appleCommon = appleCommon;
}

Expand Down
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.starlarkbuildapi.apple.XcodeConfigInfoApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcInfoApi;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ConstraintValueInfoApi;
import net.starlark.java.annot.Param;
import net.starlark.java.annot.ParamType;
Expand All @@ -48,6 +49,7 @@ public interface AppleCommonApi<
FileApiT extends FileApi,
ConstraintValueT extends ConstraintValueInfoApi,
StarlarkRuleContextT extends StarlarkRuleContextApi<ConstraintValueT>,
CcInfoApiT extends CcInfoApi<?>,
ObjcProviderApiT extends ObjcProviderApi<?>,
XcodeConfigInfoApiT extends XcodeConfigInfoApi<?, ?>,
ApplePlatformApiT extends ApplePlatformApi>
Expand Down Expand Up @@ -254,6 +256,14 @@ ObjcProviderApi<?> newObjcProvider(Dict<String, Object> kwargs, StarlarkThread t
positional = false,
defaultValue = "None",
doc = "The dylib binary artifact of the dynamic framework."),
@Param(
name = "cc_info",
named = true,
positional = false,
defaultValue = "None",
doc =
"A CcInfo which contains information about the transitive dependencies "
+ "linked into the binary."),
@Param(
name = "objc",
named = true,
Expand Down Expand Up @@ -288,6 +298,7 @@ ObjcProviderApi<?> newObjcProvider(Dict<String, Object> kwargs, StarlarkThread t
})
AppleDynamicFrameworkInfoApi<?> newDynamicFrameworkProvider(
Object dylibBinary,
Object depsCcInfo,
ObjcProviderApiT depsObjcProvider,
Object dynamicFrameworkDirs,
Object dynamicFrameworkFiles)
Expand All @@ -307,6 +318,14 @@ AppleDynamicFrameworkInfoApi<?> newDynamicFrameworkProvider(
positional = false,
defaultValue = "None",
doc = "The binary artifact of the executable."),
@Param(
name = "cc_info",
named = true,
positional = false,
defaultValue = "None",
doc =
"A CcInfo which contains information about the transitive dependencies "
+ "linked into the binary."),
@Param(
name = "objc",
named = true,
Expand All @@ -316,7 +335,8 @@ AppleDynamicFrameworkInfoApi<?> newDynamicFrameworkProvider(
+ "dependencies linked into the binary.")
})
AppleExecutableBinaryApi newExecutableBinaryProvider(
Object executableBinary, ObjcProviderApiT depsObjcProvider) throws EvalException;
Object executableBinary, Object depsCcInfo, ObjcProviderApiT depsObjcProvider)
throws EvalException;

@StarlarkMethod(
name = "link_multi_arch_binary",
Expand Down

3 comments on commit 9feeb1d

@brentleyjones
Copy link
Contributor

Choose a reason for hiding this comment

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

@googlewalt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brentleyjones Did you ask somewhere about cherry-picking to 6.0.0? I got that in my email but cannot find the comment in github.

That sounds ok to me, but we'd also need 4eb0cc8. Can you handle cherry-picking them to the 6.0 branch?

@brentleyjones
Copy link
Contributor

@brentleyjones brentleyjones commented on 9feeb1d Nov 17, 2022

Choose a reason for hiding this comment

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

I did, yes, but then I realized both those commits already made the cut, so no cherry-picking required. Sorry for the ping!

Please sign in to comment.