Skip to content

Commit

Permalink
Remove dead code around native repo rules registering toolchains on i…
Browse files Browse the repository at this point in the history
…nstantiation

Now that the native repo rules `android_{s,n}dk_repository` have been removed, these methods are no longer used and can be removed.

Includes some other random cleanups.

PiperOrigin-RevId: 676519010
Change-Id: I2412471a464d84008f7873fd87d6a82ee7997aa9
  • Loading branch information
Wyverald authored and copybara-github committed Sep 19, 2024
1 parent d661d7d commit 5d4400c
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ private Object createRuleLegacy(StarlarkThread thread, Dict<String, Object> kwar
return WorkspaceFactoryHelper.createAndAddRepositoryRule(
pkgBuilder,
ruleClass,
/* bindRuleClass= */ null,
WorkspaceFactoryHelper.getFinalKwargs(kwargs),
thread.getCallStack());
} catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,6 @@ public class RuleClass implements RuleClassData {
*/
private static final int MAX_ATTRIBUTE_NAME_LENGTH = 128;

@SerializationConstant
static final Function<? super Rule, Map<String, Label>> NO_EXTERNAL_BINDINGS =
Functions.constant(ImmutableMap.of());

@SerializationConstant
static final Function<? super Rule, List<String>> NO_TOOLCHAINS_TO_REGISTER =
Functions.constant(ImmutableList.of());

@SerializationConstant
static final Function<? super Rule, Set<String>> NO_OPTION_REFERENCE =
Functions.constant(ImmutableSet.of());
Expand Down Expand Up @@ -188,7 +180,7 @@ public interface ConfiguredTargetFactory<
*
* @throws RuleErrorException if configured target creation could not be completed due to rule
* errors
* @throws TActionConflictException if there were conflicts during action registration
* @throws ActionConflictExceptionT if there were conflicts during action registration
*/
@Nullable
ConfiguredTargetT create(ContextT ruleContext)
Expand Down Expand Up @@ -687,10 +679,6 @@ public String toString() {
private BuildSetting buildSetting = null;

private ImmutableList<? extends StarlarkSubruleApi> subrules = ImmutableList.of();
private Function<? super Rule, Map<String, Label>> externalBindingsFunction =
NO_EXTERNAL_BINDINGS;
private Function<? super Rule, ? extends List<String>> toolchainsToRegisterFunction =
NO_TOOLCHAINS_TO_REGISTER;
private Function<? super Rule, ? extends Set<String>> optionReferenceFunction =
NO_OPTION_REFERENCE;

Expand Down Expand Up @@ -860,13 +848,9 @@ private RuleClass build(String name, String key) {
type,
configuredTargetFactory,
configuredTargetFunction);
if (!workspaceOnly) {
if (starlark) {
assertStarlarkRuleClassHasImplementationFunction();
assertStarlarkRuleClassHasEnvironmentLabel();
}
Preconditions.checkState(externalBindingsFunction == NO_EXTERNAL_BINDINGS);
Preconditions.checkState(toolchainsToRegisterFunction == NO_TOOLCHAINS_TO_REGISTER);
if (!workspaceOnly && starlark) {
assertStarlarkRuleClassHasImplementationFunction();
assertStarlarkRuleClassHasEnvironmentLabel();
}
if (type == RuleClassType.PLACEHOLDER) {
Preconditions.checkNotNull(ruleDefinitionEnvironmentDigest, this.name);
Expand Down Expand Up @@ -935,8 +919,6 @@ private RuleClass build(String name, String key) {
configuredTargetFactory,
advertisedProviders.build(),
configuredTargetFunction,
externalBindingsFunction,
toolchainsToRegisterFunction,
optionReferenceFunction,
ruleDefinitionEnvironmentLabel,
ruleDefinitionEnvironmentDigest,
Expand Down Expand Up @@ -1363,19 +1345,6 @@ public ImmutableSet<? extends StarlarkSubruleApi> getParentSubrules() {
return builder.build();
}

@CanIgnoreReturnValue
public Builder setExternalBindingsFunction(Function<? super Rule, Map<String, Label>> func) {
this.externalBindingsFunction = func;
return this;
}

@CanIgnoreReturnValue
public Builder setToolchainsToRegisterFunction(
Function<? super Rule, ? extends List<String>> func) {
this.toolchainsToRegisterFunction = func;
return this;
}

/**
* Sets the rule definition environment label and transitive digest. Meant for Starlark usage.
*/
Expand Down Expand Up @@ -1692,8 +1661,8 @@ public Attribute.Builder<?> copy(String name) {
private final boolean isStarlark;
private final boolean extendable;
// The following 2 fields may be non-null only if the rule is Starlark-defined.
@Nullable private Label starlarkExtensionLabel;
@Nullable private String starlarkDocumentation;
@Nullable private final Label starlarkExtensionLabel;
@Nullable private final String starlarkDocumentation;
@Nullable private final Label extendableAllowlist;
private final boolean starlarkTestable;
private final boolean documented;
Expand Down Expand Up @@ -1754,12 +1723,6 @@ public Attribute.Builder<?> copy(String name) {
*/
private final ImmutableSet<? extends StarlarkSubruleApi> subrules;

/** Returns the extra bindings a workspace function adds to the WORKSPACE file. */
private final Function<? super Rule, Map<String, Label>> externalBindingsFunction;

/** Returns the toolchains a workspace function wants to have registered in the WORKSPACE file. */
private final Function<? super Rule, ? extends List<String>> toolchainsToRegisterFunction;

/** Returns the options referenced by this rule's attributes. */
private final Function<? super Rule, ? extends Set<String>> optionReferenceFunction;

Expand Down Expand Up @@ -1843,8 +1806,6 @@ public Attribute.Builder<?> copy(String name) {
ConfiguredTargetFactory<?, ?, ?> configuredTargetFactory,
AdvertisedProviderSet advertisedProviders,
@Nullable StarlarkCallable configuredTargetFunction,
Function<? super Rule, Map<String, Label>> externalBindingsFunction,
Function<? super Rule, ? extends List<String>> toolchainsToRegisterFunction,
Function<? super Rule, ? extends Set<String>> optionReferenceFunction,
@Nullable Label ruleDefinitionEnvironmentLabel,
@Nullable byte[] ruleDefinitionEnvironmentDigest,
Expand Down Expand Up @@ -1881,8 +1842,6 @@ public Attribute.Builder<?> copy(String name) {
this.configuredTargetFactory = configuredTargetFactory;
this.advertisedProviders = advertisedProviders;
this.configuredTargetFunction = configuredTargetFunction;
this.externalBindingsFunction = externalBindingsFunction;
this.toolchainsToRegisterFunction = toolchainsToRegisterFunction;
this.optionReferenceFunction = optionReferenceFunction;
this.ruleDefinitionEnvironmentLabel = ruleDefinitionEnvironmentLabel;
this.ruleDefinitionEnvironmentDigest = ruleDefinitionEnvironmentDigest;
Expand Down Expand Up @@ -2588,22 +2547,6 @@ public BuildSetting getBuildSetting() {
return buildSetting;
}

/**
* Returns a function that computes the external bindings a repository function contributes to the
* WORKSPACE file.
*/
Function<? super Rule, Map<String, Label>> getExternalBindingsFunction() {
return externalBindingsFunction;
}

/**
* Returns a function that computes the toolchains that should be registered for a repository
* function.
*/
Function<? super Rule, ? extends List<String>> getToolchainsToRegisterFunction() {
return toolchainsToRegisterFunction;
}

/** Returns a function that computes the options referenced by a rule. */
public Function<? super Rule, ? extends Set<String>> getOptionReferenceFunction() {
return optionReferenceFunction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,10 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
WorkspaceFactoryHelper.addMainRepoEntry(builder, externalRepoName);
WorkspaceFactoryHelper.addRepoMappings(builder, kwargs, externalRepoName);
RuleClass ruleClass = ruleClassMap.get(ruleClassName);
RuleClass bindRuleClass = ruleClassMap.get("bind");
Rule rule =
WorkspaceFactoryHelper.createAndAddRepositoryRule(
builder,
ruleClass,
bindRuleClass,
WorkspaceFactoryHelper.getFinalKwargs(kwargs),
thread.getCallStack());
RepositoryName.validateUserProvidedRepoName(rule.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
Expand All @@ -47,7 +45,6 @@ public static boolean originatesInWorkspaceSuffix(
public static Rule createAndAddRepositoryRule(
Package.Builder pkgBuilder,
RuleClass ruleClass,
RuleClass bindRuleClass,
Map<String, Object> kwargs,
ImmutableList<StarlarkThread.CallStackEntry> callstack)
throws RuleFactory.InvalidRuleException,
Expand All @@ -57,23 +54,6 @@ public static Rule createAndAddRepositoryRule(
BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
Rule rule = RuleFactory.createRule(pkgBuilder, ruleClass, attributeValues, true, callstack);
overwriteRule(pkgBuilder, rule);
for (Map.Entry<String, Label> entry :
ruleClass.getExternalBindingsFunction().apply(rule).entrySet()) {
Label nameLabel = Label.parseCanonical("//external:" + entry.getKey());
addBindRule(pkgBuilder, bindRuleClass, nameLabel, entry.getValue(), callstack);
}
// NOTE(wyv): What is this madness?? This is the only instance where a repository rule can
// register toolchains upon being instantiated. We should look into converting
// android_{s,n}dk_repository into module extensions.
ImmutableList.Builder<TargetPattern> toolchains = new ImmutableList.Builder<>();
for (String pattern : ruleClass.getToolchainsToRegisterFunction().apply(rule)) {
try {
toolchains.add(TargetPattern.defaultParser().parse(pattern));
} catch (TargetParsingException e) {
throw new LabelSyntaxException(e.getMessage());
}
}
pkgBuilder.addRegisteredToolchains(toolchains.build(), originatesInWorkspaceSuffix(callstack));
return rule;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ private void setUpContextForRule(
WorkspaceFactoryHelper.createAndAddRepositoryRule(
packageBuilder,
buildRuleClass(attributes),
null,
kwargs,
DUMMY_STACK);
DownloadManager downloader = Mockito.mock(DownloadManager.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import static com.google.devtools.build.lib.packages.BuildType.OUTPUT_LIST;
import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.substitutePlaceholderIntoTemplate;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;
import static com.google.devtools.build.lib.packages.RuleClass.NO_EXTERNAL_BINDINGS;
import static com.google.devtools.build.lib.packages.RuleClass.NO_TOOLCHAINS_TO_REGISTER;
import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
import static com.google.devtools.build.lib.packages.Type.INTEGER;
import static com.google.devtools.build.lib.packages.Type.STRING;
Expand Down Expand Up @@ -1050,8 +1048,6 @@ private static RuleClass newRuleClass(
configuredTargetFactory,
advertisedProviders,
configuredTargetFunction,
NO_EXTERNAL_BINDINGS,
NO_TOOLCHAINS_TO_REGISTER,
/* optionReferenceFunction= */ RuleClass.NO_OPTION_REFERENCE,
/* ruleDefinitionEnvironmentLabel= */ null,
/* ruleDefinitionEnvironmentDigest= */ null,
Expand Down

0 comments on commit 5d4400c

Please sign in to comment.