Address review comments

bazelbuild · Aug 23, 2023 · bf9be0b · bf9be0b
1 parent 6b3d239
commit bf9be0b
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java
@@ -102,8 +102,6 @@ public SkyValue compute(SkyKey skyKey, Environment env)
               remoteRepoSpecs.buildOrThrow());
     }
 
-    Profiler.instance().profile(ProfilerTask.BZLMOD, "module resolution completed").close();
-
     return BazelModuleResolutionValue.create(
         finalDepGraph, state.selectionResult.getUnprunedDepGraph());
   }
@@ -247,7 +245,8 @@ private static void checkNoYankedVersions(ImmutableMap<ModuleKey, InterimModule>
     }
   }
 
-  private static RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) {
+  private static RepoSpec maybeAppendAdditionalPatches(
+      @Nullable RepoSpec repoSpec, @Nullable ModuleOverride override) {
     if (!(override instanceof SingleVersionOverride)) {
       return repoSpec;
     }
@@ -270,9 +269,12 @@ private static RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOv
   /**
    * Builds a {@link Module} from an {@link InterimModule}, discarding unnecessary fields and adding
    * extra necessary ones (such as the repo spec).
+   *
+   * @param remoteRepoSpec the {@link RepoSpec} for the module obtained from a registry or null if
+   *     the module has a non-registry override
    */
   static Module moduleFromInterimModule(
-      InterimModule interim, ModuleOverride override, RepoSpec remoteRepoSpec) {
+      InterimModule interim, @Nullable ModuleOverride override, @Nullable RepoSpec remoteRepoSpec) {
     return Module.builder()
         .setName(interim.getName())
         .setVersion(interim.getVersion())

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpecFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpecFunction.java
@@ -42,23 +42,21 @@ public RepoSpecFunction(RegistryFactory registryFactory) {
   @Nullable
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws InterruptedException, RepoSpecException {
-    RepoSpecKey module = (RepoSpecKey) skyKey.argument();
+    RepoSpecKey key = (RepoSpecKey) skyKey.argument();
     try (SilentCloseable c =
         Profiler.instance()
-            .profile(ProfilerTask.BZLMOD, () -> "compute repo spec: " + module.getModuleKey())) {
+            .profile(ProfilerTask.BZLMOD, () -> "compute repo spec: " + key.getModuleKey())) {
       return registryFactory
-          .getRegistryWithUrl(module.getRegistryUrl())
+          .getRegistryWithUrl(key.getRegistryUrl())
           .getRepoSpec(
-              module.getModuleKey(),
-              module.getModuleKey().getCanonicalRepoName(),
-              env.getListener());
+              key.getModuleKey(), key.getModuleKey().getCanonicalRepoName(), env.getListener());
     } catch (IOException e) {
       throw new RepoSpecException(
           ExternalDepsException.withCauseAndMessage(
               FailureDetails.ExternalDeps.Code.ERROR_ACCESSING_REGISTRY,
               e,
               "Unable to get module repo spec for %s from registry",
-              module.getModuleKey()));
+              key.getModuleKey()));
     } catch (URISyntaxException e) {
       // This should never happen since we obtain the registry URL from an already constructed
       // registry.