Skip to content

Commit

Permalink
Disable --legacy_external_runfiles in Bazel tests
Browse files Browse the repository at this point in the history
This requires fixing an actual bug that causes the Windows Java launcher to not find the Java runtime with `--nolegacy_external_runfiles`.

Work towards #12821

Closes #20680.

PiperOrigin-RevId: 601826685
Change-Id: Ib45e60901d47efc06bf875c334edafcaeabc5f1e
  • Loading branch information
fmeum authored and copybara-github committed Jan 26, 2024
1 parent 27419e3 commit 3d4491c
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])");
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -346,6 +348,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 =
Expand All @@ -356,7 +361,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) {
Expand Down
1 change: 1 addition & 0 deletions src/test/py/bazel/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/test/shell/bazel/bazel_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/shell/bazel/external_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions src/test/shell/testenv.sh.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,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

Expand Down

0 comments on commit 3d4491c

Please sign in to comment.