python: Respect --legacy_external_runfiles for runfiles.

This was found by way of the upb downstream tests, which currently rely on the
legacy external runfiles format for runfiles (this is when files that come from
other repositories are put in `$runfilesRoot/$mainRepo/external/$otherRepo`).

What happens is a regular `runfiles.merge(other)` call will, under the hood,
create a new Runfiles.Builder object with legacyExternalRunfiles=false, thus
losing the value from the flag. To fix, we just have to create a builder with
the value from the flag so that the created Runfiles object keeps it.

Workaround for #17415

PiperOrigin-RevId: 507503578
Change-Id: I961f50f73ae1ee9980da92eff1b333cbc3b82104

src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java

@@ -323,6 +323,31 @@ public Object mergeRunfilesWithGeneratedInitsEmptyFilesSupplier(
         .build();
   }
 
+  // TODO(https://github.com/bazelbuild/bazel/issues/17415): Remove this method one
+  // --legacy_external_runfiles is defaulted to false
+  @StarlarkMethod(
+      name = "make_runfiles_respect_legacy_external_runfiles",
+      doc =
+          "Like ctx.runfiles().merge(), except the --legacy_external_runfiles flag "
+              + "is respected, otherwise files in other repos don't have the legacy "
+              + " external/ path show up; see https://github.com/bazelbuild/bazel/issues/17415",
+      parameters = {
+        @Param(name = "ctx", positional = true, named = true, defaultValue = "unbound"),
+        @Param(
+            name = "runfiles",
+            positional = true,
+            named = true,
+            defaultValue = "unbound",
+            doc = "Runfiles to include"),
+      })
+  public Object mergeAllRunfilesRespectExternalLegacyRunfiles(
+      StarlarkRuleContext starlarkCtx, Runfiles runfiles) throws EvalException {
+    return new Runfiles.Builder(
+            starlarkCtx.getWorkspaceName(), starlarkCtx.getConfiguration().legacyExternalRunfiles())
+        .merge(runfiles)
+        .build();
+  }
+
   @StarlarkMethod(
       name = "declare_constant_metadata_file",
       doc = "Declare a file that always reports it is unchanged.",

src/main/starlark/builtins_bzl/common/python/py_executable.bzl

@@ -734,8 +734,14 @@ def _create_providers(
         DefaultInfo(
             executable = executable,
             files = files_to_build,
-            default_runfiles = runfiles_details.default_runfiles,
-            data_runfiles = runfiles_details.data_runfiles,
+            default_runfiles = _py_builtins.make_runfiles_respect_legacy_external_runfiles(
+                ctx,
+                runfiles_details.default_runfiles,
+            ),
+            data_runfiles = _py_builtins.make_runfiles_respect_legacy_external_runfiles(
+                ctx,
+                runfiles_details.data_runfiles,
+            ),
         ),
         create_instrumented_files_info(ctx),
         _create_run_environment_info(ctx, inherited_environment),

src/main/starlark/builtins_bzl/common/python/py_internal.bzl

@@ -34,4 +34,5 @@ py_internal = struct(
     is_singleton_depset = _py_builtins.is_singleton_depset,
     merge_runfiles_with_generated_inits_empty_files_supplier = _py_builtins.merge_runfiles_with_generated_inits_empty_files_supplier,
     new_py_cc_link_params_provider = _py_builtins.new_py_cc_link_params_provider,
+    make_runfiles_respect_legacy_external_runfiles = _py_builtins.make_runfiles_respect_legacy_external_runfiles,
 )

src/test/shell/bazel/python_version_test.sh

@@ -344,4 +344,54 @@ EOF
   expect_log "Tool output"
 }
 
+function test_external_runfiles() {
+  cat >> WORKSPACE <<EOF
+local_repository(
+  name = "repo2",
+  path = "repo2"
+)
+EOF
+  mkdir repo2
+  touch repo2/WORKSPACE
+  cat > repo2/BUILD <<EOF
+package(default_visibility=["//visibility:public"])
+filegroup(name="r2files", srcs=["r2.txt"])
+EOF
+  touch repo2/r2.txt
+
+  mkdir py
+  cat > py/BUILD <<EOF
+py_binary(
+  name = "foo", srcs=["foo.py"],
+  data = ["@repo2//:r2files"],
+)
+EOF
+  touch py/foo.py
+
+  # We're testing for this flag's behavior, so force it to true.
+  # TODO(https://github.com/bazelbuild/bazel/issues/12821): Remove this test
+  # when this behavior is removed
+  bazel build --legacy_external_runfiles=true //py:foo
+  if "$is_windows"; then
+    exe=".exe"
+  else
+    exe=""
+  fi
+
+  # NOTE: The "main" name isn't special. It's just the name the integration test
+  # setup puts in WORKSPACE.
+  cp bazel-bin/py/foo$exe.runfiles_manifest runfiles_manifest
+  assert_contains main/external/repo2/r2.txt runfiles_manifest \
+    "runfiles manifest didn't have external path mapping"
+
+  # By default, Python binaries are put into zip files on Windows and don't
+  # have a real runfiles tree.
+  if ! "$is_windows"; then
+    find bazel-bin/py/foo.runfiles > runfiles_listing
+    assert_contains bazel-bin/py/foo.runfiles/main/external/repo2/r2.txt \
+      runfiles_listing \
+      "runfiles didn't have external links"
+  fi
+}
+
 run_suite "Tests for how the Python rules handle Python 2 vs Python 3"