From 4baebd5eb2bc53cd21331ec0bc6fdd8c0251d6c1 Mon Sep 17 00:00:00 2001
From: Luca Di Grazia <luca.digrazia94@gmail.com>
Date: Sun, 4 Sep 2022 15:51:11 +0200
Subject: [PATCH]     bzlmod: Store a Package instead of a Rule in
 BzlmodRepoRuleValue

    (https://github.com/bazelbuild/bazel/issues/13316)

    This is to work around a restriction in Google. The external packages we create in BzlmodRepoRuleFunction host just one rule each; so instead of storing a rule, we can just store the package and get the rule from there.

    This does require adding the rule to the package, which we weren't doing before. This involved a bit of a detour to clean up the RuleFactory#createAndAddRule method, which for some reason has had a much more sensible overload that's package private since time immemorial. This CL simply removes the overload that just gets two fields out of PackageContext.

    PiperOrigin-RevId: 387543141
---
 .../starlark/StarlarkRuleClassFunctions.java  |  566 +++---
 .../bazel/bzlmod/BzlmodRepoRuleCreator.java   |    5 +-
 .../lib/bazel/bzlmod/BzlmodRepoRuleValue.java |   14 +-
 .../starlark/StarlarkRepositoryModule.java    |   73 +-
 .../build/lib/packages/PackageFactory.java    | 1800 +++++++----------
 .../build/lib/packages/RuleFactory.java       |  299 ++-
 .../lib/skyframe/BzlmodRepoRuleFunction.java  |  129 +-
 .../build/lib/packages/RuleFactoryTest.java   |  267 +--
 8 files changed, 1600 insertions(+), 1553 deletions(-)

diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
index 3a68926e525..71efd94b35e 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis.starlark;
 
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER;
+import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TIMEOUT_DEFAULT;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.getTestRuntimeLabelList;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
@@ -25,36 +26,43 @@
 import static com.google.devtools.build.lib.packages.Type.STRING;
 import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
 
+import com.github.benmanes.caffeine.cache.CacheLoader;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.base.Preconditions;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.Allowlist;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
-import com.google.devtools.build.lib.analysis.RuleDefinitionContext;
+import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
 import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory.TransitionType;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AllowlistChecker;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate;
-import com.google.devtools.build.lib.packages.AttributeContainer;
-import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
 import com.google.devtools.build.lib.packages.BazelModuleContext;
 import com.google.devtools.build.lib.packages.BazelStarlarkContext;
 import com.google.devtools.build.lib.packages.BuildSetting;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.BuildType.LabelConversionContext;
+import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
 import com.google.devtools.build.lib.packages.ExecGroup;
 import com.google.devtools.build.lib.packages.FunctionSplitTransitionAllowlist;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.StarlarkImplicitOutputsFunctionWithCallback;
@@ -65,10 +73,12 @@
 import com.google.devtools.build.lib.packages.Provider;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.RuleClass.ToolchainTransitionMode;
 import com.google.devtools.build.lib.packages.RuleFactory;
 import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.packages.RuleFunction;
+import com.google.devtools.build.lib.packages.RuleTransitionData;
 import com.google.devtools.build.lib.packages.StarlarkAspect;
 import com.google.devtools.build.lib.packages.StarlarkCallbackHelper;
 import com.google.devtools.build.lib.packages.StarlarkDefinedAspect;
@@ -76,38 +86,40 @@
 import com.google.devtools.build.lib.packages.StarlarkProvider;
 import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.packages.TestSize;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.packages.Type.ConversionException;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.skylarkbuildapi.StarlarkRuleFunctionsApi;
-import com.google.devtools.build.lib.syntax.Dict;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Identifier;
-import com.google.devtools.build.lib.syntax.Location;
-import com.google.devtools.build.lib.syntax.Module;
-import com.google.devtools.build.lib.syntax.Printer;
-import com.google.devtools.build.lib.syntax.Sequence;
-import com.google.devtools.build.lib.syntax.Starlark;
-import com.google.devtools.build.lib.syntax.StarlarkCallable;
-import com.google.devtools.build.lib.syntax.StarlarkFunction;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
-import com.google.devtools.build.lib.syntax.Tuple;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
+import com.google.devtools.build.lib.starlarkbuildapi.StarlarkRuleFunctionsApi;
+import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
+import com.google.errorprone.annotations.FormatMethod;
 import java.util.Collection;
 import java.util.Map;
-import java.util.concurrent.ExecutionException;
+import javax.annotation.Nullable;
+import net.starlark.java.eval.Debug;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Printer;
+import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkCallable;
+import net.starlark.java.eval.StarlarkFunction;
+import net.starlark.java.eval.StarlarkInt;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Identifier;
+import net.starlark.java.syntax.Location;
 
 /** A helper class to provide an easier API for Starlark rule definitions. */
 public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi<Artifact> {
-
   // TODO(bazel-team): Copied from ConfiguredRuleClassProvider for the transition from built-in
   // rules to Starlark extensions. Using the same instance would require a large refactoring.
   // If we don't want to support old built-in rules and Starlark simultaneously
   // (except for transition phase) it's probably OK.
   private static final LoadingCache<String, Label> labelCache =
-      CacheBuilder.newBuilder()
+      Caffeine.newBuilder()
           .build(
               new CacheLoader<String, Label>() {
                 @Override
@@ -127,8 +139,7 @@ public Label load(String from) throws Exception {
   /** Parent rule class for non-executable non-test Starlark rules. */
   public static final RuleClass baseRule =
       BaseRuleClasses.commonCoreAndStarlarkAttributes(
-              BaseRuleClasses.nameAttribute(
-                      new RuleClass.Builder("$base_rule", RuleClassType.ABSTRACT, true))
+              new RuleClass.Builder("$base_rule", RuleClassType.ABSTRACT, true)
                   .add(attr("expect_failure", STRING)))
           // TODO(skylark-team): Allow Starlark rules to extend native rules and remove duplication.
           .add(
@@ -136,12 +147,23 @@ public Label load(String from) throws Exception {
                   .allowedFileTypes(FileTypeSet.NO_FILE)
                   .mandatoryProviders(ImmutableList.of(TemplateVariableInfo.PROVIDER.id()))
                   .dontCheckConstraints())
-          .add(attr(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT).value(ImmutableMap.of()))
+          .add(attr(RuleClass.EXEC_PROPERTIES_ATTR, Type.STRING_DICT).value(ImmutableMap.of()))
           .add(
               attr(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)
                   .allowedFileTypes()
                   .nonconfigurable("Used in toolchain resolution")
+                  .tool(
+                      "exec_compatible_with exists for constraint checking, not to create an"
+                          + " actual dependency")
                   .value(ImmutableList.of()))
+          .add(
+              attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST)
+                  .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
+                  // This should be configurable to allow for complex types of restrictions.
+                  .tool(
+                      "target_compatible_with exists for constraint checking, not to create an"
+                          + " actual dependency")
+                  .allowedFileTypes(FileTypeSet.NO_FILE))
           .build();
 
   /** Parent rule class for executable non-test Starlark rules. */
@@ -152,103 +174,97 @@ public Label load(String from) throws Exception {
           .build();
 
   /** Parent rule class for test Starlark rules. */
-  public static final RuleClass getTestBaseRule(RuleDefinitionContext env) {
+  public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) {
     String toolsRepository = env.getToolsRepository();
-    return new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule)
-        .requiresConfigurationFragments(TestConfiguration.class)
-        .add(
-            attr("size", STRING)
-                .value("medium")
-                .taggable()
-                .nonconfigurable("used in loading phase rule validation logic"))
-        .add(
-            attr("timeout", STRING)
-                .taggable()
-                .nonconfigurable("used in loading phase rule validation logic")
-                .value(timeoutAttribute))
-        .add(
-            attr("flaky", BOOLEAN)
-                .value(false)
-                .taggable()
-                .nonconfigurable("taggable - called in Rule.getRuleTags"))
-        .add(attr("shard_count", INTEGER).value(-1))
-        .add(
-            attr("local", BOOLEAN)
-                .value(false)
-                .taggable()
-                .nonconfigurable(
-                    "policy decision: this should be consistent across configurations"))
-        .add(attr("args", STRING_LIST))
-        // Input files for every test action
-        .add(
-            attr("$test_wrapper", LABEL)
-                .cfg(HostTransition.createFactory())
-                .singleArtifact()
-                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_wrapper")))
-        .add(
-            attr("$xml_writer", LABEL)
-                .cfg(HostTransition.createFactory())
-                .singleArtifact()
-                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:xml_writer")))
-        .add(
-            attr("$test_runtime", LABEL_LIST)
-                .cfg(HostTransition.createFactory())
-                // Getting this default value through the getTestRuntimeLabelList helper ensures we
-                // reuse the same ImmutableList<Label> instance for each $test_runtime attr.
-                .value(getTestRuntimeLabelList(env)))
-        .add(
-            attr("$test_setup_script", LABEL)
-                .cfg(HostTransition.createFactory())
-                .singleArtifact()
-                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_setup")))
-        .add(
-            attr("$xml_generator_script", LABEL)
-                .cfg(HostTransition.createFactory())
-                .singleArtifact()
-                .value(
-                    labelCache.getUnchecked(toolsRepository + "//tools/test:test_xml_generator")))
-        .add(
-            attr("$collect_coverage_script", LABEL)
-                .cfg(HostTransition.createFactory())
-                .singleArtifact()
-                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:collect_coverage")))
-        // Input files for test actions collecting code coverage
-        .add(
-            attr(":coverage_support", LABEL)
-                .cfg(HostTransition.createFactory())
-                .value(
-                    BaseRuleClasses.coverageSupportAttribute(
-                        labelCache.getUnchecked(
-                            toolsRepository + BaseRuleClasses.DEFAULT_COVERAGE_SUPPORT_VALUE))))
-        // Used in the one-per-build coverage report generation action.
-        .add(
-            attr(":coverage_report_generator", LABEL)
-                .cfg(HostTransition.createFactory())
-                .value(
-                    BaseRuleClasses.coverageReportGeneratorAttribute(
-                        labelCache.getUnchecked(
-                            toolsRepository
-                                + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))))
-        .add(attr(":run_under", LABEL).value(RUN_UNDER))
-        .build();
+    RuleClass.Builder builder =
+        new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule)
+            .requiresConfigurationFragments(TestConfiguration.class)
+            // TestConfiguration only needed to create TestAction and TestProvider
+            // Only necessary at top-level and can be skipped if trimmed.
+            .setMissingFragmentPolicy(TestConfiguration.class, MissingFragmentPolicy.IGNORE)
+            .add(
+                attr("size", STRING)
+                    .value("medium")
+                    .taggable()
+                    .nonconfigurable("used in loading phase rule validation logic"))
+            .add(
+                attr("timeout", STRING)
+                    .taggable()
+                    .nonconfigurable("policy decision: should be consistent across configurations")
+                    .value(TIMEOUT_DEFAULT))
+            .add(
+                attr("flaky", BOOLEAN)
+                    .value(false)
+                    .taggable()
+                    .nonconfigurable("taggable - called in Rule.getRuleTags"))
+            .add(attr("shard_count", INTEGER).value(StarlarkInt.of(-1)))
+            .add(
+                attr("local", BOOLEAN)
+                    .value(false)
+                    .taggable()
+                    .nonconfigurable(
+                        "policy decision: this should be consistent across configurations"))
+            .add(attr("args", STRING_LIST))
+            // Input files for every test action
+            .add(
+                attr("$test_wrapper", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .singleArtifact()
+                    .value(labelCache.get(toolsRepository + "//tools/test:test_wrapper")))
+            .add(
+                attr("$xml_writer", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .singleArtifact()
+                    .value(labelCache.get(toolsRepository + "//tools/test:xml_writer")))
+            .add(
+                attr("$test_runtime", LABEL_LIST)
+                    .cfg(HostTransition.createFactory())
+                    // Getting this default value through the getTestRuntimeLabelList helper ensures
+                    // we reuse the same ImmutableList<Label> instance for each $test_runtime attr.
+                    .value(getTestRuntimeLabelList(env)))
+            .add(
+                attr("$test_setup_script", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .singleArtifact()
+                    .value(labelCache.get(toolsRepository + "//tools/test:test_setup")))
+            .add(
+                attr("$xml_generator_script", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .singleArtifact()
+                    .value(labelCache.get(toolsRepository + "//tools/test:test_xml_generator")))
+            .add(
+                attr("$collect_coverage_script", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .singleArtifact()
+                    .value(labelCache.get(toolsRepository + "//tools/test:collect_coverage")))
+            // Input files for test actions collecting code coverage
+            .add(
+                attr(":coverage_support", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .value(
+                        BaseRuleClasses.coverageSupportAttribute(
+                            labelCache.get(
+                                toolsRepository + BaseRuleClasses.DEFAULT_COVERAGE_SUPPORT_VALUE))))
+            // Used in the one-per-build coverage report generation action.
+            .add(
+                attr(":coverage_report_generator", LABEL)
+                    .cfg(HostTransition.createFactory())
+                    .value(
+                        BaseRuleClasses.coverageReportGeneratorAttribute(
+                            labelCache.get(
+                                toolsRepository
+                                    + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))))
+            .add(attr(":run_under", LABEL).value(RUN_UNDER));
+
+    env.getNetworkAllowlistForTests()
+        .ifPresent(
+            label ->
+                builder.add(
+                    Allowlist.getAttributeFromAllowlistName("$network_allowlist").value(label)));
+
+    return builder.build();
   }
 
-  @AutoCodec @AutoCodec.VisibleForSerialization
-  static final Attribute.ComputedDefault timeoutAttribute =
-      new Attribute.ComputedDefault() {
-        @Override
-        public Object getDefault(AttributeMap rule) {
-          TestSize size = TestSize.getTestSize(rule.get("size", Type.STRING));
-          if (size != null) {
-            String timeout = size.getDefaultTimeout().toString();
-            if (timeout != null) {
-              return timeout;
-            }
-          }
-          return "illegal";
-        }
-      };
-
   @Override
   public Provider provider(String doc, Object fields, StarlarkThread thread) throws EvalException {
     Collection<String> fieldNames =
@@ -281,6 +297,7 @@ public StarlarkCallable rule(
       Object buildSetting,
       Object cfg,
       Object execGroups,
+      Object compileOneFiletype,
       StarlarkThread thread)
       throws EvalException {
     BazelStarlarkContext bazelContext = BazelStarlarkContext.from(thread);
@@ -309,8 +326,7 @@ public StarlarkCallable rule(
     }
 
     if (executable || test) {
-      addAttribute(
-          builder,
+      builder.addAttribute(
           attr("$is_executable", BOOLEAN)
               .value(true)
               .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")
@@ -320,11 +336,15 @@ public StarlarkCallable rule(
 
     if (implicitOutputs != Starlark.NONE) {
       if (implicitOutputs instanceof StarlarkFunction) {
+        // TODO(brandjon): Embedding bazelContext in a callback is not thread safe! Instead
+        // construct a new BazelStarlarkContext with copies of the relevant fields that are safe to
+        // share. Maybe create a method on BazelStarlarkContext for safely constructing a child
+        // context.
         StarlarkCallbackHelper callback =
             new StarlarkCallbackHelper(
                 (StarlarkFunction) implicitOutputs, thread.getSemantics(), bazelContext);
         builder.setImplicitOutputsFunction(
-            new StarlarkImplicitOutputsFunctionWithCallback(callback, thread.getCallerLocation()));
+            new StarlarkImplicitOutputsFunctionWithCallback(callback));
       } else {
         builder.setImplicitOutputsFunction(
             new StarlarkImplicitOutputsFunctionWithMap(
@@ -347,21 +367,32 @@ public StarlarkCallable rule(
         .requiresHostConfigurationFragmentsByStarlarkBuiltinName(
             Sequence.cast(hostFragments, String.class, "host_fragments"));
     builder.setConfiguredTargetFunction(implementation);
-    // Information about the .bzl module containing the call that created the rule class.
+    // Obtain the rule definition environment (RDE) from the .bzl module being initialized by the
+    // calling thread -- the label and transitive source digest of the .bzl module of the outermost
+    // function in the call stack.
+    //
+    // If this thread is initializing a BUILD file, then the toplevel function's Module has
+    // no BazelModuleContext. Such rules cannot be instantiated, so it's ok to use a
+    // dummy label and RDE in that case (but not to crash).
     BazelModuleContext bzlModule =
-        (BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
+        BazelModuleContext.of(getModuleOfOutermostStarlarkFunction(thread));
     builder.setRuleDefinitionEnvironmentLabelAndDigest(
-        bzlModule.label(), bzlModule.bzlTransitiveDigest());
+        bzlModule != null
+            ? bzlModule.label()
+            : Label.createUnvalidated(PackageIdentifier.EMPTY_PACKAGE_ID, "dummy_label"),
+        bzlModule != null ? bzlModule.bzlTransitiveDigest() : new byte[0]);
 
     builder.addRequiredToolchains(parseToolchains(toolchains, thread));
-    builder.useToolchainTransition(useToolchainTransition);
+    if (useToolchainTransition) {
+      builder.useToolchainTransition(ToolchainTransitionMode.ENABLED);
+    }
 
     if (execGroups != Starlark.NONE) {
       Map<String, ExecGroup> execGroupDict =
           Dict.cast(execGroups, String.class, ExecGroup.class, "exec_group");
       for (String group : execGroupDict.keySet()) {
         // TODO(b/151742236): document this in the param documentation.
-        if (!ExecGroupCollection.isValidGroupName(group)) {
+        if (!StarlarkExecGroupCollection.isValidGroupName(group)) {
           throw Starlark.errorf("Exec group name '%s' is not a valid name.", group);
         }
       }
@@ -376,14 +407,32 @@ public StarlarkCallable rule(
       builder.setBuildSetting((BuildSetting) buildSetting);
     }
     if (!cfg.equals(Starlark.NONE)) {
-      if (!(cfg instanceof StarlarkDefinedConfigTransition)) {
+      if (cfg instanceof StarlarkDefinedConfigTransition) {
+        StarlarkDefinedConfigTransition starlarkDefinedConfigTransition =
+            (StarlarkDefinedConfigTransition) cfg;
+        builder.cfg(new StarlarkRuleTransitionProvider(starlarkDefinedConfigTransition));
+        builder.setHasStarlarkRuleTransition();
+      } else if (cfg instanceof PatchTransition) {
+        builder.cfg((PatchTransition) cfg);
+      } else if (cfg instanceof TransitionFactory) {
+        TransitionFactory<? extends TransitionFactory.Data> transitionFactory =
+            (TransitionFactory<? extends TransitionFactory.Data>) cfg;
+        if (transitionFactory.transitionType().isCompatibleWith(TransitionType.RULE)) {
+          @SuppressWarnings("unchecked") // Actually checked due to above isCompatibleWith call.
+          TransitionFactory<RuleTransitionData> ruleTransitionFactory =
+              (TransitionFactory<RuleTransitionData>) transitionFactory;
+          builder.cfg(ruleTransitionFactory);
+        } else {
+          throw Starlark.errorf(
+              "`cfg` must be set to a transition appropriate for a rule, not an attribute-specific"
+                  + " transition.");
+        }
+      } else {
+        // This is not technically true: it could also be a native transition, but this is the
+        // most likely error case.
         throw Starlark.errorf(
             "`cfg` must be set to a transition object initialized by the transition() function.");
       }
-      StarlarkDefinedConfigTransition starlarkDefinedConfigTransition =
-          (StarlarkDefinedConfigTransition) cfg;
-      builder.cfg(new StarlarkRuleTransitionProvider(starlarkDefinedConfigTransition));
-      builder.setHasStarlarkRuleTransition();
     }
 
     for (Object o : providesArg) {
@@ -403,9 +452,27 @@ public StarlarkCallable rule(
       builder.addExecutionPlatformConstraints(parseExecCompatibleWith(execCompatibleWith, thread));
     }
 
+    if (compileOneFiletype instanceof String) {
+      builder.setPreferredDependencyPredicate(FileType.of((String) compileOneFiletype));
+    }
+
     return new StarlarkRuleFunction(builder, type, attributes, thread.getCallerLocation());
   }
 
+  /**
+   * Returns the module (file) of the outermost enclosing Starlark function on the call stack or
+   * null if none of the active calls are functions defined in Starlark.
+   */
+  @Nullable
+  private static Module getModuleOfOutermostStarlarkFunction(StarlarkThread thread) {
+    for (Debug.Frame fr : Debug.getCallStack(thread)) {
+      if (fr.getFunction() instanceof StarlarkFunction) {
+        return ((StarlarkFunction) fr.getFunction()).getModule();
+      }
+    }
+    return null;
+  }
+
   private static void checkAttributeName(String name) throws EvalException {
     if (!Identifier.isValid(name)) {
       throw Starlark.errorf("attribute name `%s` is not a valid identifier.", name);
@@ -430,32 +497,26 @@ private static void checkAttributeName(String name) throws EvalException {
     return attributes.build();
   }
 
-  private static void addAttribute(RuleClass.Builder builder, Attribute attribute)
-      throws EvalException {
-    try {
-      builder.addAttribute(attribute);
-    } catch (IllegalStateException ex) {
-      throw new EvalException(null, ex);
-    }
-  }
-
   /**
    * Parses a sequence of label strings with a repo mapping.
    *
    * @param inputs sequence of input strings
-   * @param mapping repository mapping
+   * @param thread repository mapping
    * @param adjective describes the purpose of the label; used for errors
    * @throws EvalException if the label can't be parsed
    */
   private static ImmutableList<Label> parseLabels(
-      Iterable<String> inputs,
-      ImmutableMap<RepositoryName, RepositoryName> mapping,
-      String adjective)
-      throws EvalException {
+      Iterable<String> inputs, StarlarkThread thread, String adjective) throws EvalException {
     ImmutableList.Builder<Label> parsedLabels = new ImmutableList.Builder<>();
+    BazelStarlarkContext bazelStarlarkContext = BazelStarlarkContext.from(thread);
+    LabelConversionContext context =
+        new LabelConversionContext(
+            BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread)).label(),
+            bazelStarlarkContext.getRepoMapping(),
+            bazelStarlarkContext.getConvertedLabelsInPackage());
     for (String input : inputs) {
       try {
-        Label label = Label.parseAbsolute(input, mapping);
+        Label label = context.convert(input);
         parsedLabels.add(label);
       } catch (LabelSyntaxException e) {
         throw Starlark.errorf(
@@ -467,18 +528,13 @@ private static ImmutableList<Label> parseLabels(
 
   private static ImmutableList<Label> parseToolchains(Sequence<?> inputs, StarlarkThread thread)
       throws EvalException {
-    return parseLabels(
-        Sequence.cast(inputs, String.class, "toolchains"),
-        BazelStarlarkContext.from(thread).getRepoMapping(),
-        "toolchain");
+    return parseLabels(Sequence.cast(inputs, String.class, "toolchains"), thread, "toolchain");
   }
 
   private static ImmutableList<Label> parseExecCompatibleWith(
       Sequence<?> inputs, StarlarkThread thread) throws EvalException {
     return parseLabels(
-        Sequence.cast(inputs, String.class, "exec_compatible_with"),
-        BazelStarlarkContext.from(thread).getRepoMapping(),
-        "constraint");
+        Sequence.cast(inputs, String.class, "exec_compatible_with"), thread, "constraint");
   }
 
   @Override
@@ -486,8 +542,10 @@ public StarlarkAspect aspect(
       StarlarkFunction implementation,
       Sequence<?> attributeAspects,
       Object attrs,
+      Sequence<?> requiredProvidersArg,
       Sequence<?> requiredAspectProvidersArg,
       Sequence<?> providesArg,
+      Sequence<?> requiredAspects,
       Sequence<?> fragments,
       Sequence<?> hostFragments,
       Sequence<?> toolchains,
@@ -501,7 +559,7 @@ public StarlarkAspect aspect(
       String attrName = STRING.convert(attributeAspect, "attr_aspects");
 
       if (attrName.equals("*") && attributeAspects.size() != 1) {
-        throw new EvalException(null, "'*' must be the only string in 'attr_aspects' list");
+        throw new EvalException("'*' must be the only string in 'attr_aspects' list");
       }
 
       if (!attrName.startsWith("_")) {
@@ -528,12 +586,10 @@ public StarlarkAspect aspect(
       }
       if (!Attribute.isImplicit(nativeName) && !Attribute.isLateBound(nativeName)) {
         if (!attribute.checkAllowedValues() || attribute.getType() != Type.STRING) {
-          throw new EvalException(
-              null,
-              String.format(
-                  "Aspect parameter attribute '%s' must have type 'string' and use the "
-                      + "'values' restriction.",
-                  nativeName));
+          throw Starlark.errorf(
+              "Aspect parameter attribute '%s' must have type 'string' and use the 'values'"
+                  + " restriction.",
+              nativeName);
         }
         if (!hasDefault) {
           requiredParams.add(nativeName);
@@ -541,17 +597,14 @@ public StarlarkAspect aspect(
           PredicateWithMessage<Object> allowed = attribute.getAllowedValues();
           Object defaultVal = attribute.getDefaultValue(null);
           if (!allowed.apply(defaultVal)) {
-            throw new EvalException(
-                null,
-                String.format(
-                    "Aspect parameter attribute '%s' has a bad default value: %s",
-                    nativeName, allowed.getErrorReason(defaultVal)));
+            throw Starlark.errorf(
+                "Aspect parameter attribute '%s' has a bad default value: %s",
+                nativeName, allowed.getErrorReason(defaultVal));
           }
         }
       } else if (!hasDefault) { // Implicit or late bound attribute
         String starlarkName = "_" + nativeName.substring(1);
-        throw new EvalException(
-            null, String.format("Aspect attribute '%s' has no default value.", starlarkName));
+        throw Starlark.errorf("Aspect attribute '%s' has no default value.", starlarkName);
       }
       if (attribute.getDefaultValueUnchecked() instanceof StarlarkComputedDefaultTemplate) {
         // Attributes specifying dependencies using computed value are currently not supported.
@@ -561,33 +614,37 @@ public StarlarkAspect aspect(
         // Current logic in StarlarkComputedDefault is not enough,
         // however {Conservative,Precise}AspectResolver can probably be improved to make that work.
         String starlarkName = "_" + nativeName.substring(1);
-        throw new EvalException(
-            null,
-            String.format(
-                "Aspect attribute '%s' (%s) with computed default value is unsupported.",
-                starlarkName, attribute.getType()));
+        throw Starlark.errorf(
+            "Aspect attribute '%s' (%s) with computed default value is unsupported.",
+            starlarkName, attribute.getType());
       }
       attributes.add(attribute);
     }
 
     for (Object o : providesArg) {
       if (!StarlarkAttrModule.isProvider(o)) {
-        throw new EvalException(
-            null,
-            String.format(
-                "Illegal argument: element in 'provides' is of unexpected type. "
-                    + "Should be list of providers, but got item of type %s. ",
-                Starlark.type(o)));
+        throw Starlark.errorf(
+            "Illegal argument: element in 'provides' is of unexpected type. "
+                + "Should be list of providers, but got item of type %s. ",
+            Starlark.type(o));
       }
     }
+
+    if (applyToGeneratingRules && !requiredProvidersArg.isEmpty()) {
+      throw Starlark.errorf(
+          "An aspect cannot simultaneously have required providers and apply to generating rules.");
+    }
+
     return new StarlarkDefinedAspect(
         implementation,
         attrAspects.build(),
         attributes.build(),
+        StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"),
         StarlarkAttrModule.buildProviderPredicate(
             requiredAspectProvidersArg, "required_aspect_providers"),
         StarlarkAttrModule.getStarlarkProviderIdentifiers(providesArg),
         requiredParams.build(),
+        ImmutableSet.copyOf(Sequence.cast(requiredAspects, StarlarkAspect.class, "requires")),
         ImmutableSet.copyOf(Sequence.cast(fragments, String.class, "fragments")),
         HostTransition.INSTANCE,
         ImmutableSet.copyOf(Sequence.cast(hostFragments, String.class, "host_fragments")),
@@ -601,8 +658,7 @@ public StarlarkAspect aspect(
    *
    * <p>Exactly one of {@link #builder} or {@link #ruleClass} is null except inside {@link #export}.
    */
-  public static final class StarlarkRuleFunction
-      implements StarlarkCallable, StarlarkExportable, RuleFunction {
+  public static final class StarlarkRuleFunction implements StarlarkExportable, RuleFunction {
     private RuleClass.Builder builder;
 
     private RuleClass ruleClass;
@@ -611,6 +667,10 @@ public static final class StarlarkRuleFunction
     private final Location definitionLocation;
     private Label starlarkLabel;
 
+    // TODO(adonovan): merge {Starlark,Builtin}RuleFunction and RuleClass,
+    // making the latter a callable, StarlarkExportable value.
+    // (Making RuleClasses first-class values will help us to build a
+    // rich query output mode that includes values from loaded .bzl files.)
     public StarlarkRuleFunction(
         RuleClass.Builder builder,
         RuleClassType type,
@@ -639,18 +699,18 @@ private StarlarkRuleFunction(
 
     @Override
     public String getName() {
-      return "rule";
+      return ruleClass != null ? ruleClass.getName() : "unexported rule";
     }
 
     @Override
-    public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Object> kwargs)
+    public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
         throws EvalException, InterruptedException, ConversionException {
       if (!args.isEmpty()) {
-        throw new EvalException(null, "unexpected positional arguments");
+        throw new EvalException("unexpected positional arguments");
       }
       BazelStarlarkContext.from(thread).checkLoadingPhase(getName());
       if (ruleClass == null) {
-        throw new EvalException(null, "Invalid rule class hasn't been exported by a bzl file");
+        throw new EvalException("Invalid rule class hasn't been exported by a bzl file");
       }
 
       for (Attribute attribute : ruleClass.getAttributes()) {
@@ -674,33 +734,34 @@ public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Objec
         PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
         if (pkgContext == null) {
           throw new EvalException(
-              null,
               "Cannot instantiate a rule when loading a .bzl file. "
                   + "Rules may be instantiated only in a BUILD thread.");
         }
         RuleFactory.createAndAddRule(
-            pkgContext,
+            pkgContext.getBuilder(),
             ruleClass,
             attributeValues,
+            pkgContext.getEventHandler(),
             thread.getSemantics(),
-            thread.getCallStack(),
-            new AttributeContainer(ruleClass));
+            thread.getCallStack());
       } catch (InvalidRuleException | NameConflictException e) {
-        throw new EvalException(null, e.getMessage());
+        throw new EvalException(e);
       }
       return Starlark.NONE;
     }
 
     /** Export a RuleFunction from a Starlark file with a given name. */
-    public void export(Label starlarkLabel, String ruleClassName) throws EvalException {
+    @Override
+    public void export(EventHandler handler, Label starlarkLabel, String ruleClassName) {
       Preconditions.checkState(ruleClass == null && builder != null);
       this.starlarkLabel = starlarkLabel;
       if (type == RuleClassType.TEST != TargetUtils.isTestRuleName(ruleClassName)) {
-        throw new EvalException(
-            definitionLocation,
-            "Invalid rule class name '"
-                + ruleClassName
-                + "', test rule class names must end with '_test' and other rule classes must not");
+        errorf(
+            handler,
+            "Invalid rule class name '%s', test rule class names must end with '_test' and other"
+                + " rule classes must not",
+            ruleClassName);
+        return;
       }
       // Thus far, we only know if we have a rule transition. While iterating through attributes,
       // check if we have an attribute transition.
@@ -715,10 +776,11 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti
         hasStarlarkDefinedTransition |= attr.hasStarlarkDefinedTransition();
         if (attr.hasAnalysisTestTransition()) {
           if (!builder.isAnalysisTest()) {
-            throw new EvalException(
-                getLocation(),
-                "Only rule definitions with analysis_test=True may have attributes with "
-                    + "analysis_test_transition transitions");
+            errorf(
+                handler,
+                "Only rule definitions with analysis_test=True may have attributes with"
+                    + " analysis_test_transition transitions");
+            continue;
           }
           builder.setHasAnalysisTestTransition();
         }
@@ -727,13 +789,12 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti
         if (name.equals(FunctionSplitTransitionAllowlist.ATTRIBUTE_NAME)
             || name.equals(FunctionSplitTransitionAllowlist.LEGACY_ATTRIBUTE_NAME)) {
           if (!BuildType.isLabelType(attr.getType())) {
-            throw new EvalException(
-                getLocation(), "_allowlist_function_transition attribute must be a label type");
+            errorf(handler, "_allowlist_function_transition attribute must be a label type");
+            continue;
           }
           if (attr.getDefaultValueUnchecked() == null) {
-            throw new EvalException(
-                getLocation(),
-                "_allowlist_function_transition attribute must have a default value");
+            errorf(handler, "_allowlist_function_transition attribute must have a default value");
+            continue;
           }
           Label defaultLabel = (Label) attr.getDefaultValueUnchecked();
           // Check the label value for package and target name, to make sure this works properly
@@ -750,49 +811,66 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti
                   && defaultLabel
                       .getName()
                       .equals(FunctionSplitTransitionAllowlist.LEGACY_LABEL.getName()))) {
-            throw new EvalException(
-                getLocation(),
-                "_allowlist_function_transition attribute ("
-                    + defaultLabel
-                    + ") does not have the expected value "
-                    + FunctionSplitTransitionAllowlist.LABEL);
+            errorf(
+                handler,
+                "_allowlist_function_transition attribute (%s) does not have the expected value %s",
+                defaultLabel,
+                FunctionSplitTransitionAllowlist.LABEL);
+            continue;
           }
           hasFunctionTransitionAllowlist = true;
-          builder.setHasFunctionTransitionAllowlist();
         }
-        addAttribute(builder, attr);
+
+        try {
+          builder.addAttribute(attr);
+        } catch (IllegalStateException ex) {
+          // TODO(bazel-team): stop using unchecked exceptions in this way.
+          errorf(handler, "cannot add attribute: %s", ex.getMessage());
+        }
       }
       // TODO(b/121385274): remove when we stop allowlisting starlark transitions
       if (hasStarlarkDefinedTransition) {
-        if (!hasFunctionTransitionAllowlist) {
-          throw new EvalException(
-              getLocation(),
-              String.format(
-                  "Use of Starlark transition without allowlist attribute"
-                      + " '_allowlist_function_transition'. See Starlark transitions documentation"
-                      + " for details and usage: %s %s",
-                  builder.getRuleDefinitionEnvironmentLabel(), builder.getType()));
+        if (!starlarkLabel.getRepository().getName().equals("@_builtins")) {
+          if (!hasFunctionTransitionAllowlist) {
+            errorf(
+                handler,
+                "Use of Starlark transition without allowlist attribute"
+                    + " '_allowlist_function_transition'. See Starlark transitions documentation"
+                    + " for details and usage: %s %s",
+                builder.getRuleDefinitionEnvironmentLabel(),
+                builder.getType());
+            return;
+          }
+          builder.addAllowlistChecker(FUNCTION_TRANSITION_ALLOWLIST_CHECKER);
         }
       } else {
         if (hasFunctionTransitionAllowlist) {
-          throw new EvalException(
-              getLocation(),
-              String.format(
-                  "Unused function-based split transition allowlist: %s %s",
-                  builder.getRuleDefinitionEnvironmentLabel(), builder.getType()));
+          errorf(
+              handler,
+              "Unused function-based split transition allowlist: %s %s",
+              builder.getRuleDefinitionEnvironmentLabel(),
+              builder.getType());
+          return;
         }
       }
 
       try {
         this.ruleClass = builder.build(ruleClassName, starlarkLabel + "%" + ruleClassName);
       } catch (IllegalArgumentException | IllegalStateException ex) {
-        throw new EvalException(getLocation(), ex);
+        // TODO(adonovan): this catch statement is an abuse of exceptions. Be more specific.
+        String msg = ex.getMessage();
+        errorf(handler, "%s", msg != null ? msg : ex.toString());
       }
 
       this.builder = null;
       this.attributes = null;
     }
 
+    @FormatMethod
+    private void errorf(EventHandler handler, String format, Object... args) {
+      handler.handle(Event.error(definitionLocation, String.format(format, args)));
+    }
+
     public RuleClass getRuleClass() {
       Preconditions.checkState(ruleClass != null && builder == null);
       return ruleClass;
@@ -819,6 +897,14 @@ public boolean isImmutable() {
     }
   }
 
+  @SerializationConstant
+  static final AllowlistChecker FUNCTION_TRANSITION_ALLOWLIST_CHECKER =
+      AllowlistChecker.builder()
+          .setAllowlistAttr(FunctionSplitTransitionAllowlist.NAME)
+          .setErrorMessage("Non-allowlisted use of Starlark transition")
+          .setLocationCheck(AllowlistChecker.LocationCheck.INSTANCE_OR_DEFINITION)
+          .build();
+
   @Override
   public Label label(String labelString, Boolean relativeToCallerRepository, StarlarkThread thread)
       throws EvalException {
@@ -878,8 +964,7 @@ public Label label(String labelString, Boolean relativeToCallerRepository, Starl
     } else {
       // This is the label of the innermost BUILD/.bzl file on the current call stack.
       parentLabel =
-          ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
-              .label();
+          BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread)).label();
     }
 
     try {
@@ -891,15 +976,26 @@ public Label label(String labelString, Boolean relativeToCallerRepository, Starl
                 .getUnambiguousCanonicalForm();
       }
       return labelCache.get(labelString);
-    } catch (LabelValidator.BadLabelException | LabelSyntaxException | ExecutionException e) {
+    } catch (LabelValidator.BadLabelException | LabelSyntaxException e) {
       throw Starlark.errorf("Illegal absolute label syntax: %s", labelString);
     }
   }
 
   @Override
   public ExecGroup execGroup(
-      Sequence<?> toolchains, Sequence<?> execCompatibleWith, StarlarkThread thread)
+      Sequence<?> toolchains,
+      Sequence<?> execCompatibleWith,
+      Boolean copyFromRule,
+      StarlarkThread thread)
       throws EvalException {
+    if (copyFromRule) {
+      if (!toolchains.isEmpty() || !execCompatibleWith.isEmpty()) {
+        throw Starlark.errorf(
+            "An exec group cannot set copy_from_rule=True and declare toolchains or constraints.");
+      }
+      return ExecGroup.copyFromDefault();
+    }
+
     ImmutableSet<Label> toolchainTypes = ImmutableSet.copyOf(parseToolchains(toolchains, thread));
     ImmutableSet<Label> constraints =
         ImmutableSet.copyOf(parseExecCompatibleWith(execCompatibleWith, thread));
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
index c102f0fac2d..4505fc5c8bb 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
@@ -16,6 +16,7 @@
 
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import java.util.Map;
@@ -26,10 +27,10 @@
  * parameters.
  */
 public interface BzlmodRepoRuleCreator {
-  Rule createRule(
+  Rule createAndAddRule(
       Package.Builder packageBuilder,
       StarlarkSemantics semantics,
       Map<String, Object> kwargs,
       EventHandler handler)
-      throws InterruptedException, InvalidRuleException;
+      throws InterruptedException, InvalidRuleException, NameConflictException;
 }
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java
index 50eb0992f6a..6814e625e7f 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.skyframe.AbstractSkyKey;
@@ -23,18 +24,21 @@
 import com.google.devtools.build.skyframe.SkyValue;
 
 /** The result of {@link BzlmodRepoRuleFunction}, holding a repository rule instance. */
+@AutoCodec(explicitlyAllowClass = {Package.class})
 public class BzlmodRepoRuleValue implements SkyValue {
   public static final SkyFunctionName BZLMOD_REPO_RULE =
       SkyFunctionName.createHermetic("BZLMOD_REPO_RULE");
 
-  private final Rule rule;
+  private final Package pkg;
+  private final String ruleName;
 
-  public BzlmodRepoRuleValue(Rule rule) {
-    this.rule = rule;
+  public BzlmodRepoRuleValue(Package pkg, String ruleName) {
+    this.pkg = pkg;
+    this.ruleName = ruleName;
   }
 
   public Rule getRule() {
-    return rule;
+    return pkg.getRule(ruleName);
   }
 
   public static Key key(String repositoryName) {
@@ -44,7 +48,7 @@ public static Key key(String repositoryName) {
   /** Represents an unsuccessful repository lookup. */
   public static final class RepoRuleNotFoundValue extends BzlmodRepoRuleValue {
     private RepoRuleNotFoundValue() {
-      super(/* rule= */ null);
+      super(/*pkg=*/ null, /*ruleName=*/ null);
     }
 
     @Override
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java
index db39209b903..6dc2363be3f 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java
@@ -20,10 +20,13 @@
 import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.docgen.annot.DocCategory;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
+import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
 import com.google.devtools.build.lib.packages.BazelModuleContext;
 import com.google.devtools.build.lib.packages.BazelStarlarkContext;
@@ -34,20 +37,27 @@
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.RuleFactory;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.packages.StarlarkExportable;
 import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
-import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryModuleApi;
-import com.google.devtools.build.lib.syntax.Dict;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Module;
-import com.google.devtools.build.lib.syntax.Printer;
-import com.google.devtools.build.lib.syntax.Sequence;
-import com.google.devtools.build.lib.syntax.Starlark;
-import com.google.devtools.build.lib.syntax.StarlarkCallable;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
-import com.google.devtools.build.lib.syntax.Tuple;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi;
 import java.util.Map;
+import net.starlark.java.annot.StarlarkBuiltin;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Printer;
+import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkCallable;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.StarlarkThread.CallStackEntry;
+import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Location;
 
 /**
  * The Starlark module containing the definition of {@code repository_rule} function to define a
@@ -77,12 +87,11 @@ public StarlarkCallable repositoryRule(
 
     builder.addAttribute(attr("$local", BOOLEAN).defaultValue(local).build());
     builder.addAttribute(attr("$configure", BOOLEAN).defaultValue(configure).build());
-    if (thread.getSemantics().experimentalRepoRemoteExec()) {
+    if (thread.getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC)) {
       builder.addAttribute(attr("$remotable", BOOLEAN).defaultValue(remotable).build());
       BaseRuleClasses.execPropertiesAttribute(builder);
     }
     builder.addAttribute(attr("$environ", STRING_LIST).defaultValue(environ).build());
-    BaseRuleClasses.nameAttribute(builder);
     BaseRuleClasses.commonCoreAndStarlarkAttributes(builder);
     builder.add(attr("expect_failure", STRING));
     if (attrs != Starlark.NONE) {
@@ -100,7 +109,7 @@ public StarlarkCallable repositoryRule(
     }
     builder.setConfiguredTargetFunction(implementation);
     BazelModuleContext bzlModule =
-        (BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
+        BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
     builder.setRuleDefinitionEnvironmentLabelAndDigest(
         bzlModule.label(), bzlModule.bzlTransitiveDigest());
     builder.setWorkspaceOnly();
@@ -109,8 +118,14 @@ public StarlarkCallable repositoryRule(
 
   // RepositoryRuleFunction is the result of repository_rule(...).
   // It is a callable value; calling it yields a Rule instance.
+  @StarlarkBuiltin(
+      name = "repository_rule",
+      category = DocCategory.BUILTIN,
+      doc =
+          "A callable value that may be invoked during evaluation of the WORKSPACE file to"
+              + " instantiate and return a repository rule.")
   private static final class RepositoryRuleFunction
-      implements StarlarkCallable, StarlarkExportable {
+      implements StarlarkCallable, StarlarkExportable, BzlmodRepoRuleCreator {
     private final RuleClass.Builder builder;
     private final StarlarkCallable implementation;
     private Label extensionLabel;
@@ -132,7 +147,7 @@ public boolean isImmutable() {
     }
 
     @Override
-    public void export(Label extensionLabel, String exportedName) {
+    public void export(EventHandler handler, Label extensionLabel, String exportedName) {
       this.extensionLabel = extensionLabel;
       this.exportedName = exportedName;
     }
@@ -152,10 +167,11 @@ public void repr(Printer printer) {
     }
 
     @Override
-    public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Object> kwargs)
+    public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
         throws EvalException, InterruptedException {
+      BazelStarlarkContext.from(thread).checkWorkspacePhase("repository rule " + exportedName);
       if (!args.isEmpty()) {
-        throw new EvalException(null, "unexpected positional arguments");
+        throw new EvalException("unexpected positional arguments");
       }
       String ruleClassName;
       // If the function ever got exported (the common case), we take the name
@@ -172,7 +188,7 @@ public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Objec
         // now many projects create and instantiate repository_rules without an
         // intervening export; see b/111199163. An incompatible flag is required.
         if (false) {
-          throw new EvalException(null, "attempt to instantiate a non-exported repository rule");
+          throw new EvalException("attempt to instantiate a non-exported repository rule");
         }
 
         // The historical workaround was a fragile hack to introspect on the call
@@ -203,12 +219,31 @@ public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Objec
         throw Starlark.errorf("%s", e.getMessage());
       }
     }
+
+    @Override
+    public Rule createAndAddRule(
+        Package.Builder packageBuilder,
+        StarlarkSemantics semantics,
+        Map<String, Object> kwargs,
+        EventHandler handler)
+        throws InterruptedException, InvalidRuleException, NameConflictException {
+      RuleClass ruleClass = builder.build(exportedName, exportedName);
+      BuildLangTypedAttributeValuesMap attributeValues =
+          new BuildLangTypedAttributeValuesMap(kwargs);
+      ImmutableList.Builder<CallStackEntry> callStack = ImmutableList.builder();
+      // TODO(pcloudy): Optimize the callstack
+      callStack.add(new CallStackEntry("RepositoryRuleFunction.createRule", Location.BUILTIN));
+      return RuleFactory.createAndAddRule(
+          packageBuilder, ruleClass, attributeValues, handler, semantics, callStack.build());
+    }
   }
 
   @Override
   public void failWithIncompatibleUseCcConfigureFromRulesCc(StarlarkThread thread)
       throws EvalException {
-    if (thread.getSemantics().incompatibleUseCcConfigureFromRulesCc()) {
+    if (thread
+        .getSemantics()
+        .getBool(BuildLanguageOptions.INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC)) {
       throw Starlark.errorf(
           "Incompatible flag "
               + "--incompatible_use_cc_configure_from_rules_cc has been flipped. Please use "
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index abf63f9e3e4..418d7019d6c 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -1,4 +1,4 @@
-// Copyright 2014 Google Inc. All rights reserved.
+// Copyright 2014 The Bazel Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,423 +15,278 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
-import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.common.flogger.GoogleLogger;
+import com.google.devtools.build.lib.actions.ThreadStateReceiver;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.concurrent.NamedForkJoinPool;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
-import com.google.devtools.build.lib.packages.GlobCache.BadGlobException;
-import com.google.devtools.build.lib.packages.License.DistributionType;
-import com.google.devtools.build.lib.packages.Type.ConversionException;
-import com.google.devtools.build.lib.syntax.AbstractFunction;
-import com.google.devtools.build.lib.syntax.AssignmentStatement;
-import com.google.devtools.build.lib.syntax.BuildFileAST;
-import com.google.devtools.build.lib.syntax.Environment;
-import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Expression;
-import com.google.devtools.build.lib.syntax.FuncallExpression;
-import com.google.devtools.build.lib.syntax.Function;
-import com.google.devtools.build.lib.syntax.GlobList;
-import com.google.devtools.build.lib.syntax.Ident;
-import com.google.devtools.build.lib.syntax.Label;
-import com.google.devtools.build.lib.syntax.MixedModeFunction;
-import com.google.devtools.build.lib.syntax.ParserInputSource;
-import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
-import com.google.devtools.build.lib.syntax.Statement;
-import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.packages.Globber.BadGlobException;
+import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
+import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
+import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.lib.vfs.UnixGlob;
-
-import java.io.IOException;
-import java.util.Collections;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.OptionalLong;
 import java.util.Set;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.logging.Logger;
-
-import javax.annotation.Nullable;
+import java.util.function.Consumer;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Mutability;
+import net.starlark.java.eval.NoneType;
+import net.starlark.java.eval.Printer;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkCallable;
+import net.starlark.java.eval.StarlarkFunction;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Argument;
+import net.starlark.java.syntax.CallExpression;
+import net.starlark.java.syntax.DefStatement;
+import net.starlark.java.syntax.Expression;
+import net.starlark.java.syntax.ForStatement;
+import net.starlark.java.syntax.Identifier;
+import net.starlark.java.syntax.IfStatement;
+import net.starlark.java.syntax.IntLiteral;
+import net.starlark.java.syntax.LambdaExpression;
+import net.starlark.java.syntax.ListExpression;
+import net.starlark.java.syntax.Location;
+import net.starlark.java.syntax.NodeVisitor;
+import net.starlark.java.syntax.Program;
+import net.starlark.java.syntax.StarlarkFile;
+import net.starlark.java.syntax.StringLiteral;
+import net.starlark.java.syntax.SyntaxError;
 
 /**
- * The package factory is responsible for constructing Package instances
- * from a BUILD file's abstract syntax tree (AST).
+ * The package factory is responsible for constructing Package instances from a BUILD file's
+ * abstract syntax tree (AST).
  *
- * <p>A PackageFactory is a heavy-weight object; create them sparingly.
- * Typically only one is needed per client application.
+ * <p>A PackageFactory is a heavy-weight object; create them sparingly. Typically only one is needed
+ * per client application.
  */
 public final class PackageFactory {
-  /**
-   * An argument to the {@code package()} function.
-   */
-  public abstract static class PackageArgument<T> {
-    private final String name;
-    private final Type<T> type;
+  private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
-    protected PackageArgument(String name, Type<T> type) {
-      this.name = name;
-      this.type = type;
-    }
-
-    public String getName() {
-      return name;
-    }
-
-    private void convertAndProcess(
-        Package.LegacyBuilder pkgBuilder, Location location, Object value)
-        throws EvalException, ConversionException {
-      T typedValue = type.convert(value, "'package' argument", pkgBuilder.getBuildFileLabel());
-      process(pkgBuilder, location, typedValue);
-    }
-
-    /**
-     * Process an argument.
-     *
-     * @param pkgBuilder the package builder to be mutated
-     * @param location the location of the {@code package} function for error reporting
-     * @param value the value of the argument. Typically passed to {@link Type#convert}
-     */
-    protected abstract void process(
-        Package.LegacyBuilder pkgBuilder, Location location, T value)
-        throws EvalException;
-  }
-
-  /** Interface for evaluating globs during package loading. */
-  public static interface Globber {
-    /** An opaque token for fetching the result of a glob computation. */
-    abstract static class Token {}
-
-    /**
-     * Asynchronously starts the given glob computation and returns a token for fetching the
-     * result.
-     */
-    Token runAsync(List<String> includes, List<String> excludes, boolean excludeDirs)
-        throws BadGlobException;
-
-    /** Fetches the result of a previously started glob computation. */
-    List<String> fetch(Token token) throws IOException, InterruptedException;
-
-    /** Should be called when the globber is about to be discarded due to an interrupt. */
-    void onInterrupt();
-
-    /** Should be called when the globber is no longer needed. */
-    void onCompletion();
-
-    /** Returns all the glob computations requested before {@link #onCompletion} was called. */
-    Set<Pair<String, Boolean>> getGlobPatterns();
-  }
-
-  /**
-   * An extension to the global namespace of the BUILD language.
-   */
+  /** An extension to the global namespace of the BUILD language. */
+  // TODO(bazel-team): this should probably be renamed PackageFactory.RuntimeExtension
+  //  since really we're extending the Runtime with more classes.
   public interface EnvironmentExtension {
-    /**
-     * Update the global environment with the identifiers this extension contributes.
-     */
-    void update(Environment environment, MakeEnvironment.Builder pkgMakeEnv,
-        Label buildFileLabel);
-
-    Iterable<PackageArgument<?>> getPackageArguments();
-  }
+    /** Update the predeclared environment with the identifiers this extension contributes. */
+    void update(ImmutableMap.Builder<String, Object> env);
 
-  private static final int EXCLUDE_DIR_DEFAULT = 1;
-
-  private static class DefaultVisibility extends PackageArgument<List<Label>> {
-    private DefaultVisibility() {
-      super("default_visibility", Type.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        List<Label> value) {
-      pkgBuilder.setDefaultVisibility(getVisibility(value));
-    }
-  }
+    /** Update the predeclared environment of WORKSPACE files. */
+    void updateWorkspace(ImmutableMap.Builder<String, Object> env);
 
-  private static class DefaultObsolete extends PackageArgument<Boolean> {
-    private DefaultObsolete() {
-      super("default_obsolete", Type.BOOLEAN);
-    }
+    /** Update the environment of the native module. */
+    void updateNative(ImmutableMap.Builder<String, Object> env);
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        Boolean value) {
-      pkgBuilder.setDefaultObsolete(value);
-    }
+    /** Returns the extra arguments to the {@code package()} statement. */
+    Iterable<PackageArgument<?>> getPackageArguments();
   }
 
-  private static class DefaultTestOnly extends PackageArgument<Boolean> {
-    private DefaultTestOnly() {
-      super("default_testonly", Type.BOOLEAN);
-    }
+  private final RuleFactory ruleFactory;
+  private final RuleClassProvider ruleClassProvider;
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        Boolean value) {
-      pkgBuilder.setDefaultTestonly(value);
-    }
-  }
+  private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
 
-  private static class DefaultDeprecation extends PackageArgument<String> {
-    private DefaultDeprecation() {
-      super("default_deprecation", Type.STRING);
-    }
+  private ForkJoinPool executor;
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        String value) {
-      pkgBuilder.setDefaultDeprecation(value);
-    }
-  }
+  private int maxDirectoriesToEagerlyVisitInGlobbing;
 
-  private static class Features extends PackageArgument<List<String>> {
-    private Features() {
-      super("features", Type.STRING_LIST);
-    }
+  private final ImmutableList<EnvironmentExtension> environmentExtensions;
+  private final ImmutableMap<String, PackageArgument<?>> packageArguments;
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        List<String> value) {
-      pkgBuilder.addFeatures(value);
-    }
-  }
+  private final PackageSettings packageSettings;
+  private final PackageValidator packageValidator;
+  private final PackageOverheadEstimator packageOverheadEstimator;
+  private final PackageLoadingListener packageLoadingListener;
 
-  private static class DefaultLicenses extends PackageArgument<License> {
-    private DefaultLicenses() {
-      super("licenses", Type.LICENSE);
-    }
+  private final BazelStarlarkEnvironment bazelStarlarkEnvironment;
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        License value) {
-      pkgBuilder.setDefaultLicense(value);
-    }
-  }
+  /** Builder for {@link PackageFactory} instances. Intended to only be used by unit tests. */
+  @VisibleForTesting
+  public abstract static class BuilderForTesting {
+    protected static final String VERSION = "test";
+    protected Iterable<EnvironmentExtension> environmentExtensions = ImmutableList.of();
+    protected PackageValidator packageValidator = PackageValidator.NOOP_VALIDATOR;
+    protected PackageOverheadEstimator packageOverheadEstimator =
+        PackageOverheadEstimator.NOOP_ESTIMATOR;
 
-  private static class DefaultDistribs extends PackageArgument<Set<DistributionType>> {
-    private DefaultDistribs() {
-      super("distribs", Type.DISTRIBUTIONS);
-    }
+    protected boolean doChecksForTesting = true;
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        Set<DistributionType> value) {
-      pkgBuilder.setDefaultDistribs(value);
+    public BuilderForTesting setEnvironmentExtensions(
+        Iterable<EnvironmentExtension> environmentExtensions) {
+      this.environmentExtensions = environmentExtensions;
+      return this;
     }
-  }
 
-  /**
-   * Declares the package() attribute specifying the default value for
-   * {@link RuleClass#COMPATIBLE_ENVIRONMENT_ATTR} when not explicitly specified.
-   */
-  private static class DefaultCompatibleWith extends PackageArgument<List<Label>> {
-    private DefaultCompatibleWith() {
-      super(Package.DEFAULT_COMPATIBLE_WITH_ATTRIBUTE, Type.LABEL_LIST);
+    public BuilderForTesting disableChecks() {
+      this.doChecksForTesting = false;
+      return this;
     }
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        List<Label> value) {
-      pkgBuilder.setDefaultCompatibleWith(value, Package.DEFAULT_COMPATIBLE_WITH_ATTRIBUTE,
-          location);
+    public BuilderForTesting setPackageValidator(PackageValidator packageValidator) {
+      this.packageValidator = packageValidator;
+      return this;
     }
-  }
 
-  /**
-   * Declares the package() attribute specifying the default value for
-   * {@link RuleClass#RESTRICTED_ENVIRONMENT_ATTR} when not explicitly specified.
-   */
-  private static class DefaultRestrictedTo extends PackageArgument<List<Label>> {
-    private DefaultRestrictedTo() {
-      super(Package.DEFAULT_RESTRICTED_TO_ATTRIBUTE, Type.LABEL_LIST);
+    public BuilderForTesting setPackageOverheadEstimator(
+        PackageOverheadEstimator packageOverheadEstimator) {
+      this.packageOverheadEstimator = packageOverheadEstimator;
+      return this;
     }
 
-    @Override
-    protected void process(Package.LegacyBuilder pkgBuilder, Location location,
-        List<Label> value) {
-      pkgBuilder.setDefaultRestrictedTo(value, Package.DEFAULT_RESTRICTED_TO_ATTRIBUTE, location);
-    }
-  }
-
-  public static final String PKG_CONTEXT = "$pkg_context";
-
-  /** {@link Globber} that uses the legacy GlobCache. */
-  public static class LegacyGlobber implements Globber {
-
-    private final GlobCache globCache;
-
-    public LegacyGlobber(GlobCache globCache) {
-      this.globCache = globCache;
-    }
-
-    private class Token extends Globber.Token {
-      public final List<String> includes;
-      public final List<String> excludes;
-      public final boolean excludeDirs;
-
-      public Token(List<String> includes, List<String> excludes, boolean excludeDirs) {
-        this.includes = includes;
-        this.excludes = excludes;
-        this.excludeDirs = excludeDirs;
-      }
-    }
-
-    @Override
-    public Set<Pair<String, Boolean>> getGlobPatterns() {
-      return globCache.getKeySet();
-    }
-
-    @Override
-    public Token runAsync(List<String> includes, List<String> excludes, boolean excludeDirs)
-        throws BadGlobException {
-      for (String pattern : Iterables.concat(includes, excludes)) {
-        globCache.getGlobAsync(pattern, excludeDirs);
-      }
-      return new Token(includes, excludes, excludeDirs);
-    }
-
-    @Override
-    public List<String> fetch(Globber.Token token) throws IOException, InterruptedException {
-      Token legacyToken = (Token) token;
-      try {
-        return globCache.glob(legacyToken.includes, legacyToken.excludes,
-            legacyToken.excludeDirs);
-      } catch (BadGlobException e) {
-        throw new IllegalStateException(e);
-      }
-    }
-
-    @Override
-    public void onInterrupt() {
-      globCache.cancelBackgroundTasks();
-    }
-
-    @Override
-    public void onCompletion() {
-      globCache.finishBackgroundTasks();
-    }
-  }
-
-  private static final Logger LOG = Logger.getLogger(PackageFactory.class.getName());
-
-  private final RuleFactory ruleFactory;
-  private final RuleClassProvider ruleClassProvider;
-  private final Environment globalEnv;
-
-  private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
-  private Preprocessor.Factory preprocessorFactory = Preprocessor.Factory.NullFactory.INSTANCE;
-
-  private final ThreadPoolExecutor threadPool;
-  private Map<String, String> platformSetRegexps;
-
-  private final ImmutableList<EnvironmentExtension> environmentExtensions;
-  private final ImmutableMap<String, PackageArgument<?>> packageArguments;
-
-  /**
-   * Constructs a {@code PackageFactory} instance with the given rule factory.
-   */
-  public PackageFactory(RuleClassProvider ruleClassProvider) {
-    this(ruleClassProvider, null, ImmutableList.<EnvironmentExtension>of());
+    public abstract PackageFactory build(RuleClassProvider ruleClassProvider, FileSystem fs);
   }
 
   @VisibleForTesting
-  public PackageFactory(RuleClassProvider ruleClassProvider,
-      EnvironmentExtension environmentExtensions) {
-    this(ruleClassProvider, null, ImmutableList.of(environmentExtensions));
+  public PackageSettings getPackageSettingsForTesting() {
+    return packageSettings;
   }
 
   /**
-   * Constructs a {@code PackageFactory} instance with a specific glob path translator
-   * and rule factory.
-   */
-  @VisibleForTesting
-  public PackageFactory(RuleClassProvider ruleClassProvider,
-      Map<String, String> platformSetRegexps,
-      Iterable<EnvironmentExtension> environmentExtensions) {
-    this.platformSetRegexps = platformSetRegexps;
+   * Constructs a {@code PackageFactory} instance with a specific glob path translator and rule
+   * factory.
+   *
+   * <p>Only intended to be called by BlazeRuntime or {@link BuilderForTesting#build}.
+   *
+   * <p>Do not call this constructor directly in tests; please use
+   * TestConstants#PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING instead.
+   */
+  // TODO(bazel-team): Maybe store `version` in the RuleClassProvider rather than passing it in
+  // here? It's an extra constructor parameter that all the tests have to give, and it's only needed
+  // so WorkspaceFactory can add an extra top-level builtin.
+  public PackageFactory(
+      RuleClassProvider ruleClassProvider,
+      ForkJoinPool executorForGlobbing,
+      Iterable<EnvironmentExtension> environmentExtensions,
+      String version,
+      PackageSettings packageSettings,
+      PackageValidator packageValidator,
+      PackageOverheadEstimator packageOverheadEstimator,
+      PackageLoadingListener packageLoadingListener) {
     this.ruleFactory = new RuleFactory(ruleClassProvider);
     this.ruleClassProvider = ruleClassProvider;
-    globalEnv = newGlobalEnvironment();
-    threadPool = new ThreadPoolExecutor(100, 100, 3L, TimeUnit.SECONDS,
-        new LinkedBlockingQueue<Runnable>(),
-        new ThreadFactoryBuilder().setNameFormat("PackageFactory %d").build());
-    // Do not consume threads when not in use.
-    threadPool.allowCoreThreadTimeOut(true);
+    this.executor = executorForGlobbing;
     this.environmentExtensions = ImmutableList.copyOf(environmentExtensions);
-    this.packageArguments = createPackageArguments();
+    this.packageArguments = createPackageArguments(this.environmentExtensions);
+    this.packageSettings = packageSettings;
+    this.packageValidator = packageValidator;
+    this.packageOverheadEstimator = packageOverheadEstimator;
+    this.packageLoadingListener = packageLoadingListener;
+    this.bazelStarlarkEnvironment =
+        new BazelStarlarkEnvironment(
+            ruleClassProvider,
+            buildRuleFunctions(ruleFactory),
+            this.environmentExtensions,
+            newPackageFunction(packageArguments),
+            version);
+  }
+
+  /** Sets the syscalls cache used in globbing. */
+  public void setSyscalls(AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls) {
+    this.syscalls = Preconditions.checkNotNull(syscalls);
   }
 
   /**
-   * Sets the preprocessor used.
+   * Sets the max number of threads to use for globbing.
+   *
+   * <p>Internally there is a {@link ForkJoinPool} used for globbing. If the specified {@code
+   * globbingThreads} does not match the previous value (initial value is 100), then we {@link
+   * ForkJoinPool#shutdown()} the old {@link ForkJoinPool} instance and make a new one.
    */
-  public void setPreprocessorFactory(Preprocessor.Factory preprocessorFactory) {
-    this.preprocessorFactory = preprocessorFactory;
+  public void setGlobbingThreads(int globbingThreads) {
+    if (executor == null) {
+      executor = makeForkJoinPool(globbingThreads);
+      return;
+    }
+    if (executor.getParallelism() == globbingThreads) {
+      return;
+    }
+    // We don't use ForkJoinPool#shutdownNow since it has a performance bug. See
+    // http://b/33482341#comment13.
+    executor.shutdown();
+    executor = makeForkJoinPool(globbingThreads);
   }
 
- /**
-   * Sets the syscalls cache used in globbing.
-   */
-  public void setSyscalls(AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls) {
-    this.syscalls = Preconditions.checkNotNull(syscalls);
+  public static ForkJoinPool makeDefaultSizedForkJoinPoolForGlobbing() {
+    return makeForkJoinPool(/*globbingThreads=*/ 100);
   }
 
-  /**
-   * Returns the static environment initialized once and shared by all packages
-   * created by this factory. No updates occur to this environment once created.
-   */
-  @VisibleForTesting
-  public Environment getEnvironment() {
-    return globalEnv;
+  private static ForkJoinPool makeForkJoinPool(int globbingThreads) {
+    return NamedForkJoinPool.newNamedPool("globbing pool", globbingThreads);
   }
 
   /**
-   * Returns the immutable, unordered set of names of all the known rule
-   * classes.
+   * Sets the number of directories to eagerly traverse on the first glob for a given package, in
+   * order to warm the filesystem. -1 means do no eager traversal. See {@link
+   * com.google.devtools.build.lib.pkgcache.PackageOptions#maxDirectoriesToEagerlyVisitInGlobbing}.
+   * -2 means do the eager traversal using the regular globbing infrastructure, i.e. sharing the
+   * globbing threads and caching the actual glob results.
    */
+  public void setMaxDirectoriesToEagerlyVisitInGlobbing(
+      int maxDirectoriesToEagerlyVisitInGlobbing) {
+    this.maxDirectoriesToEagerlyVisitInGlobbing = maxDirectoriesToEagerlyVisitInGlobbing;
+  }
+
+  /** Returns the immutable, unordered set of names of all the known rule classes. */
   public Set<String> getRuleClassNames() {
     return ruleFactory.getRuleClassNames();
   }
 
   /**
-   * Returns the {@link RuleClass} for the specified rule class name.
+   * Returns the {@link com.google.devtools.build.lib.packages.RuleClass} for the specified rule
+   * class name.
    */
   public RuleClass getRuleClass(String ruleClassName) {
     return ruleFactory.getRuleClass(ruleClassName);
   }
 
-  /**
-   * Returns the {@link RuleClassProvider} of this {@link PackageFactory}.
-   */
+  /** Returns the {@link RuleClassProvider} of this {@link PackageFactory}. */
   public RuleClassProvider getRuleClassProvider() {
     return ruleClassProvider;
   }
 
-  /**
-   * Creates the list of arguments for the 'package' function.
-   */
-  private ImmutableMap<String, PackageArgument<?>> createPackageArguments() {
+  public ImmutableList<EnvironmentExtension> getEnvironmentExtensions() {
+    return environmentExtensions;
+  }
+
+  public BazelStarlarkEnvironment getBazelStarlarkEnvironment() {
+    return bazelStarlarkEnvironment;
+  }
+
+  /** Creates the map of arguments for the 'package' function. */
+  private static ImmutableMap<String, PackageArgument<?>> createPackageArguments(
+      List<EnvironmentExtension> environmentExtensions) {
     ImmutableList.Builder<PackageArgument<?>> arguments =
-        ImmutableList.<PackageArgument<?>>builder()
-           .add(new DefaultDeprecation())
-           .add(new DefaultDistribs())
-           .add(new DefaultLicenses())
-           .add(new DefaultObsolete())
-           .add(new DefaultTestOnly())
-           .add(new DefaultVisibility())
-           .add(new Features())
-           .add(new DefaultCompatibleWith())
-           .add(new DefaultRestrictedTo());
+        ImmutableList.<PackageArgument<?>>builder().addAll(DefaultPackageArguments.get());
 
     for (EnvironmentExtension extension : environmentExtensions) {
       arguments.addAll(extension.getPackageArguments());
@@ -444,507 +299,329 @@ private ImmutableMap<String, PackageArgument<?>> createPackageArguments() {
     return packageArguments.build();
   }
 
-  /****************************************************************************
-   * Environment function factories.
-   */
-
-  /**
-   * Returns a function-value implementing "glob" in the specified package
-   * context.
-   *
-   * @param async if true, start globs in the background but don't block on their completion.
-   *        Only use this for heuristic preloading.
-   */
-  private static Function newGlobFunction(
-      final PackageContext originalContext, final boolean async) {
-    List<String> params = ImmutableList.of("include", "exclude", "exclude_directories");
-    return new MixedModeFunction("glob", params, 1, false) {
-        @Override
-        public Object call(Object[] namedArguments, FuncallExpression ast, Environment env)
-                throws EvalException, ConversionException, InterruptedException {
-
-          // Skylark build extensions need to get the PackageContext from the Environment;
-          // async glob functions cannot do the same because the Environment is not thread safe.
-          PackageContext context;
-          if (originalContext == null) {
-            Preconditions.checkArgument(!async);
-            try {
-              context = (PackageContext) env.lookup(PKG_CONTEXT);
-            } catch (NoSuchVariableException e) {
-              throw new EvalException(ast.getLocation(), e.getMessage());
-            }
-          } else {
-            context = originalContext;
-          }
+  /** Returns a function-value implementing "package" in the specified package context. */
+  // TODO(cparsons): Migrate this function to be defined with @StarlarkMethod.
+  // TODO(adonovan): don't call this function twice (once for BUILD files and
+  // once for the native module) as it results in distinct objects. (Using
+  // @StarlarkMethod may accomplish that.)
+  private static StarlarkCallable newPackageFunction(
+      final Map<String, PackageArgument<?>> packageArguments) {
+    return new StarlarkCallable() {
+      @Override
+      public String getName() {
+        return "package";
+      }
 
-          List<String> includes = Type.STRING_LIST.convert(namedArguments[0], "'glob' argument");
-          List<String> excludes = namedArguments[1] == null
-              ? Collections.<String>emptyList()
-              : Type.STRING_LIST.convert(namedArguments[1], "'glob' argument");
-          int excludeDirs = namedArguments[2] == null
-            ? EXCLUDE_DIR_DEFAULT
-            : Type.INTEGER.convert(namedArguments[2], "'glob' argument");
-
-          if (async) {
-            try {
-              context.globber.runAsync(includes, excludes, excludeDirs != 0);
-            } catch (GlobCache.BadGlobException e) {
-              // Ignore: errors will appear during the actual evaluation of the package.
-            }
-            return GlobList.captureResults(includes, excludes, ImmutableList.<String>of());
-          } else {
-            return handleGlob(includes, excludes, excludeDirs != 0, context, ast);
-          }
-        }
-      };
-  }
+      @Override
+      public String toString() {
+        return "package(...)";
+      }
 
-  /**
-   * Adds a glob to the package, reporting any errors it finds.
-   *
-   * @param includes the list of includes which must be non-null
-   * @param excludes the list of excludes which must be non-null
-   * @param context the package context
-   * @param ast the AST
-   * @return the list of matches
-   * @throws EvalException if globbing failed
-   */
-  private static GlobList<String> handleGlob(List<String> includes, List<String> excludes,
-      boolean excludeDirs, PackageContext context, FuncallExpression ast)
-        throws EvalException, InterruptedException {
-    try {
-      Globber.Token globToken = context.globber.runAsync(includes, excludes, excludeDirs);
-      List<String> matches = context.globber.fetch(globToken);
-      return GlobList.captureResults(includes, excludes, matches);
-    } catch (IOException expected) {
-      context.eventHandler.handle(Event.error(ast.getLocation(),
-              "error globbing [" + Joiner.on(", ").join(includes) + "]: " + expected.getMessage()));
-      context.pkgBuilder.setContainsTemporaryErrors();
-      return GlobList.captureResults(includes, excludes, ImmutableList.<String>of());
-    } catch (GlobCache.BadGlobException e) {
-      throw new EvalException(ast.getLocation(), e.getMessage());
-    }
-  }
+      @Override
+      public boolean isImmutable() {
+        return true;
+      }
 
-  /**
-   * Returns a function value implementing the "mocksubinclude" function,
-   * emitted by the PythonPreprocessor.  We annotate the
-   * package with additional dependencies.  (A 'real' subinclude will never be
-   * seen by the parser, because the presence of "subinclude" triggers
-   * preprocessing.)
-   */
-  private static Function newMockSubincludeFunction(final PackageContext context) {
-    return new MixedModeFunction("mocksubinclude", ImmutableList.of("label", "path"), 2, false) {
-        @Override
-        public Object call(Object[] args, FuncallExpression ast)
-            throws ConversionException {
-          Label label = Type.LABEL.convert(args[0], "'mocksubinclude' argument",
-                                           context.pkgBuilder.getBuildFileLabel());
-          String pathString = Type.STRING.convert(args[1], "'mocksubinclude' argument");
-          Path path = pathString.isEmpty()
-              ? null
-              : context.pkgBuilder.getFilename().getRelative(pathString);
-          // A subinclude within a package counts as a file declaration.
-          if (label.getPackageIdentifier().equals(context.pkgBuilder.getPackageIdentifier())) {
-            Location location = ast.getLocation();
-            if (location == null) {
-              location = Location.fromFile(context.pkgBuilder.getFilename());
-            }
-            context.pkgBuilder.createInputFileMaybe(label, location);
-          }
+      @Override
+      public void repr(Printer printer) {
+        printer.append("<built-in function package>");
+      }
 
-          context.pkgBuilder.addSubinclude(label, path);
-          return Environment.NONE;
+      @Override
+      public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
+          throws EvalException {
+        if (!args.isEmpty()) {
+          throw new EvalException("unexpected positional arguments");
         }
-      };
-  }
+        Package.Builder pkgBuilder = getContext(thread).pkgBuilder;
 
-  /**
-   * Fake function: subinclude calls are ignored
-   * They will disappear after the Python preprocessing.
-   */
-  private static Function newSubincludeFunction() {
-    return new MixedModeFunction("subinclude", ImmutableList.of("file"), 1, false) {
-        @Override
-        public Object call(Object[] args, FuncallExpression ast) {
-          return Environment.NONE;
+        // Validate parameter list
+        if (pkgBuilder.isPackageFunctionUsed()) {
+          throw new EvalException("'package' can only be used once per BUILD file");
         }
-      };
-  }
+        pkgBuilder.setPackageFunctionUsed();
 
-  /**
-   * Returns a function value implementing "environment_group" in the specified package context.
-   * Syntax is as follows:
-   *
-   * <pre>{@code
-   *   environment_group(
-   *       name = "sample_group",
-   *       environments = [":env1", ":env2", ...],
-   *       defaults = [":env1", ...]
-   *   )
-   * }</pre>
-   *
-   * <p>Where ":env1", "env2", ... are all environment rules declared in the same package. All
-   * parameters are mandatory.
-   */
-  private static Function newEnvironmentGroupFunction(final PackageContext context) {
-    List<String> params = ImmutableList.of("name", "environments", "defaults");
-    return new MixedModeFunction("environment_group", params, params.size(), true) {
-        @Override
-        public Object call(Object[] namedArgs, FuncallExpression ast)
-            throws EvalException, ConversionException {
-          Preconditions.checkState(namedArgs[0] != null);
-          String name = Type.STRING.convert(namedArgs[0], "'environment_group' argument");
-          Preconditions.checkState(namedArgs[1] != null);
-          List<Label> environments = Type.LABEL_LIST.convert(
-              namedArgs[1], "'environment_group argument'", context.pkgBuilder.getBuildFileLabel());
-          Preconditions.checkState(namedArgs[2] != null);
-          List<Label> defaults = Type.LABEL_LIST.convert(
-              namedArgs[2], "'environment_group argument'", context.pkgBuilder.getBuildFileLabel());
-
-          try {
-            context.pkgBuilder.addEnvironmentGroup(name, environments, defaults,
-                context.eventHandler, ast.getLocation());
-            return Environment.NONE;
-          } catch (Label.SyntaxException e) {
-            throw new EvalException(ast.getLocation(),
-                "environment group has invalid name: " + name + ": " + e.getMessage());
-          } catch (Package.NameConflictException e) {
-            throw new EvalException(ast.getLocation(), e.getMessage());
-          }
+        // Each supplied argument must name a PackageArgument.
+        if (kwargs.isEmpty()) {
+          throw new EvalException("at least one argument must be given to the 'package' function");
         }
-      };
-  }
-
-  /**
-   * Returns a function-value implementing "exports_files" in the specified
-   * package context.
-   */
-  private static Function newExportsFilesFunction(final PackageContext context) {
-    final Package.LegacyBuilder pkgBuilder = context.pkgBuilder;
-    List<String> params = ImmutableList.of("srcs", "visibility", "licenses");
-    return new MixedModeFunction("exports_files", params, 1, false) {
-      @Override
-      public Object call(Object[] namedArgs, FuncallExpression ast)
-          throws EvalException, ConversionException {
-
-        List<String> files = Type.STRING_LIST.convert(namedArgs[0], "'exports_files' operand");
-
-        RuleVisibility visibility = namedArgs[1] == null
-            ? ConstantRuleVisibility.PUBLIC
-            : getVisibility(Type.LABEL_LIST.convert(
-                namedArgs[1],
-                "'exports_files' operand",
-                pkgBuilder.getBuildFileLabel()));
-        License license = namedArgs[2] == null
-            ? null
-            : Type.LICENSE.convert(namedArgs[2], "'exports_files' operand");
-
-        for (String file : files) {
-          String errorMessage = LabelValidator.validateTargetName(file);
-          if (errorMessage != null) {
-            throw new EvalException(ast.getLocation(), errorMessage);
-          }
-          try {
-            InputFile inputFile = pkgBuilder.createInputFile(file, ast.getLocation());
-            if (inputFile.isVisibilitySpecified()
-                && inputFile.getVisibility() != visibility) {
-              throw new EvalException(ast.getLocation(),
-                  String.format("visibility for exported file '%s' declared twice",
-                      inputFile.getName()));
-            }
-            if (license != null && inputFile.isLicenseSpecified()) {
-              throw new EvalException(ast.getLocation(),
-                  String.format("licenses for exported file '%s' declared twice",
-                      inputFile.getName()));
-            }
-            if (license == null && pkgBuilder.getDefaultLicense() == License.NO_LICENSE
-                && pkgBuilder.getBuildFileLabel().toString().startsWith("//third_party/")) {
-              throw new EvalException(ast.getLocation(),
-                  "third-party file '" + inputFile.getName() + "' lacks a license declaration "
-                  + "with one of the following types: notice, reciprocal, permissive, "
-                  + "restricted, unencumbered, by_exception_only");
-            }
-
-            pkgBuilder.setVisibilityAndLicense(inputFile, visibility, license);
-          } catch (Package.Builder.GeneratedLabelConflict e) {
-            throw new EvalException(ast.getLocation(), e.getMessage());
+        Location loc = thread.getCallerLocation();
+        for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
+          String name = kwarg.getKey();
+          PackageArgument<?> pkgarg = packageArguments.get(name);
+          if (pkgarg == null) {
+            throw Starlark.errorf("unexpected keyword argument: %s", name);
           }
+          pkgarg.convertAndProcess(pkgBuilder, loc, kwarg.getValue());
         }
-        return Environment.NONE;
+        return Starlark.NONE;
       }
     };
   }
 
-  /**
-   * Returns a function-value implementing "licenses" in the specified package
-   * context.
-   * TODO(bazel-team): Remove in favor of package.licenses.
-   */
-  private static Function newLicensesFunction(final PackageContext context) {
-    return new MixedModeFunction("licenses", ImmutableList.of("object"), 1, false) {
-        @Override
-        public Object call(Object[] args, FuncallExpression ast) {
-          try {
-            License license = Type.LICENSE.convert(args[0], "'licenses' operand");
-            context.pkgBuilder.setDefaultLicense(license);
-          } catch (ConversionException e) {
-            context.eventHandler.handle(Event.error(ast.getLocation(), e.getMessage()));
-            context.pkgBuilder.setContainsErrors();
-          }
-          return Environment.NONE;
-        }
-      };
-  }
-
-  /**
-   * Returns a function-value implementing "distribs" in the specified package
-   * context.
-   * TODO(bazel-team): Remove in favor of package.distribs.
-   */
-  private static Function newDistribsFunction(final PackageContext context) {
-    return new MixedModeFunction("distribs", ImmutableList.of("object"), 1, false) {
-        @Override
-        public Object call(Object[] args, FuncallExpression ast) {
-          try {
-            Set<DistributionType> distribs = Type.DISTRIBUTIONS.convert(args[0],
-                "'distribs' operand");
-            context.pkgBuilder.setDefaultDistribs(distribs);
-          } catch (ConversionException e) {
-            context.eventHandler.handle(Event.error(ast.getLocation(), e.getMessage()));
-            context.pkgBuilder.setContainsErrors();
-          }
-          return Environment.NONE;
-        }
-      };
-  }
-
-  private static Function newPackageGroupFunction(final PackageContext context) {
-    List<String> params = ImmutableList.of("name", "packages", "includes");
-    return new MixedModeFunction("package_group", params, 1, true) {
-        @Override
-        public Object call(Object[] namedArgs, FuncallExpression ast)
-            throws EvalException, ConversionException {
-          Preconditions.checkState(namedArgs[0] != null);
-          String name = Type.STRING.convert(namedArgs[0], "'package_group' argument");
-          List<String> packages = namedArgs[1] == null
-              ? Collections.<String>emptyList()
-              : Type.STRING_LIST.convert(namedArgs[1], "'package_group' argument");
-          List<Label> includes = namedArgs[2] == null
-              ? Collections.<Label>emptyList()
-              : Type.LABEL_LIST.convert(namedArgs[2], "'package_group argument'",
-                                        context.pkgBuilder.getBuildFileLabel());
-
-          try {
-            context.pkgBuilder.addPackageGroup(name, packages, includes, context.eventHandler,
-                ast.getLocation());
-            return Environment.NONE;
-          } catch (Label.SyntaxException e) {
-            throw new EvalException(ast.getLocation(),
-                "package group has invalid name: " + name + ": " + e.getMessage());
-          } catch (Package.NameConflictException e) {
-            throw new EvalException(ast.getLocation(), e.getMessage());
-          }
-        }
-      };
+  /** Get the PackageContext by looking up in the environment. */
+  public static PackageContext getContext(StarlarkThread thread) throws EvalException {
+    PackageContext value = thread.getThreadLocal(PackageContext.class);
+    if (value == null) {
+      // if PackageContext is missing, we're not called from a BUILD file. This happens if someone
+      // uses native.some_func() in the wrong place.
+      throw Starlark.errorf(
+          "The native module can be accessed only from a BUILD thread. "
+              + "Wrap the function in a macro and call it from a BUILD file");
+    }
+    return value;
   }
 
-  public static RuleVisibility getVisibility(List<Label> original) {
-    RuleVisibility result;
-
-    result = ConstantRuleVisibility.tryParse(original);
-    if (result != null) {
-      return result;
+  private static ImmutableMap<String, BuiltinRuleFunction> buildRuleFunctions(
+      RuleFactory ruleFactory) {
+    ImmutableMap.Builder<String, BuiltinRuleFunction> result = ImmutableMap.builder();
+    for (String ruleClassName : ruleFactory.getRuleClassNames()) {
+      RuleClass cl = ruleFactory.getRuleClass(ruleClassName);
+      if (cl.getRuleClassType() == RuleClassType.NORMAL
+          || cl.getRuleClassType() == RuleClassType.TEST) {
+        result.put(ruleClassName, new BuiltinRuleFunction(cl));
+      }
     }
-
-    result = PackageGroupsRuleVisibility.tryParse(original);
-    return result;
+    return result.build();
   }
 
-  /**
-   * Returns a function-value implementing "package" in the specified package
-   * context.
-   */
-  private static Function newPackageFunction(
-      final Map<String, PackageArgument<?>> packageArguments) {
-    return new MixedModeFunction("package", packageArguments.keySet(), 0, true) {
-      @Override
-      public Object call(Object[] namedArguments, FuncallExpression ast, Environment env)
-          throws EvalException, ConversionException {
+  /** A callable Starlark value that creates Rules for native RuleClasses. */
+  // TODO(adonovan): why is this distinct from RuleClass itself?
+  // Make RuleClass implement StarlarkCallable directly.
+  private static class BuiltinRuleFunction implements RuleFunction {
+    private final RuleClass ruleClass;
 
-        Package.LegacyBuilder pkgBuilder = getContext(env, ast).pkgBuilder;
+    BuiltinRuleFunction(RuleClass ruleClass) {
+      this.ruleClass = Preconditions.checkNotNull(ruleClass);
+    }
 
-        // Validate parameter list
-        if (pkgBuilder.isPackageFunctionUsed()) {
-          throw new EvalException(ast.getLocation(),
-              "'package' can only be used once per BUILD file");
-        }
-        pkgBuilder.setPackageFunctionUsed();
+    @Override
+    public NoneType call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
+        throws EvalException, InterruptedException {
+      if (!args.isEmpty()) {
+        throw Starlark.errorf("unexpected positional arguments");
+      }
+      BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase(ruleClass.getName());
+      try {
+        PackageContext context = getContext(thread);
+        RuleFactory.createAndAddRule(
+            context.pkgBuilder,
+            ruleClass,
+            new BuildLangTypedAttributeValuesMap(kwargs),
+            context.eventHandler,
+            thread.getSemantics(),
+            thread.getCallStack());
+      } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
+        throw new EvalException(e);
+      }
+      return Starlark.NONE;
+    }
 
-        // Parse params
-        boolean foundParameter = false;
+    @Override
+    public RuleClass getRuleClass() {
+      return ruleClass;
+    }
 
-        int argNumber = 0;
-        for (Map.Entry<String, PackageArgument<?>> entry : packageArguments.entrySet()) {
-          Object arg = namedArguments[argNumber];
-          argNumber += 1;
-          if (arg == null) {
-            continue;
-          }
+    @Override
+    public String getName() {
+      return ruleClass.getName();
+    }
 
-          foundParameter = true;
-          entry.getValue().convertAndProcess(pkgBuilder, ast.getLocation(), arg);
-        }
+    @Override
+    public void repr(Printer printer) {
+      printer.append("<built-in rule " + getName() + ">");
+    }
 
-        if (!foundParameter) {
-          throw new EvalException(ast.getLocation(),
-              "at least one argument must be given to the 'package' function");
-        }
+    @Override
+    public String toString() {
+      return getName() + "(...)";
+    }
 
-        return Environment.NONE;
-      }
-    };
+    @Override
+    public boolean isImmutable() {
+      return true;
+    }
   }
 
-  // Helper function for createRuleFunction.
-  private static void addRule(RuleFactory ruleFactory,
-                              String ruleClassName,
-                              PackageContext context,
-                              Map<String, Object> kwargs,
-                              FuncallExpression ast)
-      throws RuleFactory.InvalidRuleException, Package.NameConflictException {
-    RuleClass ruleClass = getBuiltInRuleClass(ruleClassName, ruleFactory);
-    RuleFactory.createAndAddRule(context, ruleClass, kwargs, ast);
+  @VisibleForTesting // exposed to WorkspaceFileFunction and BzlmodRepoRuleFunction
+  public Package.Builder newExternalPackageBuilder(
+      RootedPath workspacePath, String workspaceName, StarlarkSemantics starlarkSemantics) {
+    return Package.newExternalPackageBuilder(
+        packageSettings, workspacePath, workspaceName, starlarkSemantics);
   }
 
-  private static RuleClass getBuiltInRuleClass(String ruleClassName, RuleFactory ruleFactory) {
-    if (ruleFactory.getRuleClassNames().contains(ruleClassName)) {
-      return ruleFactory.getRuleClass(ruleClassName);
+  // This function is public only for the benefit of skyframe.PackageFunction,
+  // which is morally part of lib.packages, so that it can create empty packages
+  // in case of error before BUILD execution. Do not call it from anywhere else.
+  // TODO(adonovan): refactor Rule{Class,Factory}Test not to need this.
+  public Package.Builder newPackageBuilder(
+      PackageIdentifier packageId,
+      String workspaceName,
+      StarlarkSemantics starlarkSemantics,
+      ImmutableMap<RepositoryName, RepositoryName> repositoryMapping) {
+    return new Package.Builder(
+        packageSettings,
+        packageId,
+        workspaceName,
+        starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
+        repositoryMapping);
+  }
+
+  /** Returns a new {@link NonSkyframeGlobber}. */
+  // Exposed to skyframe.PackageFunction.
+  public NonSkyframeGlobber createNonSkyframeGlobber(
+      Path packageDirectory,
+      PackageIdentifier packageId,
+      ImmutableSet<PathFragment> ignoredGlobPrefixes,
+      CachingPackageLocator locator,
+      ThreadStateReceiver threadStateReceiverForMetrics) {
+    return new NonSkyframeGlobber(
+        new GlobCache(
+            packageDirectory,
+            packageId,
+            ignoredGlobPrefixes,
+            locator,
+            syscalls,
+            executor,
+            maxDirectoriesToEagerlyVisitInGlobbing,
+            threadStateReceiverForMetrics));
+  }
+
+  /**
+   * This class holds state associated with the construction of a single package for the duration of
+   * execution of one BUILD file. (We use a PackageContext object in preference to storing these
+   * values in mutable fields of the PackageFactory.)
+   *
+   * <p>PLEASE NOTE: the PackageContext is referred to by the StarlarkThread, but should become
+   * unreachable once the StarlarkThread is discarded at the end of evaluation. Please be aware of
+   * your memory footprint when making changes here!
+   */
+  // TODO(adonovan): is there any reason not to merge this with Package.Builder?
+  public static class PackageContext {
+    final Package.Builder pkgBuilder;
+    final Globber globber;
+    final ExtendedEventHandler eventHandler;
+
+    @VisibleForTesting
+    public PackageContext(
+        Package.Builder pkgBuilder, Globber globber, ExtendedEventHandler eventHandler) {
+      this.pkgBuilder = pkgBuilder;
+      this.eventHandler = eventHandler;
+      this.globber = globber;
     }
-    throw new IllegalArgumentException("no such rule class: "  + ruleClassName);
-  }
 
-  /**
-   * Get the PackageContext by looking up in the environment.
-   */
-  private static PackageContext getContext(Environment env, FuncallExpression ast)
-      throws EvalException {
-    try {
-      return (PackageContext) env.lookup(PKG_CONTEXT);
-    } catch (NoSuchVariableException e) {
-      throw new EvalException(ast.getLocation(), e.getMessage());
+    /** Returns the Label of this Package's BUILD file. */
+    public Label getLabel() {
+      return pkgBuilder.getBuildFileLabel();
     }
-  }
 
-  /**
-   * Returns a function-value implementing the build rule "ruleClass" (e.g. cc_library) in the
-   * specified package context.
-   */
-  private static Function newRuleFunction(final RuleFactory ruleFactory,
-                                          final String ruleClass) {
-    return new AbstractFunction(ruleClass) {
-      @Override
-      public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast,
-          Environment env)
-          throws EvalException {
-        if (!args.isEmpty()) {
-          throw new EvalException(ast.getLocation(),
-              "build rules do not accept positional parameters");
-        }
+    /** Sets a Make variable. */
+    public void setMakeVariable(String name, String value) {
+      pkgBuilder.setMakeVariable(name, value);
+    }
 
-        try {
-          addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast);
-        } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
-          throw new EvalException(ast.getLocation(), e.getMessage());
-        }
-        return Environment.NONE;
-      }
-    };
-  }
+    /** Returns the builder of this Package. */
+    public Package.Builder getBuilder() {
+      return pkgBuilder;
+    }
 
-  /**
-   * Returns a new environment populated with common entries that can be shared
-   * across packages and that don't require the context.
-   */
-  private static Environment newGlobalEnvironment() {
-    Environment env = new Environment();
-    MethodLibrary.setupMethodEnvironment(env);
-    return env;
+    /**
+     * Returns the event handler that should be used to report events happening while building this
+     * package.
+     */
+    public ExtendedEventHandler getEventHandler() {
+      return eventHandler;
+    }
   }
 
-  /****************************************************************************
-   * Package creation.
-   */
-
   /**
-   * Loads, scans parses and evaluates the build file at "buildFile", and
-   * creates and returns a Package builder instance capable of building a package identified by
-   * "packageId".
+   * Runs final validation and administrative tasks on newly loaded package. Called by a caller of
+   * {@link #executeBuildFile} after this caller has fully loaded the package.
    *
-   * <p>This method returns a builder to allow the caller to do additional work, if necessary.
+   * @throws InvalidPackageException if the package is determined to be invalid
+   */
+  public void afterDoneLoadingPackage(
+      Package pkg,
+      StarlarkSemantics starlarkSemantics,
+      long loadTimeNanos,
+      ExtendedEventHandler eventHandler)
+      throws InvalidPackageException {
+    OptionalLong packageOverhead = packageOverheadEstimator.estimatePackageOverhead(pkg);
+
+    packageValidator.validate(pkg, packageOverhead, eventHandler);
+
+    // Enforce limit on number of compute steps in BUILD file (b/151622307).
+    long maxSteps = starlarkSemantics.get(BuildLanguageOptions.MAX_COMPUTATION_STEPS);
+    long steps = pkg.getComputationSteps();
+    if (maxSteps > 0 && steps > maxSteps) {
+      String message =
+          String.format(
+              "BUILD file computation took %d steps, but --max_computation_steps=%d",
+              steps, maxSteps);
+      throw new InvalidPackageException(
+          pkg.getPackageIdentifier(),
+          message,
+          DetailedExitCode.of(
+              FailureDetail.newBuilder()
+                  .setMessage(message)
+                  .setPackageLoading(
+                      PackageLoading.newBuilder()
+                          .setCode(PackageLoading.Code.MAX_COMPUTATION_STEPS_EXCEEDED)
+                          .build())
+                  .build()));
+    }
+
+    packageLoadingListener.onLoadingCompleteAndSuccessful(
+        pkg, starlarkSemantics, loadTimeNanos, packageOverhead);
+  }
+
+  /**
+   * Populates the Package.Builder by executing the specified BUILD file.
    *
-   * <p>This method assumes "packageId" is a valid package name according to the
-   * {@link LabelValidator#validatePackageName} heuristic.
+   * <p>The package exists---we have parsed its BUILD file---but it may contain errors, either
+   * arising from Starlark evaluation (such as an array index error, or a call to a built-in
+   * function that fails), or reported as a side effect of a built-in function, such as rule
+   * instantiation, that returns normally. A partial package is nonetheless returned in both cases,
+   * although it may have fewer rules than expected.
    *
-   * <p>See {@link #evaluateBuildFile} for information on AST retention.
+   * <p>TODO(adonovan): do not return a partial package in case of BUILD evaluation errors. Errors
+   * during .bzl execution are already fatal.
    *
-   * <p>Executes {@code globber.onCompletion()} on completion and executes
-   * {@code globber.onInterrupt()} on an {@link InterruptedException}.
-   */
-  private Package.LegacyBuilder createPackage(PackageIdentifier packageId, Path buildFile,
-      List<Statement> preludeStatements, ParserInputSource inputSource,
-      Map<PathFragment, SkylarkEnvironment> imports, ImmutableList<Label> skylarkFileDependencies,
-      CachingPackageLocator locator, RuleVisibility defaultVisibility, Globber globber)
-          throws InterruptedException {
-    StoredEventHandler localReporter = new StoredEventHandler();
-    Preprocessor.Result preprocessingResult = preprocess(packageId, buildFile, inputSource, globber,
-        localReporter);
-    return createPackageFromPreprocessingResult(packageId, buildFile, preprocessingResult,
-        localReporter.getEvents(), preludeStatements, imports, skylarkFileDependencies, locator,
-        defaultVisibility, globber);
-  }
-
-  /**
-   * Same as {@link #createPackage}, but using a {@link Preprocessor.Result} from
-   * {@link #preprocess}.
+   * <p><b>Do not call it from elsewhere! It is not in any meaningful sense a public API.</b><br>
+   * In tests, use BuildViewTestCase or PackageLoadingTestCase instead.
    *
-   * <p>Executes {@code globber.onCompletion()} on completion and executes
-   * {@code globber.onInterrupt()} on an {@link InterruptedException}.
-   */
-  // Used outside of bazel!
-  public Package.LegacyBuilder createPackageFromPreprocessingResult(PackageIdentifier packageId,
-      Path buildFile,
-      Preprocessor.Result preprocessingResult,
-      Iterable<Event> preprocessingEvents,
-      List<Statement> preludeStatements,
-      Map<PathFragment, SkylarkEnvironment> imports,
-      ImmutableList<Label> skylarkFileDependencies,
-      CachingPackageLocator locator,
-      RuleVisibility defaultVisibility,
-      Globber globber) throws InterruptedException {
-    StoredEventHandler localReporter = new StoredEventHandler();
-    // Run the lexer and parser with a local reporter, so that errors from other threads do not
-    // show up below. Merge the local and global reporters afterwards.
-    // Logged messages are used as a testability hook tracing the parsing progress
-    LOG.fine("Starting to parse " + packageId);
-    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(
-        preprocessingResult.result, preludeStatements, localReporter, locator, false);
-    LOG.fine("Finished parsing of " + packageId);
-
-    MakeEnvironment.Builder makeEnv = new MakeEnvironment.Builder();
-    if (platformSetRegexps != null) {
-      makeEnv.setPlatformSetRegexps(platformSetRegexps);
+   * <p>TODO(adonovan): move PackageFunction into this package and develop a rational API.
+   */
+  // This function is the sole entry point for package creation in production and tests. Do not add
+  // others! It changes often, and is exposed only for the benefit of skyframe.PackageFunction,
+  // which is logically part of the loading phase and should in due course be moved to lib.packages,
+  // but that cannot happen until Skyframe's core interfaces have been separated.
+  public void executeBuildFile(
+      Package.Builder pkgBuilder,
+      Program buildFileProgram,
+      ImmutableList<String> globs,
+      ImmutableList<String> globsWithDirs,
+      ImmutableMap<String, Object> predeclared,
+      ImmutableMap<String, Module> loadedModules,
+      StarlarkSemantics starlarkSemantics,
+      Globber globber)
+      throws InterruptedException {
+    // Prefetch glob patterns asynchronously.
+    if (maxDirectoriesToEagerlyVisitInGlobbing == -2) {
+      try {
+        boolean allowEmpty = true;
+        globber.runAsync(globs, ImmutableList.of(), /*excludeDirs=*/ true, allowEmpty);
+        globber.runAsync(globsWithDirs, ImmutableList.of(), /*excludeDirs=*/ false, allowEmpty);
+      } catch (BadGlobException ex) {
+        logger.atWarning().withCause(ex).log(
+            "Suppressing exception for globs=%s, globsWithDirs=%s", globs, globsWithDirs);
+        // Ignore exceptions. Errors will be properly reported when the actual globbing is done.
+      }
     }
+
     try {
-      // At this point the package is guaranteed to exist.  It may have parse or
-      // evaluation errors, resulting in a diminished number of rules.
-      prefetchGlobs(packageId, buildFileAST, preprocessingResult.preprocessed,
-          buildFile, globber, defaultVisibility, makeEnv);
-      return evaluateBuildFile(
-          packageId, buildFileAST, buildFile, globber,
-          Iterables.concat(preprocessingEvents, localReporter.getEvents()),
-          defaultVisibility, preprocessingResult.containsErrors,
-          preprocessingResult.containsTransientErrors, makeEnv, imports, skylarkFileDependencies);
+      executeBuildFileImpl(
+          pkgBuilder, buildFileProgram, predeclared, loadedModules, starlarkSemantics, globber);
     } catch (InterruptedException e) {
       globber.onInterrupt();
       throw e;
@@ -953,320 +630,277 @@ public Package.LegacyBuilder createPackageFromPreprocessingResult(PackageIdentif
     }
   }
 
-  /**
-   * Same as {@link #createPackage}, but does the required validation of "packageName" first,
-   * throwing a {@link NoSuchPackageException} if the name is invalid.
-   */
-  @VisibleForTesting
-  public Package createPackageForTesting(PackageIdentifier packageId,
-      Path buildFile,
-      CachingPackageLocator locator,
-      EventHandler eventHandler) throws NoSuchPackageException, InterruptedException {
-    String error = LabelValidator.validatePackageName(
-        packageId.getPackageFragment().getPathString());
-    if (error != null) {
-      throw new BuildFileNotFoundException(packageId.toString(),
-          "illegal package name: '" + packageId.toString() + "' (" + error + ")");
-    }
-    ParserInputSource inputSource = maybeGetParserInputSource(buildFile, eventHandler);
-    if (inputSource == null) {
-      throw new BuildFileContainsErrorsException(packageId.toString(), "IOException occured");
-    }
-    Package result = createPackage(packageId, buildFile,
-        ImmutableList.<Statement>of(), inputSource,
-        ImmutableMap.<PathFragment, SkylarkEnvironment>of(),
-        ImmutableList.<Label>of(),
-        locator, ConstantRuleVisibility.PUBLIC,
-        createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator)).build();
-    Event.replayEventsOn(eventHandler, result.getEvents());
-    return result;
-  }
+  private void executeBuildFileImpl(
+      Package.Builder pkgBuilder,
+      Program buildFileProgram,
+      ImmutableMap<String, Object> predeclared,
+      ImmutableMap<String, Module> loadedModules,
+      StarlarkSemantics semantics,
+      Globber globber)
+      throws InterruptedException {
+    pkgBuilder.setThirdPartyLicenceExistencePolicy(
+        ruleClassProvider.getThirdPartyLicenseExistencePolicy());
 
-  /** Preprocesses the given BUILD file. */
-  // Used outside of bazel!
-  public Preprocessor.Result preprocess(
-      PackageIdentifier packageId,
-      Path buildFile,
-      CachingPackageLocator locator,
-      EventHandler eventHandler) throws InterruptedException {
-    ParserInputSource inputSource = maybeGetParserInputSource(buildFile, eventHandler);
-    if (inputSource == null) {
-      return Preprocessor.Result.transientError(buildFile);
+    // TODO(adonovan): opt: don't precompute this value, which is rarely needed
+    // and can be derived from Package.loads (if available) on demand.
+    pkgBuilder.setStarlarkFileDependencies(transitiveClosureOfLabels(loadedModules));
+    if (packageSettings.recordLoadedModules()) {
+      pkgBuilder.setLoads(loadedModules);
     }
-    Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator);
-    try {
-      return preprocess(packageId, buildFile, inputSource, globber, eventHandler);
-    } finally {
-      globber.onCompletion();
-    }
-  }
 
-  /**
-   * Preprocesses the given BUILD file, executing {@code globber.onInterrupt()} on an
-   * {@link InterruptedException}.
-   */
-  // Used outside of bazel!
-  public Preprocessor.Result preprocess(
-      PackageIdentifier packageId,
-      Path buildFile,
-      ParserInputSource inputSource,
-      Globber globber,
-      EventHandler eventHandler) throws InterruptedException {
-    Preprocessor preprocessor = preprocessorFactory.getPreprocessor();
-    if (preprocessor == null) {
-      return Preprocessor.Result.noPreprocessing(inputSource);
-    }
-    try {
-      return preprocessor.preprocess(inputSource, packageId.toString(), globber, eventHandler,
-          globalEnv, ruleFactory.getRuleClassNames());
-    } catch (IOException e) {
-      eventHandler.handle(Event.error(Location.fromFile(buildFile),
-                     "preprocessing failed: " + e.getMessage()));
-      return Preprocessor.Result.transientError(buildFile);
-    } catch (InterruptedException e) {
-      globber.onInterrupt();
-      throw e;
-    }
-  }
-
-  // Used outside of bazel!
-  public LegacyGlobber createLegacyGlobber(Path packageDirectory, PackageIdentifier packageId,
-      CachingPackageLocator locator) {
-    return new LegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
-        threadPool));
-  }
+    StoredEventHandler eventHandler = new StoredEventHandler();
+    PackageContext pkgContext = new PackageContext(pkgBuilder, globber, eventHandler);
+
+    try (Mutability mu = Mutability.create("package", pkgBuilder.getFilename())) {
+      Module module = Module.withPredeclared(semantics, predeclared);
+      StarlarkThread thread = new StarlarkThread(mu, semantics);
+      thread.setLoader(loadedModules::get);
+      thread.setPrintHandler(Event.makeDebugPrintHandler(pkgContext.eventHandler));
+
+      new BazelStarlarkContext(
+              BazelStarlarkContext.Phase.LOADING,
+              ruleClassProvider.getToolsRepository(),
+              /*fragmentNameToClass=*/ null,
+              pkgBuilder.getRepositoryMapping(),
+              pkgBuilder.getConvertedLabelsInPackage(),
+              new SymbolGenerator<>(pkgBuilder.getPackageIdentifier()),
+              /*analysisRuleLabel=*/ null)
+          .storeInThread(thread);
+
+      // TODO(adonovan): save this as a field in BazelStarlarkContext.
+      // It needn't be a second thread-local.
+      thread.setThreadLocal(PackageContext.class, pkgContext);
 
-  @Nullable
-  private ParserInputSource maybeGetParserInputSource(Path buildFile, EventHandler eventHandler) {
-    try {
-      return ParserInputSource.create(buildFile);
-    } catch (IOException e) {
-      eventHandler.handle(Event.error(Location.fromFile(buildFile), e.getMessage()));
-      return null;
+      try {
+        Starlark.execFileProgram(buildFileProgram, module, thread);
+      } catch (EvalException ex) {
+        pkgContext.eventHandler.handle(
+            Package.error(null, ex.getMessageWithStack(), Code.STARLARK_EVAL_ERROR));
+        pkgBuilder.setContainsErrors();
+      } catch (InterruptedException ex) {
+        if (pkgContext.pkgBuilder.containsErrors()) {
+          // Suppress the interrupted exception: we have an error of our own to return.
+          Thread.currentThread().interrupt();
+          logger.atInfo().withCause(ex).log(
+              "Suppressing InterruptedException for Package %s because an error was also found",
+              pkgBuilder.getPackageIdentifier().getCanonicalForm());
+        } else {
+          throw ex;
+        }
+      }
+      pkgBuilder.setComputationSteps(thread.getExecutedSteps());
     }
-  }
-
-  /**
-   * This tuple holds the current package builder, current lexer, etc, for the
-   * duration of the evaluation of one BUILD file. (We use a PackageContext
-   * object in preference to storing these values in mutable fields of the
-   * PackageFactory.)
-   *
-   * <p>PLEASE NOTE: references to PackageContext objects are held by many
-   * Function closures, but should become unreachable once the Environment is
-   * discarded at the end of evaluation.  Please be aware of your memory
-   * footprint when making changes here!
-   */
-  public static class PackageContext {
-
-    final Package.LegacyBuilder pkgBuilder;
-    final Globber globber;
-    final EventHandler eventHandler;
 
-    @VisibleForTesting
-    public PackageContext(Package.LegacyBuilder pkgBuilder, Globber globber,
-        EventHandler eventHandler) {
-      this.pkgBuilder = pkgBuilder;
-      this.eventHandler = eventHandler;
-      this.globber = globber;
-    }
+    pkgBuilder.addPosts(eventHandler.getPosts());
+    pkgBuilder.addEvents(eventHandler.getEvents());
   }
 
-  /**
-   * Returns the list of native rule functions created using the {@link RuleClassProvider}
-   * of this {@link PackageFactory}.
-   */
-  public ImmutableList<Function> collectNativeRuleFunctions() {
-    ImmutableList.Builder<Function> builder = ImmutableList.builder();
-    for (String ruleClass : ruleFactory.getRuleClassNames()) {
-      builder.add(newRuleFunction(ruleFactory, ruleClass));
-    }
-    builder.add(newGlobFunction(null, false));
-    builder.add(newPackageFunction(packageArguments));
-    return builder.build();
+  private static ImmutableList<Label> transitiveClosureOfLabels(
+      ImmutableMap<String, Module> loads) {
+    Set<Label> set = Sets.newLinkedHashSet();
+    transitiveClosureOfLabelsRec(set, loads);
+    return ImmutableList.copyOf(set);
   }
 
-  private void buildPkgEnv(Environment pkgEnv, String packageName,
-      MakeEnvironment.Builder pkgMakeEnv, PackageContext context, RuleFactory ruleFactory) {
-    pkgEnv.update("distribs", newDistribsFunction(context));
-    pkgEnv.update("glob", newGlobFunction(context, /*async=*/false));
-    pkgEnv.update("mocksubinclude", newMockSubincludeFunction(context));
-    pkgEnv.update("licenses", newLicensesFunction(context));
-    pkgEnv.update("exports_files", newExportsFilesFunction(context));
-    pkgEnv.update("package_group", newPackageGroupFunction(context));
-    pkgEnv.update("package", newPackageFunction(packageArguments));
-    pkgEnv.update("subinclude", newSubincludeFunction());
-    pkgEnv.update("environment_group", newEnvironmentGroupFunction(context));
-
-    pkgEnv.update("PACKAGE_NAME", packageName);
-
-    for (String ruleClass : ruleFactory.getRuleClassNames()) {
-      Function ruleFunction = newRuleFunction(ruleFactory, ruleClass);
-      pkgEnv.update(ruleClass, ruleFunction);
-    }
-
-    for (EnvironmentExtension extension : environmentExtensions) {
-      extension.update(pkgEnv, pkgMakeEnv, context.pkgBuilder.getBuildFileLabel());
+  private static void transitiveClosureOfLabelsRec(
+      Set<Label> set, ImmutableMap<String, Module> loads) {
+    for (Module m : loads.values()) {
+      BazelModuleContext ctx = BazelModuleContext.of(m);
+      if (set.add(ctx.label())) {
+        transitiveClosureOfLabelsRec(set, ctx.loads());
+      }
     }
   }
 
   /**
-   * Constructs a Package instance, evaluates the BUILD-file AST inside the
-   * build environment, and populates the package with Rule instances as it
-   * goes.  As with most programming languages, evaluation stops when an
-   * exception is encountered: no further rules after the point of failure will
-   * be constructed.  We assume that rules constructed before the point of
-   * failure are valid; this assumption is not entirely correct, since a
-   * "vardef" after a rule declaration can affect the behavior of that rule.
+   * checkBuildSyntax is a static pass over the syntax tree of a BUILD (not .bzl) file.
    *
-   * <p>Rule attribute checking is performed during evaluation. Each attribute
-   * must conform to the type specified for that <i>(rule class, attribute
-   * name)</i> pair.  Errors reported at this stage include: missing value for
-   * mandatory attribute, value of wrong type.  Such error cause Rule
-   * construction to be aborted, so the resulting package will have missing
-   * members.
+   * <p>It reports an error to the event handler if it discovers a {@code def}, {@code if}, or
+   * {@code for} statement, or a {@code f(*args)} or {@code f(**kwargs)} call.
    *
-   * @see PackageFactory#PackageFactory
-   */
-  @VisibleForTesting // used by PackageFactoryApparatus
-  public Package.LegacyBuilder evaluateBuildFile(PackageIdentifier packageId,
-      BuildFileAST buildFileAST, Path buildFilePath, Globber globber,
-      Iterable<Event> pastEvents, RuleVisibility defaultVisibility, boolean containsError,
-      boolean containsTransientError, MakeEnvironment.Builder pkgMakeEnv,
-      Map<PathFragment, SkylarkEnvironment> imports,
-      ImmutableList<Label> skylarkFileDependencies) throws InterruptedException {
-    // Important: Environment should be unreachable by the end of this method!
-    StoredEventHandler eventHandler = new StoredEventHandler();
-    Environment pkgEnv = new Environment(globalEnv, eventHandler);
-
-    Package.LegacyBuilder pkgBuilder =
-        new Package.LegacyBuilder(packageId)
-        .setGlobber(globber)
-        .setFilename(buildFilePath)
-        .setMakeEnv(pkgMakeEnv)
-        .setDefaultVisibility(defaultVisibility)
-        // "defaultVisibility" comes from the command line. Let's give the BUILD file a chance to
-        // set default_visibility once, be reseting the PackageBuilder.defaultVisibilitySet flag.
-        .setDefaultVisibilitySet(false)
-        .setSkylarkFileDependencies(skylarkFileDependencies);
-
-    Event.replayEventsOn(eventHandler, pastEvents);
-
-    // Stuff that closes over the package context:
-    PackageContext context = new PackageContext(pkgBuilder, globber, eventHandler);
-    buildPkgEnv(pkgEnv, packageId.toString(), pkgMakeEnv, context, ruleFactory);
-
-    if (containsError) {
-      pkgBuilder.setContainsErrors();
-    }
+   * <p>It extracts literal {@code glob(include="pattern")} patterns and adds them to {@code globs},
+   * or to {@code globsWithDirs} if the call had a {@code exclude_directories=0} argument.
+   *
+   * <p>It records in {@code generatorNameByLocation} all calls of the form {@code f(name="foo",
+   * ...)} so that any rules instantiated during the call to {@code f} can be ascribed a "generator
+   * name" of {@code "foo"}.
+   *
+   * <p>It returns true if it reported no errors.
+   */
+  // TODO(adonovan): restructure so that this is called from the sole place that executes BUILD
+  // files. Also, make private; there's no reason for tests to call this directly.
+  public static boolean checkBuildSyntax(
+      StarlarkFile file,
+      Collection<String> globs,
+      Collection<String> globsWithDirs,
+      Map<Location, String> generatorNameByLocation,
+      Consumer<SyntaxError> errors) {
+    final boolean[] success = {true};
+    NodeVisitor checker =
+        new NodeVisitor() {
+          void error(Location loc, String message) {
+            errors.accept(new SyntaxError(loc, message));
+            success[0] = false;
+          }
 
-    if (containsTransientError) {
-      pkgBuilder.setContainsTemporaryErrors();
-    }
+          // Extract literal glob patterns from calls of the form:
+          //   glob(include = ["pattern"])
+          //   glob(["pattern"])
+          // This may spuriously match user-defined functions named glob;
+          // that's ok, it's only a heuristic.
+          void extractGlobPatterns(CallExpression call) {
+            if (call.getFunction() instanceof Identifier
+                && ((Identifier) call.getFunction()).getName().equals("glob")) {
+              Expression excludeDirectories = null, include = null;
+              List<Argument> arguments = call.getArguments();
+              for (int i = 0; i < arguments.size(); i++) {
+                Argument arg = arguments.get(i);
+                String name = arg.getName();
+                if (name == null) {
+                  if (i == 0) { // first positional argument
+                    include = arg.getValue();
+                  }
+                } else if (name.equals("include")) {
+                  include = arg.getValue();
+                } else if (name.equals("exclude_directories")) {
+                  excludeDirectories = arg.getValue();
+                }
+              }
+              if (include instanceof ListExpression) {
+                for (Expression elem : ((ListExpression) include).getElements()) {
+                  if (elem instanceof StringLiteral) {
+                    String pattern = ((StringLiteral) elem).getValue();
+                    // exclude_directories is (oddly) an int with default 1.
+                    boolean exclude = true;
+                    if (excludeDirectories instanceof IntLiteral) {
+                      Number v = ((IntLiteral) excludeDirectories).getValue();
+                      if (v instanceof Integer && (Integer) v == 0) {
+                        exclude = false;
+                      }
+                    }
+                    (exclude ? globs : globsWithDirs).add(pattern);
+                  }
+                }
+              }
+            }
+          }
 
-    if (!validatePackageIdentifier(packageId, buildFileAST.getLocation(), eventHandler)) {
-      pkgBuilder.setContainsErrors();
-    }
+          // Reject f(*args) and f(**kwargs) calls in BUILD files.
+          void rejectStarArgs(CallExpression call) {
+            for (Argument arg : call.getArguments()) {
+              if (arg instanceof Argument.StarStar) {
+                error(
+                    arg.getStartLocation(),
+                    "**kwargs arguments are not allowed in BUILD files. Pass the arguments in "
+                        + "explicitly.");
+              } else if (arg instanceof Argument.Star) {
+                error(
+                    arg.getStartLocation(),
+                    "*args arguments are not allowed in BUILD files. Pass the arguments in "
+                        + "explicitly.");
+              }
+            }
+          }
 
-    pkgEnv.setImportedExtensions(imports);
-    pkgEnv.updateAndPropagate(PKG_CONTEXT, context);
-    pkgEnv.updateAndPropagate(Environment.PKG_NAME, packageId.toString());
+          // Record calls of the form f(name="foo", ...)
+          // so that we can later ascribe "foo" as the "generator name"
+          // of any rules instantiated during the call of f.
+          void recordGeneratorName(CallExpression call) {
+            for (Argument arg : call.getArguments()) {
+              if (arg instanceof Argument.Keyword
+                  && arg.getName().equals("name")
+                  && arg.getValue() instanceof StringLiteral) {
+                generatorNameByLocation.put(
+                    call.getLparenLocation(), ((StringLiteral) arg.getValue()).getValue());
+              }
+            }
+          }
 
-    if (!validateAssignmentStatements(pkgEnv, buildFileAST, eventHandler)) {
-      pkgBuilder.setContainsErrors();
-    }
+          // We prune the traversal if we encounter def/if/for,
+          // as we have already reported the root error and there's
+          // no point reporting more.
 
-    if (buildFileAST.containsErrors()) {
-      pkgBuilder.setContainsErrors();
-    }
+          @Override
+          public void visit(DefStatement node) {
+            error(
+                node.getStartLocation(),
+                "functions may not be defined in BUILD files. You may move the function to "
+                    + "a .bzl file and load it.");
+          }
 
-    // TODO(bazel-team): (2009) the invariant "if errors are reported, mark the package
-    // as containing errors" is strewn all over this class.  Refactor to use an
-    // event sensor--and see if we can simplify the calling code in
-    // createPackage().
-    if (!buildFileAST.exec(pkgEnv, eventHandler)) {
-      pkgBuilder.setContainsErrors();
-    }
+          @Override
+          public void visit(LambdaExpression node) {
+            error(
+                node.getStartLocation(),
+                "functions may not be defined in BUILD files. You may move the function to "
+                    + "a .bzl file and load it.");
+          }
 
-    pkgBuilder.addEvents(eventHandler.getEvents());
-    return pkgBuilder;
-  }
+          @Override
+          public void visit(ForStatement node) {
+            error(
+                node.getStartLocation(),
+                "for statements are not allowed in BUILD files. You may inline the loop, move it "
+                    + "to a function definition (in a .bzl file), or as a last resort use a list "
+                    + "comprehension.");
+          }
 
-  /**
-   * Visit all targets and expand the globs in parallel.
-   */
-  private void prefetchGlobs(PackageIdentifier packageId, BuildFileAST buildFileAST,
-      boolean wasPreprocessed, Path buildFilePath, Globber globber,
-      RuleVisibility defaultVisibility, MakeEnvironment.Builder pkgMakeEnv)
-      throws InterruptedException {
-    if (wasPreprocessed) {
-      // No point in prefetching globs here: preprocessing implies eager evaluation
-      // of all globs.
-      return;
-    }
-    // Important: Environment should be unreachable by the end of this method!
-    Environment pkgEnv = new Environment();
-
-    Package.LegacyBuilder pkgBuilder =
-        new Package.LegacyBuilder(packageId)
-        .setFilename(buildFilePath)
-        .setMakeEnv(pkgMakeEnv)
-        .setDefaultVisibility(defaultVisibility)
-        // "defaultVisibility" comes from the command line. Let's give the BUILD file a chance to
-        // set default_visibility once, be reseting the PackageBuilder.defaultVisibilitySet flag.
-        .setDefaultVisibilitySet(false);
-
-    // Stuff that closes over the package context:
-    PackageContext context = new PackageContext(pkgBuilder, globber, NullEventHandler.INSTANCE);
-    buildPkgEnv(pkgEnv, packageId.toString(), pkgMakeEnv, context, ruleFactory);
-    pkgEnv.update("glob", newGlobFunction(context, /*async=*/true));
-    // The Fileset function is heavyweight in that it can run glob(). Avoid this during the
-    // preloading phase.
-    pkgEnv.remove("FilesetEntry");
-
-    buildFileAST.exec(pkgEnv, NullEventHandler.INSTANCE);
-  }
+          @Override
+          public void visit(IfStatement node) {
+            error(
+                node.getStartLocation(),
+                "if statements are not allowed in BUILD files. You may move conditional logic to a "
+                    + "function definition (in a .bzl file), or for simple cases use an if "
+                    + "expression.");
+          }
 
+          @Override
+          public void visit(CallExpression node) {
+            extractGlobPatterns(node);
+            rejectStarArgs(node);
+            recordGeneratorName(node);
+            // Continue traversal so as not to miss nested calls
+            // like cc_binary(..., f(**kwargs), srcs=glob(...), ...).
+            super.visit(node);
+          }
+        };
+    checker.visit(file);
+    return success[0];
+  }
+
+  // Install profiler hooks into Starlark interpreter.
+  static {
+    // parser profiler
+    StarlarkFile.setParseProfiler(
+        new StarlarkFile.ParseProfiler() {
+          @Override
+          public Object start(String filename) {
+            return Profiler.instance().profile(ProfilerTask.STARLARK_PARSER, filename);
+          }
 
-  /**
-   * Tests a build AST to ensure that it contains no assignment statements that
-   * redefine built-in build rules.
-   *
-   * @param pkgEnv a package environment initialized with all of the built-in
-   *        build rules
-   * @param ast the build file AST to be tested
-   * @param eventHandler a eventHandler where any errors should be logged
-   * @return true if the build file contains no redefinitions of built-in
-   *         functions
-   */
-  private static boolean validateAssignmentStatements(Environment pkgEnv,
-                                                      BuildFileAST ast,
-                                                      EventHandler eventHandler) {
-    for (Statement stmt : ast.getStatements()) {
-      if (stmt instanceof AssignmentStatement) {
-        Expression lvalue = ((AssignmentStatement) stmt).getLValue();
-        if (!(lvalue instanceof Ident)) {
-          continue;
-        }
-        String target = ((Ident) lvalue).getName();
-        if (pkgEnv.lookup(target, null) != null) {
-          eventHandler.handle(Event.error(stmt.getLocation(), "Reassignment of builtin build "
-              + "function '" + target + "' not permitted"));
-          return false;
-        }
-      }
-    }
-    return true;
-  }
+          @Override
+          public void end(Object span) {
+            ((SilentCloseable) span).close();
+          }
+        });
+
+    // call profiler
+    StarlarkThread.setCallProfiler(
+        new StarlarkThread.CallProfiler() {
+          @Override
+          public Object start(StarlarkCallable fn) {
+            return Profiler.instance()
+                .profile(
+                    fn instanceof StarlarkFunction
+                        ? ProfilerTask.STARLARK_USER_FN
+                        : ProfilerTask.STARLARK_BUILTIN_FN,
+                    fn.getName());
+          }
 
-  // Reports an error and returns false iff package identifier was illegal.
-  private static boolean validatePackageIdentifier(PackageIdentifier packageId, Location location,
-      EventHandler eventHandler) {
-    String error = LabelValidator.validatePackageName(packageId.getPackageFragment().toString());
-    if (error != null) {
-      eventHandler.handle(Event.error(location, error));
-      return false; // Invalid package name 'foo'
-    }
-    return true;
+          @Override
+          public void end(Object span) {
+            ((SilentCloseable) span).close();
+          }
+        });
   }
 }
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
index c79bbaa3956..195ea7f4903 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -1,4 +1,4 @@
-// Copyright 2014 Google Inc. All rights reserved.
+// Copyright 2014 The Bazel Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,51 +15,47 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
-import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
-import com.google.devtools.build.lib.syntax.FuncallExpression;
-import com.google.devtools.build.lib.syntax.Label;
-import com.google.devtools.build.lib.syntax.Label.SyntaxException;
-import com.google.devtools.build.lib.vfs.PathFragment;
-
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import java.util.Map;
 import java.util.Set;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.StarlarkThread.CallStackEntry;
+import net.starlark.java.syntax.Location;
 
 /**
- * Given a rule class and a set of attributes, returns a Rule instance. Also
- * performs a number of checks and associates the rule and the owning package
- * with each other.
+ * Given a {@link RuleClass} and a set of attribute values, returns a {@link Rule} instance. Also
+ * performs a number of checks and associates the {@link Rule} and the owning {@link Package} with
+ * each other.
+ *
+ * <p>This class is immutable, once created the set of managed {@link RuleClass}es will not change.
  *
- * <p>Note: the code that actually populates the RuleClass map has been moved
- * to {@link RuleClassProvider}.
+ * <p>Note: the code that actually populates the RuleClass map has been moved to {@link
+ * RuleClassProvider}.
  */
 public class RuleFactory {
 
-  /**
-   * Maps rule class name to the metaclass instance for that rule.
-   */
+  /** Maps rule class name to the metaclass instance for that rule. */
   private final ImmutableMap<String, RuleClass> ruleClassMap;
 
-  /**
-   * Constructs a RuleFactory instance.
-   */
+  /** Constructs a RuleFactory instance. */
   public RuleFactory(RuleClassProvider provider) {
     this.ruleClassMap = ImmutableMap.copyOf(provider.getRuleClassMap());
   }
 
-  /**
-   * Returns the (immutable, unordered) set of names of all the known rule classes.
-   */
+  /** Returns the (immutable, unordered) set of names of all the known rule classes. */
   public Set<String> getRuleClassNames() {
     return ruleClassMap.keySet();
   }
 
-  /**
-   * Returns the RuleClass for the specified rule class name.
-   */
+  /** Returns the RuleClass for the specified rule class name. */
   public RuleClass getRuleClass(String ruleClassName) {
     return ruleClassMap.get(ruleClassName);
   }
@@ -67,34 +63,24 @@ public RuleClass getRuleClass(String ruleClassName) {
   /**
    * Creates and returns a rule instance.
    *
-   * <p>It is the caller's responsibility to add the rule to the package (the
-   * caller may choose not to do so if, for example, the rule has errors).
-   *
-   * @param pkgBuilder the under-construction package to which the rule belongs
-   * @param ruleClass the class of the rule; this must not be null
-   * @param attributeValues a map of attribute names to attribute values. Each
-   *        attribute must be defined for this class of rule, and have a value
-   *        of the appropriate type. There must be a map entry for each
-   *        non-optional attribute of this class of rule.
-   * @param eventHandler a eventHandler on which errors and warnings are reported during
-   *        rule creation
-   * @param ast the abstract syntax tree of the rule expression (optional)
-   * @param location the location at which this rule was declared
-   * @throws InvalidRuleException if the rule could not be constructed for any
-   *         reason (e.g. no <code>name</code> attribute is defined)
-   * @throws NameConflictException
+   * <p>It is the caller's responsibility to add the rule to the package (the caller may choose not
+   * to do so if, for example, the rule has errors).
    */
-  static Rule createAndAddRule(Package.AbstractBuilder<?, ?> pkgBuilder,
-                  RuleClass ruleClass,
-                  Map<String, Object> attributeValues,
-                  EventHandler eventHandler,
-                  FuncallExpression ast,
-                  Location location) throws InvalidRuleException, NameConflictException {
+  public static Rule createRule(
+      Package.Builder pkgBuilder,
+      RuleClass ruleClass,
+      BuildLangTypedAttributeValuesMap attributeValues,
+      EventHandler eventHandler,
+      StarlarkSemantics semantics,
+      ImmutableList<StarlarkThread.CallStackEntry> callstack)
+      throws InvalidRuleException, InterruptedException {
     Preconditions.checkNotNull(ruleClass);
     String ruleClassName = ruleClass.getName();
-    Object nameObject = attributeValues.get("name");
-    if (!(nameObject instanceof String)) {
+    Object nameObject = attributeValues.getAttributeValue("name");
+    if (nameObject == null) {
       throw new InvalidRuleException(ruleClassName + " rule has no 'name' attribute");
+    } else if (!(nameObject instanceof String)) {
+      throw new InvalidRuleException(ruleClassName + " 'name' attribute must be a string");
     }
     String name = (String) nameObject;
     Label label;
@@ -102,44 +88,219 @@ static Rule createAndAddRule(Package.AbstractBuilder<?, ?> pkgBuilder,
       // Test that this would form a valid label name -- in particular, this
       // catches cases where Makefile variables $(foo) appear in "name".
       label = pkgBuilder.createLabel(name);
-    } catch (Label.SyntaxException e) {
+    } catch (LabelSyntaxException e) {
       throw new InvalidRuleException("illegal rule name: " + name + ": " + e.getMessage());
     }
-    boolean inWorkspaceFile = location.getPath() != null
-        && location.getPath().endsWith(new PathFragment("WORKSPACE"));
+    boolean inWorkspaceFile = pkgBuilder.isWorkspace();
     if (ruleClass.getWorkspaceOnly() && !inWorkspaceFile) {
-      throw new RuleFactory.InvalidRuleException(ruleClass + " must be in the WORKSPACE file "
-          + "(used by " + label + ")");
+      throw new RuleFactory.InvalidRuleException(
+          ruleClass + " must be in the WORKSPACE file " + "(used by " + label + ")");
     } else if (!ruleClass.getWorkspaceOnly() && inWorkspaceFile) {
-      throw new RuleFactory.InvalidRuleException(ruleClass + " cannot be in the WORKSPACE file "
-          + "(used by " + label + ")");
+      throw new RuleFactory.InvalidRuleException(
+          ruleClass + " cannot be in the WORKSPACE file " + "(used by " + label + ")");
     }
 
+    AttributesAndLocation generator =
+        generatorAttributesForMacros(pkgBuilder, attributeValues, callstack);
+
+    // The raw stack is of the form [<toplevel>@BUILD:1, macro@lib.bzl:1, cc_library@<builtin>].
+    // Pop the innermost frame for the rule, since it's obvious.
+    callstack = callstack.subList(0, callstack.size() - 1); // pop
+
     try {
-      Rule rule = ruleClass.createRuleWithLabel(pkgBuilder, label, attributeValues,
-          eventHandler, ast, location);
-      pkgBuilder.addRule(rule);
-      return rule;
-    } catch (SyntaxException e) {
+      // Examines --incompatible_disable_third_party_license_checking to see if we should check
+      // third party targets for license existence.
+      //
+      // This flag is overridable by RuleClass.ThirdPartyLicenseEnforcementPolicy (which is checked
+      // in RuleClass). This lets Bazel and Blaze migrate away from license logic on independent
+      // timelines. See --incompatible_disable_third_party_license_checking comments for details.
+      boolean checkThirdPartyLicenses =
+          !semantics.getBool(
+              BuildLanguageOptions.INCOMPATIBLE_DISABLE_THIRD_PARTY_LICENSE_CHECKING);
+      return ruleClass.createRule(
+          pkgBuilder,
+          label,
+          generator.attributes,
+          eventHandler,
+          generator.location, // see b/23974287 for rationale
+          callstack,
+          checkThirdPartyLicenses);
+    } catch (LabelSyntaxException | CannotPrecomputeDefaultsException e) {
       throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage());
     }
   }
 
-  public static Rule createAndAddRule(PackageContext context,
+  /**
+   * Creates a {@link Rule} instance, adds it to the {@link Package.Builder} and returns it.
+   *
+   * @param pkgBuilder the under-construction {@link Package.Builder} to which the rule belongs
+   * @param ruleClass the {@link RuleClass} of the rule
+   * @param attributeValues a {@link BuildLangTypedAttributeValuesMap} mapping attribute names to
+   *     attribute values of build-language type. Each attribute must be defined for this class of
+   *     rule, and have a build-language-typed value which can be converted to the appropriate
+   *     native type of the attribute (i.e. via {@link BuildType#selectableConvert}). There must be
+   *     a map entry for each non-optional attribute of this class of rule.
+   * @param eventHandler a eventHandler on which errors and warnings are reported during rule
+   *     creation
+   * @param semantics the Starlark semantics
+   * @param callstack the stack of active calls in the Starlark thread
+   * @throws InvalidRuleException if the rule could not be constructed for any reason (e.g. no
+   *     {@code name} attribute is defined)
+   * @throws NameConflictException if the rule's name or output files conflict with others in this
+   *     package
+   * @throws InterruptedException if interrupted
+   */
+  public static Rule createAndAddRule(
+      Package.Builder pkgBuilder,
       RuleClass ruleClass,
-      Map<String, Object> attributeValues,
-      FuncallExpression ast) throws InvalidRuleException, NameConflictException {
-    return createAndAddRule(context.pkgBuilder, ruleClass, attributeValues, context.eventHandler,
-        ast, ast.getLocation());
+      BuildLangTypedAttributeValuesMap attributeValues,
+      EventHandler eventHandler,
+      StarlarkSemantics semantics,
+      ImmutableList<StarlarkThread.CallStackEntry> callstack)
+      throws InvalidRuleException, NameConflictException, InterruptedException {
+    Rule rule =
+        createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, semantics, callstack);
+    pkgBuilder.addRule(rule);
+    return rule;
   }
 
   /**
-   * InvalidRuleException is thrown by createRule() if the Rule could not be
-   * constructed. It contains an error message.
+   * InvalidRuleException is thrown by {@link Rule} creation methods if the {@link Rule} could not
+   * be constructed. It contains an error message.
    */
   public static class InvalidRuleException extends Exception {
-    private InvalidRuleException(String message) {
+    public InvalidRuleException(String message) {
       super(message);
     }
   }
+
+  /** A pair of attributes and location. */
+  private static final class AttributesAndLocation {
+    final BuildLangTypedAttributeValuesMap attributes;
+    final Location location;
+
+    AttributesAndLocation(BuildLangTypedAttributeValuesMap attributes, Location location) {
+      this.attributes = attributes;
+      this.location = location;
+    }
+  }
+
+  /**
+   * A wrapper around an map of named attribute values that specifies whether the map's values are
+   * of "build-language" or of "native" types.
+   */
+  public interface AttributeValues<T> {
+    /**
+     * Returns {@code true} if all the map's values are "build-language typed", i.e., resulting from
+     * the evaluation of an expression in the build language. Returns {@code false} if all the map's
+     * values are "natively typed", i.e. of a type returned by {@link BuildType#selectableConvert}.
+     */
+    boolean valuesAreBuildLanguageTyped();
+
+    Iterable<T> getAttributeAccessors();
+
+    String getName(T attributeAccessor);
+
+    Object getValue(T attributeAccessor);
+
+    boolean isExplicitlySpecified(T attributeAccessor);
+  }
+
+  /** A {@link AttributeValues} of explicit "build-language" values. */
+  public static final class BuildLangTypedAttributeValuesMap
+      implements AttributeValues<Map.Entry<String, Object>> {
+    private final Map<String, Object> attributeValues;
+
+    public BuildLangTypedAttributeValuesMap(Map<String, Object> attributeValues) {
+      this.attributeValues = attributeValues;
+    }
+
+    private boolean containsAttributeNamed(String attributeName) {
+      return attributeValues.containsKey(attributeName);
+    }
+
+    private Object getAttributeValue(String attributeName) {
+      return attributeValues.get(attributeName);
+    }
+
+    @Override
+    public boolean valuesAreBuildLanguageTyped() {
+      return true;
+    }
+
+    @Override
+    public Iterable<Map.Entry<String, Object>> getAttributeAccessors() {
+      return attributeValues.entrySet();
+    }
+
+    @Override
+    public String getName(Map.Entry<String, Object> attributeAccessor) {
+      return attributeAccessor.getKey();
+    }
+
+    @Override
+    public Object getValue(Map.Entry<String, Object> attributeAccessor) {
+      return attributeAccessor.getValue();
+    }
+
+    @Override
+    public boolean isExplicitlySpecified(Map.Entry<String, Object> attributeAccessor) {
+      return true;
+    }
+  }
+
+  /**
+   * If the rule was created by a macro, this method sets the appropriate values for the attributes
+   * generator_{name, function, location} and returns all attributes.
+   *
+   * <p>Otherwise, it returns the given attributes without any changes.
+   */
+  private static AttributesAndLocation generatorAttributesForMacros(
+      Package.Builder pkgBuilder,
+      BuildLangTypedAttributeValuesMap args,
+      ImmutableList<CallStackEntry> stack) {
+    // For a callstack [BUILD <toplevel>, .bzl <function>, <rule>],
+    // location is that of the caller of 'rule' (the .bzl function).
+    Location location = stack.size() < 2 ? Location.BUILTIN : stack.get(stack.size() - 2).location;
+
+    boolean hasName = args.containsAttributeNamed("generator_name");
+    boolean hasFunc = args.containsAttributeNamed("generator_function");
+    // TODO(bazel-team): resolve cases in our code where hasName && !hasFunc, or hasFunc && !hasName
+    if (hasName || hasFunc) {
+      return new AttributesAndLocation(args, location);
+    }
+
+    // The "generator" of a rule is the function (sometimes called "macro")
+    // outermost in the call stack.
+    // The stack must contain at least two entries:
+    // 0: the outermost function (e.g. a BUILD file),
+    // 1: the function called by it (e.g. a "macro" in a .bzl file).
+    // optionally followed by other Starlark or built-in functions,
+    // and finally the rule instantiation function.
+    if (stack.size() < 2 || !stack.get(1).location.file().endsWith(".bzl")) {
+      return new AttributesAndLocation(args, location); // macro is not a Starlark function
+    }
+    Location generatorLocation = stack.get(0).location; // location of call to generator
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    for (Map.Entry<String, Object> attributeAccessor : args.getAttributeAccessors()) {
+      String attributeName = args.getName(attributeAccessor);
+      builder.put(attributeName, args.getValue(attributeAccessor));
+    }
+    String generatorName = pkgBuilder.getGeneratorNameByLocation(generatorLocation);
+    if (generatorName == null) {
+      generatorName = (String) args.getAttributeValue("name");
+    }
+    builder.put("generator_name", generatorName);
+
+    try {
+      args = new BuildLangTypedAttributeValuesMap(builder.build());
+    } catch (IllegalArgumentException unused) {
+      // We just fall back to the default case and swallow any messages.
+    }
+
+    // TODO(adonovan): is it appropriate to use generatorLocation as the rule's main location?
+    // Or would 'location' (the immediate call) be more informative? When there are errors, the
+    // location of the toplevel call of the generator may be quite unrelated to the error message.
+    return new AttributesAndLocation(args, generatorLocation);
+  }
 }
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
index 58a29a25388..a9b2d5dc1da 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
@@ -14,15 +14,21 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator;
 import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleHelper;
 import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue;
 import com.google.devtools.build.lib.bazel.bzlmod.RepoSpec;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
@@ -30,6 +36,8 @@
 import com.google.devtools.build.lib.packages.RuleFactory;
 import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -37,14 +45,16 @@
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import java.io.IOException;
 import java.util.Optional;
 import javax.annotation.Nullable;
+import net.starlark.java.eval.Module;
 import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread.CallStackEntry;
 import net.starlark.java.syntax.Location;
 
 /**
- * Look up the {link@ RepoSpec} of a given repository name and create its repository rule instance.
+ * Looks up the {@link RepoSpec} of a given repository name and create its repository rule instance.
  */
 public final class BzlmodRepoRuleFunction implements SkyFunction {
 
@@ -54,6 +64,7 @@ public final class BzlmodRepoRuleFunction implements SkyFunction {
   private final RuleClassProvider ruleClassProvider;
   private final BlazeDirectories directories;
   private final BzlmodRepoRuleHelper bzlmodRepoRuleHelper;
+  private static final PackageIdentifier ROOT_PACKAGE = PackageIdentifier.createInMainRepo("");
 
   public BzlmodRepoRuleFunction(
       PackageFactory packageFactory,
@@ -95,12 +106,16 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       return createRuleFromSpec(repoSpec, starlarkSemantics, env);
     }
 
-    Optional<RepoSpec> result = bzlmodRepoRuleHelper.getRepoSpec(env, repositoryName);
-    if (env.valuesMissing()) {
-      return null;
-    }
-    if (result.isPresent()) {
-      return createRuleFromSpec(result.get(), starlarkSemantics, env);
+    try {
+      Optional<RepoSpec> result = bzlmodRepoRuleHelper.getRepoSpec(env, repositoryName);
+      if (env.valuesMissing()) {
+        return null;
+      }
+      if (result.isPresent()) {
+        return createRuleFromSpec(result.get(), starlarkSemantics, env);
+      }
+    } catch (IOException e) {
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
     }
 
     return BzlmodRepoRuleValue.REPO_RULE_NOT_FOUND_VALUE;
@@ -112,13 +127,18 @@ private BzlmodRepoRuleValue createRuleFromSpec(
     if (repoSpec.isNativeRepoRule()) {
       return createNativeRepoRule(repoSpec, starlarkSemantics, env);
     }
-    // TODO(pcloudy): Implement creating starlark repository rule
-    return BzlmodRepoRuleValue.REPO_RULE_NOT_FOUND_VALUE;
+    return createStarlarkRepoRule(repoSpec, starlarkSemantics, env);
   }
 
   private BzlmodRepoRuleValue createNativeRepoRule(
       RepoSpec repoSpec, StarlarkSemantics semantics, Environment env)
       throws InterruptedException, BzlmodRepoRuleFunctionException {
+    if (!ruleClassProvider.getRuleClassMap().containsKey(repoSpec.ruleClassName())) {
+      InvalidRuleException e =
+          new InvalidRuleException(
+              "Unrecognized native repository rule: " + repoSpec.getRuleClass());
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    }
     RuleClass ruleClass = ruleClassProvider.getRuleClassMap().get(repoSpec.ruleClassName());
     BuildLangTypedAttributeValuesMap attributeValues =
         new BuildLangTypedAttributeValuesMap(repoSpec.attributes());
@@ -133,16 +153,95 @@ private BzlmodRepoRuleValue createNativeRepoRule(
     Package.Builder pkg = createExternalPackageBuilder(semantics);
     try {
       rule =
-          RuleFactory.createRule(
+          RuleFactory.createAndAddRule(
               pkg, ruleClass, attributeValues, env.getListener(), semantics, callStack.build());
-      // We need to actually build the package so that the rule has the correct package reference.
-      pkg.build();
+      return new BzlmodRepoRuleValue(pkg.build(), rule.getName());
     } catch (InvalidRuleException e) {
       throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
     } catch (NoSuchPackageException e) {
       throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    } catch (NameConflictException e) {
+      // This literally cannot happen -- we just created the package!
+      throw new IllegalStateException(e);
+    }
+  }
+
+  /** Loads modules from the given bzl file. */
+  private ImmutableMap<String, Module> loadBzlModules(Environment env, String bzlFile)
+      throws InterruptedException, BzlmodRepoRuleFunctionException {
+    ImmutableList<Pair<String, Location>> programLoads =
+        ImmutableList.of(Pair.of(bzlFile, Location.BUILTIN));
+
+    ImmutableList<Label> loadLabels =
+        BzlLoadFunction.getLoadLabels(
+            env.getListener(), programLoads, ROOT_PACKAGE, /*repoMapping=*/ ImmutableMap.of());
+    if (loadLabels == null) {
+      NoSuchPackageException e =
+          PackageFunction.PackageFunctionException.builder()
+              .setType(PackageFunction.PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS)
+              .setPackageIdentifier(ROOT_PACKAGE)
+              .setMessage("malformed load statements")
+              .setPackageLoadingCode(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR)
+              .buildCause();
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    }
+
+    Preconditions.checkArgument(loadLabels.size() == 1);
+    ImmutableList<BzlLoadValue.Key> keys =
+        ImmutableList.of(BzlLoadValue.keyForBzlmod(loadLabels.get(0)));
+
+    // Load the .bzl module.
+    try {
+      return PackageFunction.loadBzlModules(env, ROOT_PACKAGE, programLoads, keys, null);
+    } catch (NoSuchPackageException e) {
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    }
+  }
+
+  private BzlmodRepoRuleValue createStarlarkRepoRule(
+      RepoSpec repoSpec, StarlarkSemantics semantics, Environment env)
+      throws InterruptedException, BzlmodRepoRuleFunctionException {
+
+    ImmutableMap<String, Module> loadedModules = loadBzlModules(env, repoSpec.bzlFile().get());
+
+    if (env.valuesMissing()) {
+      return null;
+    }
+
+    BzlmodRepoRuleCreator repoRuleCreator = getRepoRuleCreator(repoSpec, loadedModules);
+
+    // TODO(bazel-team): Don't use the {@link Rule} class for repository rule.
+    // Currently, the repository rule is represented with the {@link Rule} class that's designed
+    // for build rules. Therefore, we have to create a package instance for it, which doesn't make
+    // sense. We should migrate away from this implementation so that we don't refer to any build
+    // rule specific things in repository rule.
+    Rule rule;
+    Package.Builder pkg = createExternalPackageBuilder(semantics);
+    StoredEventHandler eventHandler = new StoredEventHandler();
+    try {
+      rule = repoRuleCreator.createAndAddRule(pkg, semantics, repoSpec.attributes(), eventHandler);
+      return new BzlmodRepoRuleValue(pkg.build(), rule.getName());
+    } catch (InvalidRuleException e) {
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    } catch (NoSuchPackageException e) {
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
+    } catch (NameConflictException e) {
+      // This literally cannot happen -- we just created the package!
+      throw new IllegalStateException(e);
+    }
+  }
+
+  private BzlmodRepoRuleCreator getRepoRuleCreator(
+      RepoSpec repoSpec, ImmutableMap<String, Module> loadedModules)
+      throws BzlmodRepoRuleFunctionException {
+    Object object = loadedModules.get(repoSpec.bzlFile().get()).getGlobal(repoSpec.ruleClassName());
+    if (object instanceof BzlmodRepoRuleCreator) {
+      return (BzlmodRepoRuleCreator) object;
+    } else {
+      InvalidRuleException e =
+          new InvalidRuleException("Invalid repository rule: " + repoSpec.getRuleClass());
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
     }
-    return new BzlmodRepoRuleValue(rule);
   }
 
   /**
@@ -174,5 +273,9 @@ private static final class BzlmodRepoRuleFunctionException extends SkyFunctionEx
     BzlmodRepoRuleFunctionException(NoSuchPackageException e, Transience transience) {
       super(e, transience);
     }
+
+    BzlmodRepoRuleFunctionException(IOException e, Transience transience) {
+      super(e, transience);
+    }
   }
 }
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
index a495ea026f0..52c9666bef1 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
@@ -14,212 +14,225 @@
 package com.google.devtools.build.lib.packages;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assert.assertThrows;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
+import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
 import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
-import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.vfs.Path;
-
+import com.google.devtools.build.lib.vfs.RootedPath;
 import java.util.HashMap;
 import java.util.Map;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.syntax.Location;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 
-public class RuleFactoryTest extends PackageLoadingTestCase {
+@RunWith(JUnit4.class)
+public final class RuleFactoryTest extends PackageLoadingTestCase {
 
   private ConfiguredRuleClassProvider provider = TestRuleClassProvider.getRuleClassProvider();
-  private RuleFactory ruleFactory = new RuleFactory(provider);
+  private final RuleFactory ruleFactory = new RuleFactory(provider);
+
+  private static final ImmutableList<StarlarkThread.CallStackEntry> DUMMY_STACK =
+      ImmutableList.of(
+          new StarlarkThread.CallStackEntry(
+              "<toplevel>", Location.fromFileLineColumn("BUILD", 42, 1)),
+          new StarlarkThread.CallStackEntry("foo", Location.fromFileLineColumn("foo.bzl", 10, 1)),
+          new StarlarkThread.CallStackEntry(
+              "myrule", Location.fromFileLineColumn("bar.bzl", 30, 6)));
 
-  public static final Location LOCATION_42 = Location.fromFileAndOffsets(null, 42, 42);
+  private Package.Builder newBuilder(PackageIdentifier id, Path filename) {
+    return packageFactory
+        .newPackageBuilder(
+            id, "TESTING", StarlarkSemantics.DEFAULT, /* repositoryMapping= */ ImmutableMap.of())
+        .setFilename(RootedPath.toRootedPath(root, filename));
+  }
 
+  @Test
   public void testCreateRule() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
-    Package.Builder pkgBuilder =
-        new Package.Builder(PackageIdentifier.createInDefaultRepo("mypkg"), "TESTING")
-            .setFilename(myPkgPath)
-            .setMakeEnv(new MakeEnvironment.Builder());
+    Path myPkgPath = scratch.resolve("/workspace/mypkg/BUILD");
+    Package.Builder pkgBuilder = newBuilder(PackageIdentifier.createInMainRepo("mypkg"), myPkgPath);
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("name", "foo");
     attributeValues.put("alwayslink", true);
 
+    RuleClass ruleClass = provider.getRuleClassMap().get("cc_library");
     Rule rule =
         RuleFactory.createAndAddRule(
             pkgBuilder,
-            provider.getRuleClassMap().get("cc_library"),
-            attributeValues,
-            new Reporter(),
-            /*ast=*/ null,
-            LOCATION_42,
-            /*env=*/ null);
+            ruleClass,
+            new BuildLangTypedAttributeValuesMap(attributeValues),
+            new Reporter(new EventBus()),
+            StarlarkSemantics.DEFAULT,
+            DUMMY_STACK);
 
-    assertSame(rule, rule.getAssociatedRule());
+    assertThat(rule.getAssociatedRule()).isSameInstanceAs(rule);
 
     // pkg.getRules() = [rule]
     Package pkg = pkgBuilder.build();
     assertThat(Sets.newHashSet(pkg.getTargets(Rule.class))).hasSize(1);
-    assertEquals(rule, pkg.getTargets(Rule.class).iterator().next());
+    assertThat(pkg.getTargets(Rule.class).iterator().next()).isEqualTo(rule);
 
-    assertSame(rule, pkg.getTarget("foo"));
+    assertThat(pkg.getTarget("foo")).isSameInstanceAs(rule);
 
-    assertEquals(Label.parseAbsolute("//mypkg:foo"), rule.getLabel());
-    assertEquals("foo", rule.getName());
+    assertThat(rule.getLabel()).isEqualTo(Label.parseAbsolute("//mypkg:foo", ImmutableMap.of()));
+    assertThat(rule.getName()).isEqualTo("foo");
 
-    assertEquals("cc_library", rule.getRuleClass());
-    assertEquals("cc_library rule", rule.getTargetKind());
-    assertEquals(42, rule.getLocation().getStartOffset());
-    assertNull(rule.getSyntaxTree());
-    assertFalse(rule.containsErrors());
+    assertThat(rule.getRuleClass()).isEqualTo("cc_library");
+    assertThat(rule.getTargetKind()).isEqualTo("cc_library rule");
+    // The rule reports the location of the outermost call (aka generator), in the BUILD file.
+    // Thie behavior was added to fix b/23974287, but it loses informtion and is redundant
+    // w.r.t. generator_location. A better fix to that issue would be to keep rule.location as
+    // the innermost call, and to report the entire call stack at the first error for the rule.
+    assertThat(rule.getLocation().file()).isEqualTo("BUILD");
+    assertThat(rule.getLocation().line()).isEqualTo(42);
+    assertThat(rule.getLocation().column()).isEqualTo(1);
+    assertThat(rule.containsErrors()).isFalse();
 
     // Attr with explicitly-supplied value:
     AttributeMap attributes = RawAttributeMapper.of(rule);
-    assertTrue(attributes.get("alwayslink", Type.BOOLEAN));
-    try {
-      attributes.get("alwayslink", Type.STRING); // type error: boolean, not string!
-      fail();
-    } catch (Exception e) {
-      /* Class of exception and error message are not specified by API. */
-    }
-    try {
-      attributes.get("nosuchattr", Type.STRING); // no such attribute
-      fail();
-    } catch (Exception e) {
-      /* Class of exception and error message are not specified by API. */
-    }
+    assertThat(attributes.get("alwayslink", Type.BOOLEAN)).isTrue();
+    assertThrows(Exception.class, () -> attributes.get("alwayslink", Type.STRING));
+    assertThrows(Exception.class, () -> attributes.get("nosuchattr", Type.STRING));
 
     // Attrs with default values:
     // cc_library linkstatic default=0 according to build encyc.
-    assertFalse(attributes.get("linkstatic", Type.BOOLEAN));
-    assertFalse(attributes.get("testonly", Type.BOOLEAN));
+    assertThat(attributes.get("linkstatic", Type.BOOLEAN)).isFalse();
+    assertThat(attributes.get("testonly", Type.BOOLEAN)).isFalse();
     assertThat(attributes.get("srcs", BuildType.LABEL_LIST)).isEmpty();
   }
 
+  @Test
   public void testCreateWorkspaceRule() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE");
-    Package.Builder pkgBuilder = Package.newExternalPackageBuilder(myPkgPath, "TESTING");
+    Path myPkgPath = scratch.resolve("/workspace/WORKSPACE");
+    Package.Builder pkgBuilder =
+        packageFactory.newExternalPackageBuilder(
+            RootedPath.toRootedPath(root, myPkgPath), "TESTING", StarlarkSemantics.DEFAULT);
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("name", "foo");
     attributeValues.put("actual", "//foo:bar");
 
+    RuleClass ruleClass = provider.getRuleClassMap().get("bind");
     Rule rule =
         RuleFactory.createAndAddRule(
             pkgBuilder,
-            provider.getRuleClassMap().get("bind"),
-            attributeValues,
-            new Reporter(),
-            /*ast=*/ null,
-            Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
-            /*env=*/ null);
-    assertFalse(rule.containsErrors());
+            ruleClass,
+            new BuildLangTypedAttributeValuesMap(attributeValues),
+            new Reporter(new EventBus()),
+            StarlarkSemantics.DEFAULT,
+            DUMMY_STACK);
+    assertThat(rule.containsErrors()).isFalse();
   }
 
+  @Test
   public void testWorkspaceRuleFailsInBuildFile() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
-    Package.Builder pkgBuilder =
-        new Package.Builder(PackageIdentifier.createInDefaultRepo("mypkg"), "TESTING")
-            .setFilename(myPkgPath)
-            .setMakeEnv(new MakeEnvironment.Builder());
+    Path myPkgPath = scratch.resolve("/workspace/mypkg/BUILD");
+    Package.Builder pkgBuilder = newBuilder(PackageIdentifier.createInMainRepo("mypkg"), myPkgPath);
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("name", "foo");
     attributeValues.put("actual", "//bar:baz");
 
-    try {
-      RuleFactory.createAndAddRule(
-          pkgBuilder,
-          provider.getRuleClassMap().get("bind"),
-          attributeValues,
-          new Reporter(),
-          /*ast=*/ null,
-          LOCATION_42,
-          /*env=*/ null);
-      fail();
-    } catch (RuleFactory.InvalidRuleException e) {
-      assertThat(e.getMessage()).contains("must be in the WORKSPACE file");
-    }
+    RuleClass ruleClass = provider.getRuleClassMap().get("bind");
+    RuleFactory.InvalidRuleException e =
+        assertThrows(
+            RuleFactory.InvalidRuleException.class,
+            () ->
+                RuleFactory.createAndAddRule(
+                    pkgBuilder,
+                    ruleClass,
+                    new BuildLangTypedAttributeValuesMap(attributeValues),
+                    new Reporter(new EventBus()),
+                    StarlarkSemantics.DEFAULT,
+                    DUMMY_STACK));
+    assertThat(e).hasMessageThat().contains("must be in the WORKSPACE file");
   }
 
+  @Test
   public void testBuildRuleFailsInWorkspaceFile() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE");
-    Package.Builder pkgBuilder =
-        new Package.Builder(PackageIdentifier.createInDefaultRepo("mypkg"), "TESTING")
-            .setFilename(myPkgPath)
-            .setMakeEnv(new MakeEnvironment.Builder());
+    Path myPkgPath = scratch.resolve("/workspace/WORKSPACE");
+    Package.Builder pkgBuilder = newBuilder(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER, myPkgPath);
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("name", "foo");
     attributeValues.put("alwayslink", true);
 
-    try {
-      RuleFactory.createAndAddRule(
-          pkgBuilder,
-          provider.getRuleClassMap().get("cc_library"),
-          attributeValues,
-          new Reporter(),
-          /*ast=*/ null,
-          Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
-          /*env=*/ null);
-      fail();
-    } catch (RuleFactory.InvalidRuleException e) {
-      assertThat(e.getMessage()).contains("cannot be in the WORKSPACE file");
-    }
+    RuleClass ruleClass = provider.getRuleClassMap().get("cc_library");
+    RuleFactory.InvalidRuleException e =
+        assertThrows(
+            RuleFactory.InvalidRuleException.class,
+            () ->
+                RuleFactory.createAndAddRule(
+                    pkgBuilder,
+                    ruleClass,
+                    new BuildLangTypedAttributeValuesMap(attributeValues),
+                    new Reporter(new EventBus()),
+                    StarlarkSemantics.DEFAULT,
+                    DUMMY_STACK));
+    assertThat(e).hasMessageThat().contains("cannot be in the WORKSPACE file");
   }
 
   private void assertAttr(RuleClass ruleClass, String attrName, Type<?> type) throws Exception {
-    assertTrue(
-        "Rule class '"
-            + ruleClass.getName()
-            + "' should have attribute '"
-            + attrName
-            + "' of type '"
-            + type
-            + "'",
-        ruleClass.hasAttr(attrName, type));
+    assertWithMessage(
+            "Rule class '"
+                + ruleClass.getName()
+                + "' should have attribute '"
+                + attrName
+                + "' of type '"
+                + type
+                + "'")
+        .that(ruleClass.hasAttr(attrName, type))
+        .isTrue();
   }
 
+  @Test
   public void testOutputFileNotEqualDot() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo");
-    Package.Builder pkgBuilder =
-        new Package.Builder(PackageIdentifier.createInDefaultRepo("mypkg"), "TESTING")
-            .setFilename(myPkgPath)
-            .setMakeEnv(new MakeEnvironment.Builder());
+    Path myPkgPath = scratch.resolve("/workspace/mypkg");
+    Package.Builder pkgBuilder = newBuilder(PackageIdentifier.createInMainRepo("mypkg"), myPkgPath);
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("outs", Lists.newArrayList("."));
     attributeValues.put("name", "some");
-    try {
-      RuleFactory.createAndAddRule(
-          pkgBuilder,
-          provider.getRuleClassMap().get("genrule"),
-          attributeValues,
-          new Reporter(),
-          /*ast=*/ null,
-          Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
-          /*env=*/ null);
-      fail();
-    } catch (RuleFactory.InvalidRuleException e) {
-      assertTrue(e.getMessage(), e.getMessage().contains("output file name can't be equal '.'"));
-    }
+    RuleClass ruleClass = provider.getRuleClassMap().get("genrule");
+    RuleFactory.InvalidRuleException e =
+        assertThrows(
+            RuleFactory.InvalidRuleException.class,
+            () ->
+                RuleFactory.createAndAddRule(
+                    pkgBuilder,
+                    ruleClass,
+                    new BuildLangTypedAttributeValuesMap(attributeValues),
+                    new Reporter(new EventBus()),
+                    StarlarkSemantics.DEFAULT,
+                    DUMMY_STACK));
+    assertWithMessage(e.getMessage())
+        .that(e.getMessage().contains("output file name can't be equal '.'"))
+        .isTrue();
   }
 
-  /**
-   * Tests mandatory attribute definitions for test rules.
-   */
+  /** Tests mandatory attribute definitions for test rules. */
   // TODO(ulfjack): Remove this check when we switch over to the builder
   // pattern, which will always guarantee that these attributes are present.
+  @Test
   public void testTestRules() throws Exception {
-    Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
-    Package pkg =
-        new Package.Builder(PackageIdentifier.createInDefaultRepo("mypkg"), "TESTING")
-            .setFilename(myPkgPath)
-            .setMakeEnv(new MakeEnvironment.Builder())
-            .build();
+    Path myPkgPath = scratch.resolve("/workspace/mypkg/BUILD");
+    Package pkg = newBuilder(PackageIdentifier.createInMainRepo("mypkg"), myPkgPath).build();
 
     for (String name : ruleFactory.getRuleClassNames()) {
       // Create rule instance directly so we'll avoid mandatory attribute check yet will be able
@@ -228,11 +241,11 @@ public void testTestRules() throws Exception {
       Rule rule =
           new Rule(
               pkg,
-              pkg.createLabel("myrule"),
+              Label.create(pkg.getPackageIdentifier(), "myrule"),
               ruleClass,
-              null,
-              Location.fromFile(myPkgPath),
-              new AttributeContainer(ruleClass));
+              Location.fromFile(myPkgPath.toString()),
+              CallStack.EMPTY,
+              AttributeContainer.newMutableInstance(ruleClass));
       if (TargetUtils.isTestRule(rule)) {
         assertAttr(ruleClass, "tags", Type.STRING_LIST);
         assertAttr(ruleClass, "size", Type.STRING);