Repository: bazelbuild/bazel
Issue: runfiles,Python: move to //tools/python/runfiles
Labels: P3

### Description of the problem / feature request:

The language-specific tools should be under `//tools/<language>/<tool>` but the Python-specific runfiles library is currently under `@bazel_tools//tools/runfiles:py-runfiles` (in Bazel 0.12.0 release candidate).

See also https://github.com/bazelbuild/bazel/pull/4873

Issue comments:
Blocking https://github.com/bazelbuild/bazel/issues/4582
Can we close this issue or is it still a release blocker for 0.12.0?
It's no longer a blocker, but it's also not resolved.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/tools/runfiles/BUILD b/src/tools/runfiles/BUILD
index 0df3386cad158d..81e156d4b7dc32 100644
--- a/src/tools/runfiles/BUILD
+++ b/src/tools/runfiles/BUILD
@@ -22,24 +22,11 @@ filegroup(
         "BUILD.tools",
         "runfiles.cc",
         "runfiles.h",
-        "runfiles.py",
         "//src/tools/runfiles/java/com/google/devtools/build/runfiles:embedded_tools",
     ],
     visibility = ["//src:__pkg__"],
 )
 
-py_library(
-    name = "py-runfiles",
-    srcs = ["runfiles.py"],
-)
-
-py_test(
-    name = "py-runfiles-test",
-    srcs = ["runfiles_test.py"],
-    main = "runfiles_test.py",
-    deps = [":py-runfiles"],
-)
-
 cc_library(
     name = "cc-runfiles",
     srcs = ["runfiles.cc"],
diff --git a/tools/BUILD b/tools/BUILD
index 4488574d28943b..21a95fc486899a 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -57,7 +57,7 @@ filegroup(
         "//tools/def_parser:srcs",
         "//tools/platforms:srcs",
         "//tools/objc:srcs",
-        "//tools/python:srcs",
+        "//tools/python:embedded_tools",
         "//tools/runfiles:embedded_tools",
         "//tools/test:srcs",
         "//tools/osx/crosstool:srcs",
diff --git a/tools/python/BUILD b/tools/python/BUILD
index 6933f9d4197a4e..ab09128e0c8650 100644
--- a/tools/python/BUILD
+++ b/tools/python/BUILD
@@ -6,10 +6,28 @@ sh_binary(
 )
 
 filegroup(
-    name = "srcs",
+    name = "srcs_and_embedded_tools",
     srcs = [
         # Tools are build from the workspace for tests.
         "2to3.sh",
         "BUILD",
     ],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "srcs",
+    srcs = [
+        ":srcs_and_embedded_tools",
+        "//tools/python/runfiles:srcs",
+    ],
+)
+
+filegroup(
+    name = "embedded_tools",
+    srcs = [
+        ":srcs_and_embedded_tools",
+        "//tools/python/runfiles:embedded_tools",
+    ],
+    visibility = ["//tools:__pkg__"],
 )
diff --git a/tools/python/runfiles/BUILD b/tools/python/runfiles/BUILD
new file mode 100644
index 00000000000000..2723302bb4cdc6
--- /dev/null
+++ b/tools/python/runfiles/BUILD
@@ -0,0 +1,34 @@
+package(default_visibility = ["//visibility:private"])
+
+filegroup(
+    name = "srcs",
+    srcs = glob(
+        ["**"],
+        exclude = [
+            ".*",
+            "*~",
+        ],
+    ),
+    visibility = ["//tools/python:__pkg__"],
+)
+
+filegroup(
+    name = "embedded_tools",
+    srcs = [
+        "BUILD.tools",
+        "runfiles.py",
+    ],
+    visibility = ["//tools/python:__pkg__"],
+)
+
+py_library(
+    name = "runfiles",
+    testonly = 1,
+    srcs = ["runfiles.py"],
+)
+
+py_library(
+    name = "runfiles_test",
+    srcs = ["runfiles_test.py"],
+    visibility = ["//visibility:public"],
+)
diff --git a/tools/python/runfiles/BUILD.tools b/tools/python/runfiles/BUILD.tools
new file mode 100644
index 00000000000000..4080d68774d468
--- /dev/null
+++ b/tools/python/runfiles/BUILD.tools
@@ -0,0 +1,5 @@
+py_library(
+    name = "runfiles",
+    srcs = ["runfiles.py"],
+    visibility = ["//visibility:public"],
+)


Test patch:
diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py
index 1a9a6703b2eed3..a348b932f9370b 100644
--- a/src/test/py/bazel/runfiles_test.py
+++ b/src/test/py/bazel/runfiles_test.py
@@ -74,7 +74,7 @@ def _AssertPythonRunfilesLibraryInBazelToolsRepo(self, family, lang_name):
     for s, t in [
         ("WORKSPACE.mock", "WORKSPACE"),
         ("foo/BUILD.mock", "foo/BUILD"),
-        ("foo/runfiles.py", "foo/runfiles.py"),
+        ("foo/foo.py", "foo/foo.py"),
         ("foo/datadep/hello.txt", "foo/datadep/hello.txt"),
         ("bar/BUILD.mock", "bar/BUILD"),
         ("bar/bar.py", "bar/bar.py"),
@@ -131,19 +131,15 @@ def _AssertPythonRunfilesLibraryInBazelToolsRepo(self, family, lang_name):
 
       i += 2
 
-  # TODO(laszlocsomor): re-enable after
-  # https://github.com/bazelbuild/bazel/issues/4878 is fixed.
-  # def testPythonRunfilesLibraryInBazelToolsRepo(self):
-  #  self._AssertPythonRunfilesLibraryInBazelToolsRepo("py", "Python")
+  def testPythonRunfilesLibraryInBazelToolsRepo(self):
+   self._AssertPythonRunfilesLibraryInBazelToolsRepo("py", "Python")
 
   def testRunfilesLibrariesFindRunfilesWithoutEnvvars(self):
     for s, t in [
         ("WORKSPACE.mock", "WORKSPACE"),
         ("bar/BUILD.mock", "bar/BUILD"),
-        # TODO(laszlocsomor): uncomment Python files after
-        # https://github.com/bazelbuild/bazel/issues/4878 is fixed.
-        # ("bar/bar.py", "bar/bar.py"),
-        # ("bar/bar-py-data.txt", "bar/bar-py-data.txt"),
+        ("bar/bar.py", "bar/bar.py"),
+        ("bar/bar-py-data.txt", "bar/bar-py-data.txt"),
         ("bar/Bar.java", "bar/Bar.java"),
         ("bar/bar-java-data.txt", "bar/bar-java-data.txt"),
     ]:
@@ -155,12 +151,10 @@ def testRunfilesLibrariesFindRunfilesWithoutEnvvars(self):
     self.AssertExitCode(exit_code, 0, stderr)
     bazel_bin = stdout[0]
 
-    exit_code, _, stderr = self.RunBazel(["build", "//bar:bar-java"])
+    exit_code, _, stderr = self.RunBazel(["build", "//bar:bar-py", "//bar:bar-java"])
     self.AssertExitCode(exit_code, 0, stderr)
 
-    # TODO(laszlocsomor): add Python after
-    # https://github.com/bazelbuild/bazel/issues/4878 is fixed.
-    for lang in [("java", "Java", "Bar.java")]:
+    for lang in [("py", "Python", "bar.py"), ("java", "Java", "Bar.java")]:
       if test_base.TestBase.IsWindows():
         bin_path = os.path.join(bazel_bin, "bar/bar-%s.exe" % lang[0])
       else:
diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
index 92098bce73213e..b6d52f717e9d2c 100644
--- a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
+++ b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
@@ -5,7 +5,7 @@ py_binary(
     srcs = ["bar.py"],
     data = ["bar-py-data.txt"],
     main = "bar.py",
-    deps = ["@bazel_tools//tools/runfiles:py-runfiles"],
+    deps = ["@bazel_tools//tools/python/runfiles"],
 )
 
 java_binary(
diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/bar.py b/src/test/py/bazel/testdata/runfiles_test/bar/bar.py
index a88b10b28ba73e..43d21525a54c42 100644
--- a/src/test/py/bazel/testdata/runfiles_test/bar/bar.py
+++ b/src/test/py/bazel/testdata/runfiles_test/bar/bar.py
@@ -14,7 +14,7 @@
 """Mock Python binary, only used in tests."""
 
 from __future__ import print_function
-from bazel_tools.tools.runfiles import runfiles
+from bazel_tools.tools.python.runfiles import runfiles
 
 print('Hello Python Bar!')
 r = runfiles.Create()
diff --git a/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock b/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
index 15a3a7d467b3ad..04fc23ebace7e3 100644
--- a/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
+++ b/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
@@ -1,13 +1,13 @@
 py_binary(
     name = "runfiles-py",
-    srcs = ["runfiles.py"],
+    srcs = ["foo.py"],
     data = [
         "datadep/hello.txt",
         "//bar:bar-py",
         "//bar:bar-java",
     ],
-    main = "runfiles.py",
-    deps = ["@bazel_tools//tools/runfiles:py-runfiles"],
+    main = "foo.py",
+    deps = ["@bazel_tools//tools/python/runfiles"],
 )
 
 java_binary(
diff --git a/src/test/py/bazel/testdata/runfiles_test/foo/runfiles.py b/src/test/py/bazel/testdata/runfiles_test/foo/foo.py
similarity index 97%
rename from src/test/py/bazel/testdata/runfiles_test/foo/runfiles.py
rename to src/test/py/bazel/testdata/runfiles_test/foo/foo.py
index 32100447e95c55..47fd6197f3083d 100644
--- a/src/test/py/bazel/testdata/runfiles_test/foo/runfiles.py
+++ b/src/test/py/bazel/testdata/runfiles_test/foo/foo.py
@@ -18,7 +18,7 @@
 import os
 import subprocess
 
-from bazel_tools.tools.runfiles import runfiles
+from bazel_tools.tools.python.runfiles import runfiles
 
 
 def IsWindows():