Skip to content

Commit

Permalink
Do not depend on PRECOMPUTED:repo_env if the repository doesn't depen…
Browse files Browse the repository at this point in the history
…d on any environment variable.

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
  • Loading branch information
lberki authored and copybara-github committed Jan 17, 2024
1 parent c9e4446 commit 80a4a14
Showing 1 changed file with 8 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 80a4a14

Please sign in to comment.