Skip to content

Commit

Permalink
Add --incompatible_objc_provider_remove_linking_info
Browse files Browse the repository at this point in the history
bazelbuild#19000

PiperOrigin-RevId: 559141220
Change-Id: I76bc8e6dd764952f4ca2240c1c6c764a8e842494
  • Loading branch information
googlewalt authored and copybara-github committed Aug 22, 2023
1 parent e992fa4 commit 3e170fb
Show file tree
Hide file tree
Showing 12 changed files with 716 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,15 @@ public final class BuildLanguageOptions extends OptionsBase {
+ " specified through features configuration.")
public boolean experimentalGetFixedConfiguredEnvironment;

@Option(
name = "incompatible_objc_provider_remove_linking_info",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help = "If set to true, the ObjcProvider's APIs for linking info will be removed.")
public boolean incompatibleObjcProviderRemoveLinkingInfo;

/**
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
* never accumulate a large number of these and being able to shortcut on object identity makes a
Expand Down Expand Up @@ -772,6 +781,9 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(
EXPERIMENTAL_GET_FIXED_CONFIGURED_ACTION_ENV,
experimentalGetFixedConfiguredEnvironment)
.setBool(
INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO,
incompatibleObjcProviderRemoveLinkingInfo)
.build();
return INTERNER.intern(semantics);
}
Expand Down Expand Up @@ -858,6 +870,8 @@ public StarlarkSemantics toStarlarkSemantics() {
"-incompatible_disable_starlark_host_transitions";
public static final String EXPERIMENTAL_GET_FIXED_CONFIGURED_ACTION_ENV =
"-experimental_get_fixed_configured_action_env";
public static final String INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO =
"-incompatible_objc_provider_remove_linking_info";

// non-booleans
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ public static AppleLinkingOutputs linkMultiArchBinary(
}
outputGroupCollector.put(OutputGroupInfo.VALIDATION, headerTokens.build());

ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder();
ObjcProvider.Builder objcProviderBuilder =
new ObjcProvider.Builder(ruleContext.getAnalysisEnvironment().getStarlarkSemantics());
ImmutableList.Builder<CcInfo> ccInfos = new ImmutableList.Builder<>();
for (DependencySpecificConfiguration dependencySpecificConfiguration :
dependencySpecificConfigurations.values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.rules.objc;

import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -88,6 +90,12 @@ public class AppleStarlarkCommon
@VisibleForTesting
public static final String NOT_SET_ERROR = "Value for key %s must be a set, instead found %s.";

@VisibleForTesting
public static final String DEPRECATED_OBJC_PROVIDER_ERROR = "Key 'objc' no longer needed in %s.";

@VisibleForTesting
public static final String REQUIRED_CC_INFO_ERROR = "Key 'cc_info' is required in %s.";

@Nullable private StructImpl platformType;
@Nullable private StructImpl platform;

Expand Down Expand Up @@ -166,21 +174,34 @@ public ImmutableMap<String, String> getTargetAppleEnvironment(
// This method is registered statically for Starlark, and never called directly.
public ObjcProvider newObjcProvider(Dict<String, Object> kwargs, StarlarkThread thread)
throws EvalException {
ObjcProvider.StarlarkBuilder resultBuilder = new ObjcProvider.StarlarkBuilder();
ObjcProvider.StarlarkBuilder resultBuilder =
new ObjcProvider.StarlarkBuilder(thread.getSemantics());
for (Map.Entry<String, Object> entry : kwargs.entrySet()) {
ObjcProvider.Key<?> key = ObjcProvider.getStarlarkKeyForString(entry.getKey());
if (key != null) {
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)
&& ObjcProvider.DEPRECATED_KEYS.contains(key)) {
throw new EvalException(String.format(DEPRECATED_KEY_ERROR, key.getStarlarkKeyName()));
}
resultBuilder.addElementsFromStarlark(key, entry.getValue());
} else {
switch (entry.getKey()) {
case "cc_library":
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(
String.format(DEPRECATED_KEY_ERROR, key.getStarlarkKeyName()));
}
CcModule.checkPrivateStarlarkificationAllowlist(thread);
resultBuilder.uncheckedAddTransitive(
ObjcProvider.CC_LIBRARY,
ObjcProviderStarlarkConverters.convertToJava(
ObjcProvider.CC_LIBRARY, entry.getValue()));
break;
case "flag":
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(
String.format(DEPRECATED_KEY_ERROR, key.getStarlarkKeyName()));
}
resultBuilder.add(ObjcProvider.FLAG, Flag.USES_CPP);
break;
case "strict_include":
Expand Down Expand Up @@ -212,12 +233,24 @@ public AppleDynamicFrameworkInfo newDynamicFrameworkProvider(
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;
CcInfo ccInfo;
if (depsCcInfo != Starlark.NONE) {
ccInfo = (CcInfo) depsCcInfo;
} else {
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(String.format(REQUIRED_CC_INFO_ERROR, "AppleDynamicFrameworkInfo"));
}
ccInfo = CcInfo.EMPTY;
}
ObjcProvider objcProvider;
if (depsObjcProvider != Starlark.NONE) {
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(
String.format(DEPRECATED_OBJC_PROVIDER_ERROR, "AppleDynamicFrameworkInfo"));
}
objcProvider = (ObjcProvider) depsObjcProvider;
} else {
objcProvider = new ObjcProvider.StarlarkBuilder().build();
objcProvider = new ObjcProvider.StarlarkBuilder(thread.getSemantics()).build();
}
return new AppleDynamicFrameworkInfo(
binary, ccInfo, objcProvider, frameworkDirs, frameworkFiles);
Expand All @@ -229,12 +262,24 @@ public AppleExecutableBinaryInfo newExecutableBinaryProvider(
throws EvalException {
Artifact binary = (executableBinary != Starlark.NONE) ? (Artifact) executableBinary : null;
// TODO(b/252909384): Disallow Starlark.NONE once rules have been migrated to supply CcInfo.
CcInfo ccInfo = (depsCcInfo != Starlark.NONE) ? (CcInfo) depsCcInfo : CcInfo.EMPTY;
CcInfo ccInfo;
if (depsCcInfo != Starlark.NONE) {
ccInfo = (CcInfo) depsCcInfo;
} else {
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(String.format(REQUIRED_CC_INFO_ERROR, "AppleExecutableBinaryInfo"));
}
ccInfo = CcInfo.EMPTY;
}
ObjcProvider objcProvider;
if (depsObjcProvider != Starlark.NONE) {
if (thread.getSemantics().getBool(INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)) {
throw new EvalException(
String.format(DEPRECATED_OBJC_PROVIDER_ERROR, "AppleExecutableBinaryInfo"));
}
objcProvider = (ObjcProvider) depsObjcProvider;
} else {
objcProvider = new ObjcProvider.StarlarkBuilder().build();
objcProvider = new ObjcProvider.StarlarkBuilder(thread.getSemantics()).build();
}
return new AppleExecutableBinaryInfo(binary, ccInfo, objcProvider);
}
Expand Down
15 changes: 1 addition & 14 deletions src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,10 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/compilation_mode",
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_aware_aspect_builder",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_option_converters",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/split_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/starlark_exposed_rule_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
"//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
"//src/main/java/com/google/devtools/build/lib/analysis:template_variable_info",
Expand All @@ -50,16 +42,11 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages:exec_group",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/rules/apple",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation",
"//src/main/java/com/google/devtools/build/lib/rules/proto",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/apple",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/objc",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ ObjcCommon build() {
ObjcCompilationContext.Builder objcCompilationContextBuilder =
ObjcCompilationContext.builder();

ObjcProvider.Builder objcProvider = new ObjcProvider.Builder();
ObjcProvider.Builder objcProvider =
new ObjcProvider.Builder(context.getAnalysisEnvironment().getStarlarkSemantics());

objcProvider
.addTransitiveAndPropagate(objcProviders);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkList;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;

/**
Expand Down Expand Up @@ -240,6 +241,22 @@ public enum Flag {
*/
static final ImmutableSet<Key<?>> KEYS_FOR_DIRECT = ImmutableSet.<Key<?>>of(MODULE_MAP, SOURCE);

