Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator",
"//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:environment_variable_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:repo_environment_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
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.skyframe.ActionEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
import com.google.devtools.build.lib.skyframe.EnvironmentVariableValue;
import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.DirectoryListingValue;
import com.google.devtools.build.lib.skyframe.DirectoryTreeDigestValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
Expand Down Expand Up @@ -625,17 +625,18 @@ public String toStringInternal() {

@Override
public SkyKey getSkyKey(BlazeDirectories directories) {
return ActionEnvironmentFunction.key(name);
return RepoEnvironmentFunction.key(name);
}

@Override
public Optional<String> isOutdated(
Environment env, BlazeDirectories directories, @Nullable String oldValue)
throws InterruptedException {
String v = PrecomputedValue.REPO_ENV.get(env).get(name);
if (v == null) {
v = ((ClientEnvironmentValue) env.getValue(getSkyKey(directories))).getValue();
var envValue = (EnvironmentVariableValue) env.getValue(getSkyKey(directories));
if (envValue == null) {
return Optional.empty();
}
String v = envValue.value();
// Note that `oldValue` can be null if the env var was not set.
if (!Objects.equals(oldValue, v)) {
return Optional.of(
Expand Down
29 changes: 29 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ java_library(
":cached_bzl_load_value_and_deps_builder_factory",
":client_environment_function",
":client_environment_value",
":environment_variable_value",
":collect_packages_under_directory_function",
":collect_packages_under_directory_value",
":collect_targets_in_package_function",
Expand Down Expand Up @@ -173,6 +174,7 @@ java_library(
":recursive_package_provider_backed_target_pattern_resolver",
":recursive_pkg_function",
":recursive_pkg_value",
":repo_environment_function",
":repo_file_function",
":repo_package_args_function",
":repository_mapping_function",
Expand Down Expand Up @@ -1025,6 +1027,16 @@ java_library(
],
)

java_library(
name = "environment_variable_value",
srcs = ["EnvironmentVariableValue.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:jsr305",
],
)

java_library(
name = "collect_packages_under_directory_function",
srcs = ["CollectPackagesUnderDirectoryFunction.java"],
Expand Down Expand Up @@ -2335,6 +2347,23 @@ java_library(
],
)

java_library(
name = "repo_environment_function",
srcs = ["RepoEnvironmentFunction.java"],
deps = [
":client_environment_function",
":client_environment_value",
":environment_variable_value",
":precomputed_value",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
"//third_party:jsr305",
],
)

java_library(
name = "repo_file_function",
srcs = ["RepoFileFunction.java"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2026 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.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// 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.skyframe;

import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.SkyValue;
import javax.annotation.Nullable;

/**
* The value of an environmental variable from an environment (client env, action env or repository
* env). These are invalidated and injected by {@link SequencedSkyframeExecutor}.
*
* @param value the value in the client environment or null if unset in the environment.
*/
@AutoCodec
public record EnvironmentVariableValue(@Nullable String value) implements SkyValue {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright 2026 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.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// 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.skyframe;

import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableSortedMap;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.annotation.Nullable;

/**
* Skyframe function that provides the effective value for an environment variable in the context of
* repository rules and module extensions. This checks {@code --repo_env} overrides first, then falls
* back to the client environment.
*/
public final class RepoEnvironmentFunction implements SkyFunction {
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
Map<String, String> repoEnv = PrecomputedValue.REPO_ENV.get(env);
String key = (String) skyKey.argument();
if (repoEnv.containsKey(key)) {
return new EnvironmentVariableValue(repoEnv.get(key));
}
// Fall back to the client environment.
ClientEnvironmentValue clientValue =
(ClientEnvironmentValue) env.getValue(ClientEnvironmentFunction.key(key));
if (clientValue == null) {
return null;
}
return new EnvironmentVariableValue(clientValue.getValue());
}

/** Returns the SkyKey to invoke this function for the environment variable {@code variable}. */
public static Key key(String variable) {
return Key.create(variable);
}

@VisibleForSerialization
@AutoCodec
static class Key extends AbstractSkyKey<String> {
private static final SkyKeyInterner<Key> interner = SkyKey.newInterner();

private Key(String arg) {
super(arg);
}

private static Key create(String arg) {
return interner.intern(new Key(arg));
}

@VisibleForSerialization
@AutoCodec.Interner
static Key intern(Key key) {
return interner.intern(key);
}

@Override
public SkyFunctionName functionName() {
return SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE;
}

@Override
public SkyKeyInterner<Key> getSkyKeyInterner() {
return interner;
}
}

/**
* Returns a map of environment variable key => values, getting them from Skyframe. Returns null
* if and only if some dependencies from Skyframe still need to be resolved.
*/
@Nullable
public static ImmutableSortedMap<String, Optional<String>> getEnvironmentView(
Environment env, Set<String> keys) throws InterruptedException {
var skyKeys = Collections2.transform(keys, RepoEnvironmentFunction::key);
SkyframeLookupResult values = env.getValuesAndExceptions(skyKeys);
if (env.valuesMissing()) {
return null;
}

var result = ImmutableSortedMap.<String, Optional<String>>naturalOrder();
for (var key : skyKeys) {
var value = (EnvironmentVariableValue) values.get(key);
if (value == null) {
return null;
}
result.put(key.argument().toString(), Optional.ofNullable(value.value()));
}
return result.buildOrThrow();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public final class SkyFunctions {
static final SkyFunctionName ACTION_SKETCH = SkyFunctionName.createHermetic("ACTION_SKETCH");
public static final SkyFunctionName ACTION_ENVIRONMENT_VARIABLE =
SkyFunctionName.createHermetic("ACTION_ENVIRONMENT_VARIABLE");
public static final SkyFunctionName REPOSITORY_ENVIRONMENT_VARIABLE =
SkyFunctionName.createHermetic("REPOSITORY_ENVIRONMENT_VARIABLE");
public static final SkyFunctionName DIRECTORY_LISTING_STATE =
SkyFunctionName.createNonHermetic("DIRECTORY_LISTING_STATE");
public static final SkyFunctionName DIRECTORY_LISTING =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions() {
map.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction());
map.put(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction(clientEnv));
map.put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction());
map.put(SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE, new RepoEnvironmentFunction());
map.put(FileStateKey.FILE_STATE, newFileStateFunction());
map.put(SkyFunctions.DIRECTORY_LISTING_STATE, newDirectoryListingStateFunction());
map.put(FileSymlinkCycleUniquenessFunction.NAME, new FileSymlinkCycleUniquenessFunction());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:repo_environment_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -198,6 +199,7 @@ public void handle(Event event) {}
SkyFunctions.DIRECTORY_LISTING_STATE,
new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE))
.put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction())
.put(SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE, new RepoEnvironmentFunction())
.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction())
.put(
SkyFunctions.LOCAL_REPOSITORY_LOOKUP,
Expand Down
68 changes: 68 additions & 0 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,74 @@ function test_starlark_repository_environ_invalidation_action_env_batch() {
environ_invalidation_action_env_test_template --batch
}

# Cherry-picked from https://github.com/bazelbuild/bazel/pull/29946. On master,
# the repository environment used to be a single whole-map Skyframe node, so
# changing one environment variable invalidated every repository rule and module
# extension, even ones that didn't depend on it; that PR fixed the over-invalidation.
#
# That bug never applied to 8.x, so #29946's production fix is not ported: the
# narrow repo-env port on this branch injects only `repoEnvFromOptions` into
# `REPO_ENV` and otherwise falls back to the already per-variable
# `ClientEnvironmentFunction`, so an unrelated client variable never mutates the
# whole-map node. This test is kept here to prove that.
#
# The repo here only depends on FOO, yet an interleaved `mod` invocation with an
# unrelated variable OTHER changed must not cause it to be refetched.
function test_unrelated_env_var_does_not_invalidate_repo() {
local execution_file="${TEST_TMPDIR}/execution"
echo 0 > "${execution_file}"

cat > $(setup_module_dot_bazel) <<'EOF'
ext = use_extension("//:ext.bzl", "ext")
use_repo(ext, "foo")
EOF
cat > ext.bzl <<EOF
def _repo_impl(repository_ctx):
# Declares a dependency on FOO only.
foo = repository_ctx.getenv("FOO")
count = int(repository_ctx.execute(["cat", "${execution_file}"]).stdout.strip()) + 1
repository_ctx.execute(["bash", "-c", "echo %s > ${execution_file}" % count])
repository_ctx.file(
"BUILD", "filegroup(name = 'bar', visibility = ['//visibility:public'])")
repository_ctx.file("REPO.bazel", "")

# A local repo is refetched whenever its node is recomputed, which makes spurious
# invalidation directly observable through the fetch counter.
repo = repository_rule(implementation = _repo_impl, local = True)

def _ext_impl(module_ctx):
repo(name = "foo")

ext = module_extension(implementation = _ext_impl)
EOF
cat > BUILD <<'EOF'
filegroup(name = "t", srcs = ["@foo//:bar"])
EOF

# Initial fetch.
FOO=1 OTHER=a bazel build //:t >& $TEST_log || fail "Failed to build"
assert_equals 1 $(cat "${execution_file}")
# Rebuilding with the same environment must not refetch.
FOO=1 OTHER=a bazel build //:t >& $TEST_log || fail "Failed to build"
assert_equals 1 $(cat "${execution_file}")

# Evaluate the whole module/repo graph with an unrelated variable (OTHER)
# changed, then build again with the original environment. The repo only reads
# FOO, so it must not be refetched.
FOO=1 OTHER=b bazel mod graph >& $TEST_log || fail "Failed to run mod"
FOO=1 OTHER=a bazel build //:t >& $TEST_log || fail "Failed to build"
assert_equals 1 $(cat "${execution_file}")

# A second interleaving with yet another value must likewise not refetch.
FOO=1 OTHER=c bazel mod graph >& $TEST_log || fail "Failed to run mod"
FOO=1 OTHER=a bazel build //:t >& $TEST_log || fail "Failed to build"
assert_equals 1 $(cat "${execution_file}")

# Sanity check: changing FOO, which the repo does depend on, must refetch.
FOO=2 OTHER=a bazel build //:t >& $TEST_log || fail "Failed to build"
assert_equals 2 $(cat "${execution_file}")
}

# Test invalidation based on change to the bzl files
function bzl_invalidation_test_template() {
local startup_flag="${1-}"
Expand Down
Loading