Stop using Rule+Package for repos

src/MODULE.tools

@@ -12,7 +12,8 @@ bazel_dep(name = "zlib", version = "1.3.1.bcr.5")
 bazel_dep(name = "rules_proto", version = "7.0.2")
 
 # Enforce a minimal required version
-bazel_dep(name = "bazel_features", version = "1.30.0")
+bazel_dep(name = "bazel_features", version = "1.30.0", repo_name = None)
+bazel_dep(name = "rules_jvm_external", version = "6.5", repo_name = None)
 
 xcode_configure = use_extension("//tools/osx:xcode_configure.bzl", "xcode_configure_extension")
 use_repo(xcode_configure, "local_config_xcode")

src/main/java/com/google/devtools/build/lib/bazel/BUILD

@@ -20,7 +20,10 @@ filegroup(
 
 java_library(
     name = "repository_module",
-    srcs = ["BazelRepositoryModule.java"],
+    srcs = [
+        "BazelRepositoryModule.java",
+        "CacheHitReportingModule.java",
+    ],
     deps = [
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
@@ -41,13 +44,15 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_fetch_function",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
+        "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache:events",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/profiler",
+        "//src/main/java/com/google/devtools/build/lib/repository:repository_events",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_dirtiness_checker",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
@@ -59,6 +64,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:os",
+        "//src/main/java/com/google/devtools/build/lib/util:pair",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",
@@ -182,7 +188,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:bazel_lockfile_module",
         "//src/main/java/com/google/devtools/build/lib/bazel/coverage",
         "//src/main/java/com/google/devtools/build/lib/bazel/debug:workspace-rule-module",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
         "//src/main/java/com/google/devtools/build/lib/bazel/rules",
         "//src/main/java/com/google/devtools/build/lib/buildeventservice",

src/main/java/com/google/devtools/build/lib/bazel/Bazel.java

@@ -62,7 +62,7 @@ public final class Bazel {
           com.google.devtools.build.lib.bazel.debug.WorkspaceRuleModule.class,
           com.google.devtools.build.lib.bazel.coverage.BazelCoverageReportModule.class,
           com.google.devtools.build.lib.starlarkdebug.module.StarlarkDebuggerModule.class,
-          com.google.devtools.build.lib.bazel.repository.CacheHitReportingModule.class,
+          CacheHitReportingModule.class,
           com.google.devtools.build.lib.bazel.SpawnLogModule.class,
           com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileModule.class,
           com.google.devtools.build.lib.outputfilter.OutputFilteringModule.class,

src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java → src/main/java/com/google/devtools/build/lib/bazel/CacheHitReportingModule.java

@@ -11,7 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-package com.google.devtools.build.lib.bazel.repository;
+package com.google.devtools.build.lib.bazel;
 
 import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.bazel.repository.cache.DownloadCacheHitEvent;

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveRepoSpecBuilder.java

@@ -15,16 +15,16 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
-
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.List;
-import java.util.Map.Entry;
+import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.StarlarkInt;
+import net.starlark.java.eval.StarlarkList;
 
 /**
  * Builder for a {@link RepoSpec} object that indicates how to materialize a repo corresponding to
@@ -37,15 +37,13 @@ public class ArchiveRepoSpecBuilder {
           Label.parseCanonicalUnchecked("@@bazel_tools//tools/build_defs/repo:http.bzl"),
           "http_archive");
 
-  private final ImmutableMap.Builder<String, Object> attrBuilder;
+  private final Dict.Builder<String, Object> attrBuilder = Dict.builder();
 
-  public ArchiveRepoSpecBuilder() {
-    attrBuilder = new ImmutableMap.Builder<>();
-  }
+  public ArchiveRepoSpecBuilder() {}
 
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setUrls(ImmutableList<String> urls) {
-    attrBuilder.put("urls", urls);
+    attrBuilder.put("urls", StarlarkList.immutableCopyOf(urls));
     return this;
   }
 
@@ -63,32 +61,29 @@ public ArchiveRepoSpecBuilder setStripPrefix(String stripPrefix) {
 
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setPatches(ImmutableList<Label> patches) {
-    attrBuilder.put("patches", patches);
+    attrBuilder.put("patches", StarlarkList.immutableCopyOf(patches));
     return this;
   }
 
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setRemotePatches(ImmutableMap<String, String> remotePatches) {
-    attrBuilder.put("remote_patches", remotePatches);
+    attrBuilder.put("remote_patches", Dict.immutableCopyOf(remotePatches));
     return this;
   }
 
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setOverlay(ImmutableMap<String, RemoteFile> overlay) {
-    ImmutableMap<String, List<String>> remoteFiles =
-        overlay.entrySet().stream()
-            .collect(toImmutableMap(Entry::getKey, e -> e.getValue().urls()));
-    ImmutableMap<String, String> remoteFilesIntegrity =
-        overlay.entrySet().stream()
-            .collect(toImmutableMap(Entry::getKey, e -> e.getValue().integrity()));
-    attrBuilder.put("remote_file_urls", remoteFiles);
-    attrBuilder.put("remote_file_integrity", remoteFilesIntegrity);
+    var remoteFiles = Maps.transformValues(overlay, rf -> StarlarkList.immutableCopyOf(rf.urls()));
+    var remoteFilesIntegrity = Maps.transformValues(overlay, RemoteFile::integrity);
+    attrBuilder.put("remote_file_urls", Dict.immutableCopyOf(remoteFiles));
+    attrBuilder.put("remote_file_integrity", Dict.immutableCopyOf(remoteFilesIntegrity));
     return this;
   }
 
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setRemoteModuleFile(RemoteFile remoteModuleFile) {
-    attrBuilder.put("remote_module_file_urls", remoteModuleFile.urls());
+    attrBuilder.put(
+        "remote_module_file_urls", StarlarkList.immutableCopyOf(remoteModuleFile.urls()));
     attrBuilder.put("remote_module_file_integrity", remoteModuleFile.integrity());
     return this;
   }
@@ -108,7 +103,7 @@ public ArchiveRepoSpecBuilder setArchiveType(String archiveType) {
   }
 
   public RepoSpec build() {
-    return new RepoSpec(HTTP_ARCHIVE, AttributeValues.create(attrBuilder.buildOrThrow()));
+    return new RepoSpec(HTTP_ARCHIVE, AttributeValues.create(attrBuilder.buildImmutable()));
   }
 
   /**

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/AttributeValues.java

@@ -15,89 +15,25 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import static java.util.Collections.singletonList;
-
 import com.google.auto.value.AutoValue;
-import com.google.common.collect.Maps;
-import com.google.devtools.build.lib.cmdline.Label;
-import java.util.Collection;
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 import net.starlark.java.eval.Dict;
-import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Starlark;
 
-/** Wraps a dictionary of attribute names and values. Always uses a dict to represent them */
+/**
+ * Wrapper around a dictionary of attribute names and values to facilitate de/serialization (see
+ * {@link AttributeValuesAdapter}).
+ *
+ * <p>Note that all attribute values are stored as Starlark values (that is, {@link Starlark#valid}
+ * -- so {@link net.starlark.java.eval.StarlarkInt} vs {@code int}, {@link
+ * net.starlark.java.eval.StarlarkList} vs {@link List}, etc).
+ */
 @AutoValue
 public abstract class AttributeValues {
 
   public static AttributeValues create(Dict<String, Object> attribs) {
     return new AutoValue_AttributeValues(attribs);
   }
 
-  public static AttributeValues create(Map<String, Object> attribs) {
-    return new AutoValue_AttributeValues(
-        Dict.immutableCopyOf(Maps.transformValues(attribs, AttributeValues::valueToStarlark)));
-  }
-
   public abstract Dict<String, Object> attributes();
-
-  public static void validateAttrs(AttributeValues attributes, String where, String what)
-      throws EvalException {
-    for (var entry : attributes.attributes().entrySet()) {
-      validateSingleAttr(entry.getKey(), entry.getValue(), where, what);
-    }
-  }
-
-  public static void validateSingleAttr(
-      String attrName, Object attrValue, String where, String what) throws EvalException {
-    var maybeNonVisibleLabel = getFirstNonVisibleLabel(attrValue);
-    if (maybeNonVisibleLabel.isEmpty()) {
-      return;
-    }
-    Label label = maybeNonVisibleLabel.get();
-    String repoName = label.getRepository().getName();
-    throw Starlark.errorf(
-        "no repository visible as '@%s' %s, but referenced by label '@%s//%s:%s' in"
-            + " attribute '%s'%s.",
-        repoName,
-        where,
-        repoName,
-        label.getPackageName(),
-        label.getName(),
-        attrName,
-        what.isEmpty() ? "" : " of " + what);
-  }
-
-  private static Optional<Label> getFirstNonVisibleLabel(Object nativeAttrValue) {
-    Collection<?> toValidate =
-        switch (nativeAttrValue) {
-          case List<?> list -> list;
-          case Map<?, ?> map -> map.keySet();
-          case null, default -> singletonList(nativeAttrValue);
-        };
-    for (var item : toValidate) {
-      if (item instanceof Label label && !label.getRepository().isVisible()) {
-        return Optional.of(label);
-      }
-    }
-    return Optional.empty();
-  }
-
-  // TODO(salmasamy) this is a copy of Attribute::valueToStarlark, Maybe think of a better place?
-  private static Object valueToStarlark(Object x) {
-    // Is x a non-empty string_list_dict?
-    if (x instanceof Map<?, ?> map) {
-      if (!map.isEmpty() && map.values().iterator().next() instanceof List) {
-        Dict.Builder<Object, Object> dict = Dict.builder();
-        for (Map.Entry<?, ?> e : map.entrySet()) {
-          dict.put(e.getKey(), Starlark.fromJava(e.getValue(), null));
-        }
-        return dict.buildImmutable();
-      }
-    }
-    // For all other attribute values, shallow conversion is safe.
-    return Starlark.fromJava(x, null);
-  }
 }