/** Keys that are deprecated and will be removed at conclusion of linking info migration. */
static final ImmutableSet<Key<?>> DEPRECATED_KEYS =
ImmutableSet.<Key<?>>of(
CC_LIBRARY,
DYNAMIC_FRAMEWORK_FILE,
FORCE_LOAD_LIBRARY,
FLAG,
IMPORTED_LIBRARY,
LIBRARY,
LINK_INPUTS,
LINKOPT,
SDK_DYLIB,
SDK_FRAMEWORK,
STATIC_FRAMEWORK_FILE,
WEAK_SDK_FRAMEWORK);

public ImmutableList<PathFragment> getStrictDependencyIncludes() {
return strictDependencyIncludes;
}
Expand Down Expand Up @@ -546,6 +563,7 @@ NestedSet<String> staticFrameworkPaths() {
*/
public static class Builder {

protected final StarlarkSemantics starlarkSemantics;
private final Map<Key<?>, NestedSetBuilder<?>> items = new HashMap<>();
private final ImmutableList.Builder<PathFragment> strictDependencyIncludes =
ImmutableList.builder();
Expand All @@ -554,7 +572,9 @@ public static class Builder {
private final ImmutableListMultimap.Builder<Key<?>, ?> directItems =
new ImmutableListMultimap.Builder<>();

public Builder() {}
public Builder(StarlarkSemantics semantics) {
this.starlarkSemantics = semantics;
}

private static void maybeAddEmptyBuilder(Map<Key<?>, NestedSetBuilder<?>> set, Key<?> key) {
set.computeIfAbsent(key, k -> new NestedSetBuilder<>(k.order));
Expand Down Expand Up @@ -680,8 +700,8 @@ ObjcProvider build() {

/** A builder for this context, specialized for Starlark use. */
public static final class StarlarkBuilder extends Builder {
public StarlarkBuilder() {
super();
public StarlarkBuilder(StarlarkSemantics semantics) {
super(semantics);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcInfoApi;
Expand Down Expand Up @@ -91,6 +92,7 @@ public interface AppleDynamicFrameworkInfoApi<FileApiT extends FileApi> extends
structField = true,
doc =
"A provider which contains information about the transitive dependencies linked into "
+ "the dynamic framework.")
+ "the dynamic framework.",
disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)
ObjcProviderApi<FileApiT> getDepsObjcProvider();
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.starlarkbuildapi.objc;

import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcInfoApi;
Expand Down Expand Up @@ -52,6 +53,7 @@ public interface AppleExecutableBinaryApi extends StructApi {
structField = true,
doc =
"A provider which contains information about the transitive dependencies linked into "
+ "the binary.")
+ "the binary.",
disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO)
ObjcProviderApi<?> getDepsObjcProvider();
}
Loading

0 comments on commit 3e170fb

Please sign in to comment.