From ec2ebbaf1a901cf2adedc0d82c3a3ff29720f0ce Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 27 Dec 2023 12:06:47 +0100 Subject: [PATCH] Disable `--legacy_external_runfiles` in Bazel tests Prepares for the flip (https://github.com/bazelbuild/bazel/issues/12821) by ensuring that Bazel itself doesn't regress. --- .../builtins_bzl/bazel/java/bazel_java_binary.bzl | 3 +-- .../lib/analysis/LocationExpanderIntegrationTest.java | 3 ++- .../lib/analysis/RunfilesRepoMappingManifestTest.java | 3 +-- .../build/lib/analysis/util/AnalysisTestCase.java | 10 ++++++++-- src/test/py/bazel/test_base.py | 1 + src/test/shell/bazel/bazel_java_test.sh | 1 + src/test/shell/bazel/external_integration_test.sh | 4 ++-- src/test/shell/testenv.sh.tmpl | 3 +++ 8 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl b/src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl index 5ee25b8dc43083..b0cdf933f8695d 100644 --- a/src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl +++ b/src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl @@ -251,8 +251,7 @@ def _create_windows_exe_launcher(ctx, java_executable, classpath, main_class, jv launch_info.add(main_class, format = "java_start_class=%s") launch_info.add_joined(classpath, map_each = _short_path, join_with = ";", format_joined = "classpath=%s", omit_if_empty = False) launch_info.add_joined(jvm_flags_for_launcher, join_with = "\t", format_joined = "jvm_flags=%s", omit_if_empty = False) - jar_bin_path = semantics.find_java_runtime_toolchain(ctx).java_home + "/bin/jar.exe" - launch_info.add(jar_bin_path, format = "jar_bin_path=%s") + launch_info.add(semantics.find_java_runtime_toolchain(ctx).java_home_runfiles_path, format = "jar_bin_path=%s/bin/jar.exe") # TODO(b/295221112): Change to use the "launcher" attribute (only windows use a fixed _launcher attribute) launcher_artifact = ctx.executable._launcher diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java index 951f810f148334..10a0b4aef58f2a 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java @@ -155,7 +155,7 @@ public void otherPathExpansion() throws Exception { } @Test - public void otherPathExternalExpansion() throws Exception { + public void otherPathExternalExpansionLegacyExternalRunfiles() throws Exception { scratch.file( "expansion/BUILD", "sh_library(name='lib', srcs=['@r//p:foo'])"); @@ -171,6 +171,7 @@ public void otherPathExternalExpansion() throws Exception { scratch.file("/r/WORKSPACE", "workspace(name = 'r')"); scratch.file("/r/p/BUILD", "genrule(name='foo', outs=['foo.txt'], cmd='never executed')"); + useConfiguration("--legacy_external_runfiles"); LocationExpander expander = makeExpander("//expansion:lib"); assertThat(expander.expand("foo $(execpath @r//p:foo) bar")) .matches("foo .*-out/.*/external/r/p/foo\\.txt bar"); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java index 21cd1832c3ba50..523d6fec469193 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java @@ -371,9 +371,8 @@ public void hasMappingForSymlinks() throws Exception { .map(PathFragment::getPathString) .collect(toImmutableList()); assertThat(runfilesPaths) - .containsExactly( + .containsAtLeast( "aaa~1.0/aaa", - getRuleClassProvider().getRunfilesPrefix() + "/external/aaa~1.0/aaa", getRuleClassProvider().getRunfilesPrefix() + "/path/to/pkg/symlink", "symlinks~1.0/path/to/pkg/root_symlink", "_repo_mapping"); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index d4892a269d655b..9240bfe0e996f4 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -128,7 +128,9 @@ public enum Flag { // Flags from TestConstants.PRODUCT_SPECIFIC_FLAGS. PRODUCT_SPECIFIC_FLAGS, // The --enable_bzlmod flags. - ENABLE_BZLMOD + ENABLE_BZLMOD, + // The --nolegacy_external_runfiles flag. + NO_LEGACY_EXTERNAL_RUNFILES } /** Helper class to make it easy to enable and disable flags. */ @@ -350,6 +352,9 @@ public void useConfiguration(String... args) throws Exception { } else { optionsParser.parse("--noenable_bzlmod"); } + if (defaultFlags().contains(Flag.NO_LEGACY_EXTERNAL_RUNFILES)) { + optionsParser.parse("--nolegacy_external_runfiles"); + } optionsParser.parse(args); buildOptions = @@ -360,7 +365,8 @@ protected FlagBuilder defaultFlags() { return new FlagBuilder() .with(Flag.PUBLIC_VISIBILITY) .with(Flag.CPU_K8) - .with(Flag.PRODUCT_SPECIFIC_FLAGS); + .with(Flag.PRODUCT_SPECIFIC_FLAGS) + .with(Flag.NO_LEGACY_EXTERNAL_RUNFILES); } protected Action getGeneratingAction(Artifact artifact) { diff --git a/src/test/py/bazel/test_base.py b/src/test/py/bazel/test_base.py index ead5fcc4b9c753..27545aba83248c 100644 --- a/src/test/py/bazel/test_base.py +++ b/src/test/py/bazel/test_base.py @@ -115,6 +115,7 @@ def setUp(self): self._test_cwd = tempfile.mkdtemp(dir=self._tests_root) self._test_bazelrc = os.path.join(self._temp, 'test_bazelrc') with open(self._test_bazelrc, 'wt') as f: + f.write('common --nolegacy_external_runfiles\n') shared_repo_home = os.environ.get('TEST_REPOSITORY_HOME') if shared_repo_home and os.path.exists(shared_repo_home): for repo in self._SHARED_REPOS: diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh index 38f968b9aaaeea..18bcb36c2ef1c4 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -1576,6 +1576,7 @@ EOF chmod +x "${pkg}"/run.sh bazel test //"${pkg}":bar --test_output=all --verbose_failures >& "$TEST_log" \ + --legacy_external_runfiles \ || fail "Expected success" } diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh index e378a7074ccc4b..58cbdfb89d849f 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh @@ -624,7 +624,7 @@ genrule( name = "test_sh", outs = ["test.sh"], srcs = ["@toto//file"], - cmd = "echo '#!/bin/sh' > $@ && echo $(location @toto//file) >> $@", + cmd = "echo '#!/bin/sh' > $@ && echo $(rootpath @toto//file) >> $@", ) EOF @@ -682,7 +682,7 @@ genrule( name = "test_sh", outs = ["test.sh"], srcs = ["@toto//file"], - cmd = "echo '#!/bin/sh' > $@ && echo cat $(location @toto//file) >> $@", + cmd = "echo '#!/bin/sh' > $@ && echo cat $(rootpath @toto//file) >> $@", ) EOF diff --git a/src/test/shell/testenv.sh.tmpl b/src/test/shell/testenv.sh.tmpl index b0abb4959b8a06..745335bb37b0ad 100755 --- a/src/test/shell/testenv.sh.tmpl +++ b/src/test/shell/testenv.sh.tmpl @@ -267,6 +267,9 @@ build --incompatible_use_toolchain_resolution_for_java_rules # Enable Bzlmod in all shell integration tests common --enable_bzlmod +# Verify compatibility before the flip (https://github.com/bazelbuild/bazel/issues/12821) +common --nolegacy_external_runfiles + ${EXTRA_BAZELRC:-} EOF