Do not depend on PRECOMPUTED:repo_env if the repository doesn't depend on any environment variable.

lberki · copybara-github · commit 80a4a1483e38 · 2024-01-17T11:49:11.000-08:00
HEAD is wasteful because sometimes just checking the up-to-dateness of fetched repositories takes a lot of time (if there is a lot of hashing to be done), so it's better to get a Skyframe cache hit if at all possible.

This is not the full answer because repositories that depend only on unchanged environment variables will still need to be re-checked if there is an environment variable that does change, but it's still a step forward.

RELNOTES: None.
PiperOrigin-RevId: 599242763
Change-Id: I895c5793ed06ef2c7a3337ef232ab13a7596b325
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -308,6 +308,10 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env)
   protected Map<String, String> declareEnvironmentDependencies(
       Map<String, String> markerData, Environment env, Set<String> keys)
       throws InterruptedException {
+    if (keys.isEmpty()) {
+      return ImmutableMap.of();
+    }
+
     ImmutableMap<String, String> envDep = getEnvVarValues(env, keys);
     if (envDep == null) {
       return null;
@@ -349,6 +353,10 @@ public static ImmutableMap<String, String> getEnvVarValues(Environment env, Set<
   protected boolean verifyEnvironMarkerData(
       Map<String, String> markerData, Environment env, Set<String> keys)
       throws InterruptedException {
+    if (keys.isEmpty()) {
+      return true;
+    }
+
     ImmutableMap<String, String> environ = ActionEnvironmentFunction.getEnvironmentView(env, keys);
     if (env.valuesMissing()) {
       return false; // Returns false so caller knows to return immediately
