diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 90b79587a2d6d9..844f1f09cac985 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -269,7 +269,6 @@ private Object createRuleLegacy(StarlarkThread thread, Dict kwar return WorkspaceFactoryHelper.createAndAddRepositoryRule( pkgBuilder, ruleClass, - /* bindRuleClass= */ null, WorkspaceFactoryHelper.getFinalKwargs(kwargs), thread.getCallStack()); } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index d29c48c6fe85cb..ffa376fd6c6217 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -149,14 +149,6 @@ public class RuleClass implements RuleClassData { */ private static final int MAX_ATTRIBUTE_NAME_LENGTH = 128; - @SerializationConstant - static final Function> NO_EXTERNAL_BINDINGS = - Functions.constant(ImmutableMap.of()); - - @SerializationConstant - static final Function> NO_TOOLCHAINS_TO_REGISTER = - Functions.constant(ImmutableList.of()); - @SerializationConstant static final Function> NO_OPTION_REFERENCE = Functions.constant(ImmutableSet.of()); @@ -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) @@ -687,10 +679,6 @@ public String toString() { private BuildSetting buildSetting = null; private ImmutableList subrules = ImmutableList.of(); - private Function> externalBindingsFunction = - NO_EXTERNAL_BINDINGS; - private Function> toolchainsToRegisterFunction = - NO_TOOLCHAINS_TO_REGISTER; private Function> optionReferenceFunction = NO_OPTION_REFERENCE; @@ -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); @@ -935,8 +919,6 @@ private RuleClass build(String name, String key) { configuredTargetFactory, advertisedProviders.build(), configuredTargetFunction, - externalBindingsFunction, - toolchainsToRegisterFunction, optionReferenceFunction, ruleDefinitionEnvironmentLabel, ruleDefinitionEnvironmentDigest, @@ -1363,19 +1345,6 @@ public ImmutableSet getParentSubrules() { return builder.build(); } - @CanIgnoreReturnValue - public Builder setExternalBindingsFunction(Function> func) { - this.externalBindingsFunction = func; - return this; - } - - @CanIgnoreReturnValue - public Builder setToolchainsToRegisterFunction( - Function> func) { - this.toolchainsToRegisterFunction = func; - return this; - } - /** * Sets the rule definition environment label and transitive digest. Meant for Starlark usage. */ @@ -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; @@ -1754,12 +1723,6 @@ public Attribute.Builder copy(String name) { */ private final ImmutableSet subrules; - /** Returns the extra bindings a workspace function adds to the WORKSPACE file. */ - private final Function> externalBindingsFunction; - - /** Returns the toolchains a workspace function wants to have registered in the WORKSPACE file. */ - private final Function> toolchainsToRegisterFunction; - /** Returns the options referenced by this rule's attributes. */ private final Function> optionReferenceFunction; @@ -1843,8 +1806,6 @@ public Attribute.Builder copy(String name) { ConfiguredTargetFactory configuredTargetFactory, AdvertisedProviderSet advertisedProviders, @Nullable StarlarkCallable configuredTargetFunction, - Function> externalBindingsFunction, - Function> toolchainsToRegisterFunction, Function> optionReferenceFunction, @Nullable Label ruleDefinitionEnvironmentLabel, @Nullable byte[] ruleDefinitionEnvironmentDigest, @@ -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; @@ -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> getExternalBindingsFunction() { - return externalBindingsFunction; - } - - /** - * Returns a function that computes the toolchains that should be registered for a repository - * function. - */ - Function> getToolchainsToRegisterFunction() { - return toolchainsToRegisterFunction; - } - /** Returns a function that computes the options referenced by a rule. */ public Function> getOptionReferenceFunction() { return optionReferenceFunction; diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 26b658ad146f33..4f5f96e6f2664b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -262,12 +262,10 @@ public Object call(StarlarkThread thread, Tuple args, Dict 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()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java index 1f19413ca64c6f..d3d6ed25fd7014 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java @@ -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; @@ -47,7 +45,6 @@ public static boolean originatesInWorkspaceSuffix( public static Rule createAndAddRepositoryRule( Package.Builder pkgBuilder, RuleClass ruleClass, - RuleClass bindRuleClass, Map kwargs, ImmutableList callstack) throws RuleFactory.InvalidRuleException, @@ -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 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 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; } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java index ac719c55583614..15a8b13fe723fc 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java @@ -154,7 +154,6 @@ private void setUpContextForRule( WorkspaceFactoryHelper.createAndAddRepositoryRule( packageBuilder, buildRuleClass(attributes), - null, kwargs, DUMMY_STACK); DownloadManager downloader = Mockito.mock(DownloadManager.class); diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index 5a1fdf23424f27..3d6bf361f6c200 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -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; @@ -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,