From 4d1d2158470b417c12c809eb1e8780a91ad19915 Mon Sep 17 00:00:00 2001 From: Matt Mackay Date: Thu, 2 Jun 2022 17:38:08 -0400 Subject: [PATCH] fix: add the runfiles root as a python package --- py/private/venv/venv.bzl | 11 +++++++++-- py/tests/external-deps/BUILD.bazel | 10 +++++++++- py/tests/external-deps/expected_pathing | 1 + .../external-deps/test_can_import_runfiles_helper.py | 4 ++++ py/tests/import-pathing/tests.bzl | 10 ++++++++++ 5 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 py/tests/external-deps/test_can_import_runfiles_helper.py diff --git a/py/private/venv/venv.bzl b/py/private/venv/venv.bzl index 8c9f2a14..4bafc0b5 100644 --- a/py/private/venv/venv.bzl +++ b/py/private/venv/venv.bzl @@ -69,9 +69,9 @@ def _make_venv(ctx, name = None, main = None, strip_pth_workspace_root = None): # each segment from site-packages in the venv to the root of the runfiles tree. # Four .. will get us back to the root of the venv: # {name}.runfiles/.{name}.venv/lib/python{version}/site-packages/first_party.pth - escape = ([".."] * 4) + escape = "/".join(([".."] * 4)) pth_add_all_kwargs = dict({ - "format_each": "/".join(escape) + "/%s", + "format_each": escape + "/%s", }) # If we are creating a venv for an IDE we likely don't have a workspace folder at with everything inside, so strip @@ -82,6 +82,13 @@ def _make_venv(ctx, name = None, main = None, strip_pth_workspace_root = None): "map_each": _pth_import_line_map, }) + # A few imports rely on being able to reference the root of the runfiles tree as a Python module, + # the common case here being the @rules_python//python/runfiles target that adds the runfiles helper, + # which ends up in bazel_tools/tools/python/runfiles/runfiles.py, but there are no imports attrs that hint we + # should be adding the root to the PYTHONPATH + # Maybe in the future we can opt out of this? + pth_lines.add(escape) + pth_lines.add_all( imports_depset, **pth_add_all_kwargs diff --git a/py/tests/external-deps/BUILD.bazel b/py/tests/external-deps/BUILD.bazel index 54afebb9..dd0fcf6d 100644 --- a/py/tests/external-deps/BUILD.bazel +++ b/py/tests/external-deps/BUILD.bazel @@ -1,4 +1,4 @@ -load("//py:defs.bzl", "py_binary", "py_library") +load("//py:defs.bzl", "py_binary", "py_library", "py_test") load("@python_toolchain//:defs.bzl", "host_platform") load("@aspect_bazel_lib//lib:write_source_files.bzl", "write_source_files") @@ -54,3 +54,11 @@ write_source_files( "expected_click": "click_out", }, ) + +py_test( + name = "test_can_import_runfiles_helper", + srcs = ["test_can_import_runfiles_helper.py"], + deps = [ + "@bazel_tools//tools/python/runfiles", + ], +) diff --git a/py/tests/external-deps/expected_pathing b/py/tests/external-deps/expected_pathing index 637ec25f..894dc2a2 100644 --- a/py/tests/external-deps/expected_pathing +++ b/py/tests/external-deps/expected_pathing @@ -10,6 +10,7 @@ sys path: (py_toolchain)/lib/python3.9 (py_toolchain)/lib/python3.9/lib-dynload (pwd)/bazel-out/host/bin/py/tests/external-deps/pathing.runfiles/pathing.venv/lib/python3.9/site-packages +(pwd)/bazel-out/host/bin/py/tests/external-deps/pathing.runfiles (pwd)/bazel-out/host/bin/py/tests/external-deps/pathing.runfiles/aspect_rules_py/py/tests/external-deps Entrypoint Path: (pwd)/bazel-out/host/bin/py/tests/external-deps/pathing.runfiles/aspect_rules_py/py/tests/external-deps/pathing.py diff --git a/py/tests/external-deps/test_can_import_runfiles_helper.py b/py/tests/external-deps/test_can_import_runfiles_helper.py new file mode 100644 index 00000000..6fb7f092 --- /dev/null +++ b/py/tests/external-deps/test_can_import_runfiles_helper.py @@ -0,0 +1,4 @@ +try: + from bazel_tools.tools.python.runfiles import runfiles +except ModuleNotFoundError as err: + assert False, "Expected to be able to import runfiles helper" diff --git a/py/tests/import-pathing/tests.bzl b/py/tests/import-pathing/tests.bzl index d7f2f590..c5580893 100644 --- a/py/tests/import-pathing/tests.bzl +++ b/py/tests/import-pathing/tests.bzl @@ -20,6 +20,16 @@ def _can_resolve_path_in_workspace_test_impl(ctx): imports = py_library.make_imports_depset(fake_ctx).to_list() asserts.equals(env, "aspect_rules_py/foo/bar", imports[0]) + # Empty string import is semantically equal to "." + fake_ctx = _ctx_with_imports([""]) + imports = py_library.make_imports_depset(fake_ctx).to_list() + asserts.equals(env, "aspect_rules_py/foo/bar", imports[0]) + + # Empty imports array results in no imports + fake_ctx = _ctx_with_imports([]) + imports = py_library.make_imports_depset(fake_ctx).to_list() + asserts.equals(env, 0, len(imports)) + # The import path is the parent package fake_ctx = _ctx_with_imports([".."]) imports = py_library.make_imports_depset(fake_ctx).to_list()