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 index f0be814f5cc65b..16cc990f9d9748 100644 --- 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 @@ -331,9 +331,13 @@ protected Map declareEnvironmentDependencies( return null; } + // Only depend on --repo_env values that are specified in the "environ" attribute. Map repoEnv = new LinkedHashMap(environ); - for (Map.Entry value : repoEnvOverride.entrySet()) { - repoEnv.put(value.getKey(), value.getValue()); + for (String key : keys) { + String value = repoEnvOverride.get(key); + if (value != null) { + repoEnv.put(key, value); + } } // Add the dependencies to the marker file @@ -362,9 +366,13 @@ protected boolean verifyEnvironMarkerData(Map markerData, Enviro return false; } + // Only depend on --repo_env values that are specified in the "environ" attribute. Map repoEnv = new LinkedHashMap<>(environ); - for (Map.Entry value : repoEnvOverride.entrySet()) { - repoEnv.put(value.getKey(), value.getValue()); + for (String key : keys) { + String value = repoEnvOverride.get(key); + if (value != null) { + repoEnv.put(key, value); + } } // Verify that all environment variable in the marker file are also in keys diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 086c6f1249b8e4..0fbbeae8f99abb 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -120,7 +120,7 @@ EOF bazel build //zoo:ball-pit1 >& $TEST_log || fail "Failed to build" expect_log "bleh" expect_log "Tra-la!" # Invalidation - cat bazel-genfiles/zoo/ball-pit1.txt >$TEST_log + cat bazel-bin/zoo/ball-pit1.txt >$TEST_log expect_log "Tra-la!" bazel build //zoo:ball-pit1 >& $TEST_log || fail "Failed to build" @@ -128,7 +128,7 @@ EOF bazel build //zoo:ball-pit2 >& $TEST_log || fail "Failed to build" expect_not_log "Tra-la!" # No invalidation - cat bazel-genfiles/zoo/ball-pit2.txt >$TEST_log + cat bazel-bin/zoo/ball-pit2.txt >$TEST_log expect_log "Tra-la!" # Test invalidation of the WORKSPACE file @@ -154,7 +154,7 @@ EOF bazel build //zoo:ball-pit1 >& $TEST_log || fail "Failed to build" expect_log "blah" expect_log "Tra-la-la!" # Invalidation - cat bazel-genfiles/zoo/ball-pit1.txt >$TEST_log + cat bazel-bin/zoo/ball-pit1.txt >$TEST_log expect_log "Tra-la-la!" bazel build //zoo:ball-pit1 >& $TEST_log || fail "Failed to build" @@ -162,7 +162,7 @@ EOF bazel build //zoo:ball-pit2 >& $TEST_log || fail "Failed to build" expect_not_log "Tra-la-la!" # No invalidation - cat bazel-genfiles/zoo/ball-pit2.txt >$TEST_log + cat bazel-bin/zoo/ball-pit2.txt >$TEST_log expect_log "Tra-la-la!" } @@ -355,7 +355,7 @@ EOF bazel build @foo//:bar >& $TEST_log || fail "Failed to build" expect_log "foo" expect_not_log "Workspace name in .*/WORKSPACE (.*) does not match the name given in the repository's definition (@foo)" - cat bazel-genfiles/external/foo/bar.txt >$TEST_log + cat bazel-bin/external/foo/bar.txt >$TEST_log expect_log "foo" } @@ -892,8 +892,8 @@ build:bar --repo_env=FOO=bar EOF bazel build --config=foo //:repoenv //:unrelated - cp `bazel info bazel-genfiles 2>/dev/null`/repoenv.txt repoenv1.txt - cp `bazel info bazel-genfiles 2> /dev/null`/unrelated.txt unrelated1.txt + cp `bazel info bazel-bin 2>/dev/null`/repoenv.txt repoenv1.txt + cp `bazel info bazel-bin 2> /dev/null`/unrelated.txt unrelated1.txt echo; cat repoenv1.txt; echo; cat unrelated1.txt; echo grep -q 'FOO=foo' repoenv1.txt \ @@ -904,8 +904,8 @@ EOF FOO=CHANGED bazel build --config=foo //:repoenv //:unrelated # nothing should change, as actions don't see FOO and for repositories # the value is fixed by --repo_env - cp `bazel info bazel-genfiles 2>/dev/null`/repoenv.txt repoenv2.txt - cp `bazel info bazel-genfiles 2> /dev/null`/unrelated.txt unrelated2.txt + cp `bazel info bazel-bin 2>/dev/null`/repoenv.txt repoenv2.txt + cp `bazel info bazel-bin 2> /dev/null`/unrelated.txt unrelated2.txt echo; cat repoenv2.txt; echo; cat unrelated2.txt; echo diff repoenv1.txt repoenv2.txt \ @@ -916,8 +916,8 @@ EOF bazel build --config=bar //:repoenv //:unrelated # The new config should be picked up, but the unrelated target should # not be rerun - cp `bazel info bazel-genfiles 3>/dev/null`/repoenv.txt repoenv3.txt - cp `bazel info bazel-genfiles 3> /dev/null`/unrelated.txt unrelated3.txt + cp `bazel info bazel-bin 3>/dev/null`/repoenv.txt repoenv3.txt + cp `bazel info bazel-bin 3> /dev/null`/unrelated.txt unrelated3.txt echo; cat repoenv3.txt; echo; cat unrelated3.txt; echo grep -q 'FOO=bar' repoenv3.txt \ @@ -926,6 +926,49 @@ EOF || fail "Expected unrelated action to not be rerun" } +function test_repo_env_inverse() { + # This test makes sure that a repository rule that has no dependencies on + # environment variables does _not_ get refetched when --repo_env changes. + setup_starlark_repository + + cat > test.bzl <<'EOF' +def _impl(ctx): + # Record a time stamp to verify that the rule is not rerun. + ctx.execute(["bash", "-c", "date +%s >> env.txt"]) + ctx.file("BUILD", 'exports_files(["env.txt"])') + +repo = repository_rule( + implementation = _impl, +) +EOF + cat > BUILD <<'EOF' +genrule( + name = "repoenv", + outs = ["repoenv.txt"], + srcs = ["@foo//:env.txt"], + cmd = "cp $< $@", +) +EOF + cat > .bazelrc </dev/null`/repoenv.txt repoenv1.txt + echo; cat repoenv1.txt; echo; + + sleep 2 # ensure any rerun will have a different time stamp + + bazel build --config=bar //:repoenv + # The new config should not trigger a rerun of repoenv. + cp `bazel info bazel-bin 2>/dev/null`/repoenv.txt repoenv2.txt + echo; cat repoenv2.txt; echo; + + diff repoenv1.txt repoenv2.txt \ + || fail "Expected repository to not change" +} + function test_repo_env_invalidation() { # regression test for https://github.com/bazelbuild/bazel/issues/8869 WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") @@ -962,18 +1005,18 @@ genrule( EOF bazel build //:repotime - cp `bazel info bazel-genfiles 2>/dev/null`/repotime.txt time1.txt + cp `bazel info bazel-bin 2>/dev/null`/repotime.txt time1.txt sleep 2; bazel build --repo_env=foo=bar //:repotime - cp `bazel info bazel-genfiles 2>/dev/null`/repotime.txt time2.txt + cp `bazel info bazel-bin 2>/dev/null`/repotime.txt time2.txt diff time1.txt time2.txt && fail "Expected repo to be refetched" || : bazel shutdown sleep 2; bazel build --repo_env=foo=bar //:repotime - cp `bazel info bazel-genfiles 2>/dev/null`/repotime.txt time3.txt + cp `bazel info bazel-bin 2>/dev/null`/repotime.txt time3.txt diff time2.txt time3.txt || fail "Expected repo to not be refetched" } @@ -1387,9 +1430,9 @@ EOF echo "initial" > reference.txt.shadow bazel build //:source //:configure - grep 'initial' `bazel info bazel-genfiles`/source.txt \ + grep 'initial' `bazel info bazel-bin`/source.txt \ || fail '//:source not generated properly' - grep 'initial' `bazel info bazel-genfiles`/configure.txt \ + grep 'initial' `bazel info bazel-bin`/configure.txt \ || fail '//:configure not generated properly' echo "new value" > reference.txt.shadow @@ -1401,9 +1444,9 @@ EOF && fail "Expected 'source' not to be synced" || : bazel build //:source //:configure - grep -q 'initial' `bazel info bazel-genfiles`/source.txt \ + grep -q 'initial' `bazel info bazel-bin`/source.txt \ || fail '//:source did not keep its old value' - grep -q 'new value' `bazel info bazel-genfiles`/configure.txt \ + grep -q 'new value' `bazel info bazel-bin`/configure.txt \ || fail '//:configure not synced properly' } @@ -1752,9 +1795,9 @@ load("@netrc//:data.bzl", "netrc") ) for name in ["login", "password"]] EOF bazel build //:login //:password - grep 'myusername' `bazel info bazel-genfiles`/login.txt \ + grep 'myusername' `bazel info bazel-bin`/login.txt \ || fail "Username not parsed correctly" - grep 'mysecret' `bazel info bazel-genfiles`/password.txt \ + grep 'mysecret' `bazel info bazel-bin`/password.txt \ || fail "Password not parsed correctly" # Also check the precise value of parsed file @@ -1789,7 +1832,7 @@ genrule( ) EOF bazel build //:check_expected - grep 'OK' `bazel info bazel-genfiles`/check_expected.txt \ + grep 'OK' `bazel info bazel-bin`/check_expected.txt \ || fail "Parsed dict not equal to expected value" } @@ -1895,7 +1938,7 @@ genrule( ) EOF bazel build //:check_expected - grep 'OK' `bazel info bazel-genfiles`/check_expected.txt \ + grep 'OK' `bazel info bazel-bin`/check_expected.txt \ || fail "Authentication merged incorrectly" }