Skip to content

Commit

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

This has been the default behavior.  Remove flag and support code for when flag
is false.

PiperOrigin-RevId: 601570796
Change-Id: Ied4bbbcf0087e551f99dcc914276d9b027290b6e
  • Loading branch information
googlewalt authored and copybara-github committed Jan 25, 2024
1 parent d1df1be commit 91ebbc2
Show file tree
Hide file tree
Showing 16 changed files with 17 additions and 1,232 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -650,15 +650,6 @@ public final class BuildLanguageOptions extends OptionsBase {
+ " in to 'env'. If disabled, the value of 'env' is completely ignored in this case.")
public boolean incompatibleMergeFixedAndDefaultShellEnv;

@Option(
name = "incompatible_objc_provider_remove_linking_info",
defaultValue = "true",
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;

@Option(
name = "incompatible_disable_objc_library_transition",
defaultValue = "true",
Expand Down Expand Up @@ -822,9 +813,6 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(
INCOMPATIBLE_MERGE_FIXED_AND_DEFAULT_SHELL_ENV,
incompatibleMergeFixedAndDefaultShellEnv)
.setBool(
INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO,
incompatibleObjcProviderRemoveLinkingInfo)
.setBool(
INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION,
incompatibleDisableObjcLibraryTransition)
Expand Down Expand Up @@ -923,8 +911,6 @@ public StarlarkSemantics toStarlarkSemantics() {
"-incompatible_disable_starlark_host_transitions";
public static final String INCOMPATIBLE_MERGE_FIXED_AND_DEFAULT_SHELL_ENV =
"+experimental_merge_fixed_and_default_shell_env";
public static final String INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO =
"+incompatible_objc_provider_remove_linking_info";
public static final String INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION =
"+incompatible_disable_objc_library_transition";
public static final String INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ public static AppleLinkingOutputs linkMultiArchBinary(
}
outputGroupCollector.put(OutputGroupInfo.VALIDATION, headerTokens.build());

ObjcProvider.Builder objcProviderBuilder =
new ObjcProvider.Builder(ruleContext.getAnalysisEnvironment().getStarlarkSemantics());
ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder();
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 @@ -35,9 +35,6 @@
* <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
* </ul>
*/
@Immutable
Expand All @@ -56,17 +53,14 @@ public final class AppleDynamicFrameworkInfo extends NativeInfo
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 Down Expand Up @@ -95,9 +89,4 @@ public Artifact getAppleDylibBinary() {
public CcInfo getDepsCcInfo() {
return depsCcInfo;
}

@Override
public ObjcProvider getDepsObjcProvider() {
return depsObjcProvider;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@
* <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
* </ul>
*/
@Immutable
Expand All @@ -48,17 +45,14 @@ public final class AppleExecutableBinaryInfo extends NativeInfo

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, CcInfo depsCcInfo, ObjcProvider depsObjcProvider) {
public AppleExecutableBinaryInfo(Artifact appleExecutableBinary, CcInfo depsCcInfo) {
this.appleExecutableBinary = appleExecutableBinary;
this.depsCcInfo = depsCcInfo;
this.depsObjcProvider = depsObjcProvider;
}

@Override
Expand All @@ -82,13 +76,4 @@ public Artifact getAppleExecutableBinary() {
public CcInfo getDepsCcInfo() {
return depsCcInfo;
}

/**
* Returns the {@link ObjcProvider} which contains information about the transitive dependencies
* linked into the dylib.
*/
@Override
public ObjcProvider getDepsObjcProvider() {
return depsObjcProvider;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

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;
Expand All @@ -39,10 +38,8 @@
import com.google.devtools.build.lib.rules.apple.DottedVersion;
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.cpp.UserVariablesExtension;
import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag;
import com.google.devtools.build.lib.starlarkbuildapi.objc.AppleCommonApi;
import java.util.Map;
import javax.annotation.Nullable;
Expand All @@ -68,10 +65,6 @@ public class AppleStarlarkCommon
XcodeConfigInfo,
ApplePlatform> {

@VisibleForTesting
public static final String DEPRECATED_KEY_ERROR =
"Key '%s' no longer supported in ObjcProvider (use CcInfo instead).";

@VisibleForTesting
public static final String BAD_KEY_ERROR =
"Argument %s not a recognized key, 'strict_include', or 'providers'.";
Expand All @@ -88,12 +81,6 @@ 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 @@ -172,36 +159,13 @@ 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(thread.getSemantics());
ObjcProvider.StarlarkBuilder resultBuilder = new ObjcProvider.StarlarkBuilder();
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":
resultBuilder.addStrictIncludeFromStarlark(entry.getValue());
break;
Expand All @@ -219,8 +183,7 @@ public ObjcProvider newObjcProvider(Dict<String, Object> kwargs, StarlarkThread
@Override
public AppleDynamicFrameworkInfo newDynamicFrameworkProvider(
Object dylibBinary,
Object depsCcInfo,
Object depsObjcProvider,
CcInfo depsCcInfo,
Object dynamicFrameworkDirs,
Object dynamicFrameworkFiles,
StarlarkThread thread)
Expand All @@ -230,56 +193,14 @@ 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;
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(thread.getSemantics()).build();
}
return new AppleDynamicFrameworkInfo(
binary, ccInfo, objcProvider, frameworkDirs, frameworkFiles);
return new AppleDynamicFrameworkInfo(binary, depsCcInfo, frameworkDirs, frameworkFiles);
}

@Override
public AppleExecutableBinaryInfo newExecutableBinaryProvider(
Object executableBinary, Object depsCcInfo, Object depsObjcProvider, StarlarkThread thread)
throws EvalException {
Object executableBinary, CcInfo depsCcInfo, StarlarkThread thread) 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;
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(thread.getSemantics()).build();
}
return new AppleExecutableBinaryInfo(binary, ccInfo, objcProvider);
return new AppleExecutableBinaryInfo(binary, depsCcInfo);
}

private Dict<?, ?> asDict(Object o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ 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/semantics",
"//src/main/java/com/google/devtools/build/lib/rules:alias",
"//src/main/java/com/google/devtools/build/lib/rules/apple",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ ObjcCommon build() {
ObjcCompilationContext.Builder objcCompilationContextBuilder =
ObjcCompilationContext.builder();

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

objcProvider
.addTransitiveAndPropagate(objcProviders);
Expand Down
Loading

0 comments on commit 91ebbc2

Please sign in to comment.