python,runfiles library: don't pick up TEST_SRCDIR

The Python runfiles library will no longer pick up
the runfiles directory from $TEST_SRCDIR.

This is in accordance with the Java runfiles
library, see the motivation in
https://github.com/bazelbuild/bazel/issues/4598

The commit also adds a test for this functionality
as well as exercising that the runfiles library
can find the runfiles directory even if no
RUNFILES_* envvars are exported.

See https://github.com/bazelbuild/bazel/issues/4598
See https://github.com/bazelbuild/bazel/issues/4460

Change-Id: I6e4d154cdca66b29d7c6e77743cf8109638238b5
PiperOrigin-RevId: 185009819

src/test/py/bazel/runfiles_test.py

diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py
index abe23d0..f416904 100644
--- a/src/test/py/bazel/runfiles_test.py
+++ b/src/test/py/bazel/runfiles_test.py
@@ -122,6 +122,53 @@
       self.fail("lines: %s" % lines)
     self.assertEqual(lines[0], "data for bar.py")
 
+  def testPythonRunfilesLibraryFindsRunfilesWithoutEnvvars(self):
+    for s, t in [
+        ("WORKSPACE.mock", "WORKSPACE"),
+        ("bar/BUILD.mock", "bar/BUILD"),
+        ("bar/bar.py", "bar/bar.py"),
+        ("bar/bar-py-data.txt", "bar/bar-py-data.txt"),
+    ]:
+      self.CopyFile(
+          self.Rlocation(
+              "io_bazel/src/test/py/bazel/testdata/runfiles_test/" + s), t)
+
+    exit_code, stdout, stderr = self.RunBazel(["info", "bazel-bin"])
+    self.AssertExitCode(exit_code, 0, stderr)
+    bazel_bin = stdout[0]
+
+    exit_code, _, stderr = self.RunBazel(["build", "//bar:all"])
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    if test_base.TestBase.IsWindows():
+      bin_path = os.path.join(bazel_bin, "bar/bar-py.exe")
+    else:
+      bin_path = os.path.join(bazel_bin, "bar/bar-py")
+
+    self.assertTrue(os.path.exists(bin_path))
+
+    exit_code, stdout, stderr = self.RunProgram(
+        [bin_path],
+        env_remove=set([
+            "RUNFILES_MANIFEST_FILE",
+            "RUNFILES_MANIFEST_ONLY",
+            "RUNFILES_DIR",
+            "JAVA_RUNFILES",
+        ]),
+        env_add={"TEST_SRCDIR": "__ignore_me__"})
+    self.AssertExitCode(exit_code, 0, stderr)
+    if len(stdout) < 2:
+      self.fail("stdout: %s" % stdout)
+    self.assertEqual(stdout[0], "Hello Python Bar!")
+    six.assertRegex(self, stdout[1], "^rloc=.*/bar/bar-py-data.txt")
+    self.assertNotIn("__ignore_me__", stdout[1])
+
+    with open(stdout[1].split("=", 1)[1], "r") as f:
+      lines = [l.strip() for l in f.readlines()]
+    if len(lines) != 1:
+      self.fail("lines: %s" % lines)
+    self.assertEqual(lines[0], "data for bar.py")
+
 
 if __name__ == "__main__":
   unittest.main()

src/tools/runfiles/runfiles.py

diff --git a/src/tools/runfiles/runfiles.py b/src/tools/runfiles/runfiles.py
index 0a9149b..f889def 100644
--- a/src/tools/runfiles/runfiles.py
+++ b/src/tools/runfiles/runfiles.py
@@ -69,9 +69,8 @@
   the whole manifest file upon instantiation; this may be relevant for
   performance consideration.
 
-  Otherwise, if `env` contains "RUNFILES_DIR" or "TEST_SRCDIR" with non-empty
-  value (checked in this priority order), this method returns a directory-based
-  implementation.
+  Otherwise, if `env` contains "RUNFILES_DIR" with non-empty value (checked in
+  this priority order), this method returns a directory-based implementation.
 
   If neither cases apply, this method returns null.
 
@@ -87,8 +86,6 @@
     return CreateManifestBased(manifest)
 
   directory = env_map.get("RUNFILES_DIR")
-  if not directory:
-    directory = env_map.get("TEST_SRCDIR")
   if directory:
     return CreateDirectoryBased(directory)
 

src/tools/runfiles/runfiles_test.py

diff --git a/src/tools/runfiles/runfiles_test.py b/src/tools/runfiles/runfiles_test.py
index 10c1b8e..2255542 100644
--- a/src/tools/runfiles/runfiles_test.py
+++ b/src/tools/runfiles/runfiles_test.py
@@ -46,7 +46,7 @@
       r = runfiles.Create({
           "RUNFILES_MANIFEST_FILE": mf.Path(),
           "RUNFILES_DIR": "ignored when RUNFILES_MANIFEST_FILE has a value",
-          "TEST_SRCDIR": "ignored when RUNFILES_MANIFEST_FILE has a value"
+          "TEST_SRCDIR": "always ignored",
       })
       self.assertEqual(r.Rlocation("a/b"), "c/d")
       self.assertIsNone(r.Rlocation("foo"))
@@ -55,17 +55,12 @@
   def testCreatesDirectoryBasedRunfiles(self):
     r = runfiles.Create({
         "RUNFILES_DIR": "runfiles/dir",
-        "TEST_SRCDIR": "ignored when RUNFILES_DIR is set"
+        "TEST_SRCDIR": "always ignored",
     })
     self.assertEqual(r.Rlocation("a/b"), "runfiles/dir/a/b")
     self.assertEqual(r.Rlocation("foo"), "runfiles/dir/foo")
     self.assertDictEqual(r.EnvVar(), {"RUNFILES_DIR": "runfiles/dir"})
 
-    r = runfiles.Create({"TEST_SRCDIR": "test/srcdir"})
-    self.assertEqual(r.Rlocation("a/b"), "test/srcdir/a/b")
-    self.assertEqual(r.Rlocation("foo"), "test/srcdir/foo")
-    self.assertDictEqual(r.EnvVar(), {"RUNFILES_DIR": "test/srcdir"})
-
   def testFailsToCreateManifestBasedBecauseManifestDoesNotExist(self):
 
     def _Run():
@@ -78,10 +73,13 @@
       runfiles.Create({
           "RUNFILES_MANIFEST_FILE": mf.Path(),
           "RUNFILES_DIR": "whatever",
-          "TEST_SRCDIR": "whatever"
+          "TEST_SRCDIR": "always ignored",
       })
-    runfiles.Create({"RUNFILES_DIR": "whatever", "TEST_SRCDIR": "whatever"})
-    runfiles.Create({"TEST_SRCDIR": "whatever"})
+    runfiles.Create({
+        "RUNFILES_DIR": "whatever",
+        "TEST_SRCDIR": "always ignored",
+    })
+    self.assertIsNone(runfiles.Create({"TEST_SRCDIR": "always ignored"}))
     self.assertIsNone(runfiles.Create({"FOO": "bar"}))
 
   def testManifestBasedRlocation(self):