Skip to content

Commit 781650c

Browse files
authored
fix: Handle single-file module pypi-deps in py_pex_binary (#392)
Addresses the bug in #391. Single-file modules such as `six` and `typing-extensions` does not work with the current `py_pex_binary` rule, since it's assumed that there exist a sub-directory within `site-packages` that is not dist-info. An example of what a single-file module pypi-dep looks like: ``` ls bazel-reppro_pex_err/external/rules_python~~pip~pypi_311_six/site-packages/ __init__.py six-1.16.0.dist-info six.py ``` Since `Distribution.load(..)` takes in the `site-packages` directory, we only emit this part of the path, and let `uniquify=True` handle deduplication after `_map_srcs` is applied. --- ### Changes are visible to end-users: no ### Test plan <!-- Delete any which do not apply --> - Manual testing; please provide instructions so we can reproduce: Add `six` to requirements and `"@pypi_six//:pkg"` as a dep to the py_pex_binary example; then import `six` in py_pex_binary's `say.py`. Printing the module or `cowsay.cow(f"{six}")` shows that the previous example and single-files modules now also work.
1 parent df9476f commit 781650c

File tree

8 files changed

+62
-19
lines changed

8 files changed

+62
-19
lines changed

gazelle_python.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2161,6 +2161,7 @@ manifest:
21612161
numpy.lib.ufunclike: numpy
21622162
numpy.lib.user_array: numpy
21632163
numpy.lib.utils: numpy
2164+
numpy.libs.libopenblas64_p-r0-0cf96a72: numpy
21642165
numpy.linalg: numpy
21652166
numpy.linalg.lapack_lite: numpy
21662167
numpy.linalg.linalg: numpy
@@ -3716,6 +3717,7 @@ manifest:
37163717
past.types.olddict: future
37173718
past.types.oldstr: future
37183719
past.utils: future
3720+
pillow.libs.libopenjp2-05423b53: pillow
37193721
pluggy: pluggy
37203722
proxytypes: jsonref
37213723
psutil: psutil
@@ -3986,4 +3988,4 @@ manifest:
39863988
yaml.tokens: PyYAML
39873989
pip_repository:
39883990
name: pypi
3989-
integrity: e0cc13413ec04d597469a3a5baa75d320d7c614017bbccd34aa3e356675dd984
3991+
integrity: 2e0cbc780621ce75951013e6968f66d1aadd8d1ecb8b7c470883c998b169e733

py/private/py_pex_binary.bzl

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,15 @@ def _map_srcs(f, workspace):
5656

5757
site_packages_i = f.path.find("site-packages")
5858

59-
# if path contains `site-packages` and there is only two path segments
60-
# after it, it will be treated as third party dep.
61-
# Here are some examples of path we expect and use and ones we ignore.
62-
#
63-
# Match: `external/rules_python~~pip~pypi_39_rtoml/site-packages/rtoml-0.11.0.dist-info/INSTALLER`
64-
# Reason: It has two `/` after first `site-packages` substring.
65-
#
66-
# No Match: `external/rules_python~~pip~pypi_39_rtoml/site-packages/rtoml-0.11.0/src/mod/parse.py`
67-
# Reason: It has three `/` after first `site-packages` substring.
68-
if site_packages_i != -1 and f.path.count("/", site_packages_i) == 2:
69-
if f.path.find("dist-info", site_packages_i) != -1:
59+
# If the path contains 'site-packages', treat it as a third party dep
60+
if site_packages_i != -1:
61+
if f.path.find("dist-info", site_packages_i) != -1 and f.path.count("/", site_packages_i) == 2:
7062
return ["--distinfo={}".format(f.dirname)]
71-
return ["--dep={}".format(f.dirname)]
7263

73-
elif site_packages_i == -1:
74-
# If the path does not have a `site-packages` in it, then put it into
75-
# the standard runfiles tree.
76-
return ["--source={}={}".format(f.path, dest_path)]
64+
return ["--dep={}".format(f.path[:site_packages_i + len("site-packages")])]
7765

78-
return []
66+
# If the path does not have a `site-packages` in it, then put it into the standard runfiles tree.
67+
return ["--source={}={}".format(f.path, dest_path)]
7968

8069
def _py_python_pex_impl(ctx):
8170
py_toolchain = _py_semantics.resolve_toolchain(ctx)

py/tests/py-pex-binary/BUILD.bazel

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
load("@aspect_bazel_lib//lib:testing.bzl", "assert_contains")
2+
load("//py:defs.bzl", "py_binary", "py_pex_binary")
3+
4+
# Test that both single-file modules (six) and multi-file modules (cowsay) work with py_pex_binary.
5+
py_binary(
6+
name = "print_modules_bin",
7+
srcs = ["print_modules.py"],
8+
data = ["data.txt"],
9+
deps = [
10+
"@bazel_tools//tools/python/runfiles",
11+
"@pypi_cowsay//:pkg",
12+
"@pypi_six//:pkg",
13+
],
14+
)
15+
16+
py_pex_binary(
17+
name = "print_modules_pex",
18+
binary = ":print_modules_bin",
19+
python_interpreter_constraints = [],
20+
)
21+
22+
# PEX_ROOT is set to avoid warning on default user PEX_ROOT not being writable
23+
genrule(
24+
name = "run_print_modules_pex",
25+
outs = ["print_modules_pex.out"],
26+
cmd = "PEX_ROOT=.pex $(execpath print_modules_pex) >$@",
27+
tools = ["print_modules_pex"],
28+
)
29+
30+
assert_contains(
31+
name = "test__print_modules_pex",
32+
actual = "print_modules_pex.out",
33+
expected = "Mooo!,cowsay-6.1/cowsay/__init__.py,six-1.16.0/six.py",
34+
)

py/tests/py-pex-binary/data.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Mooo!
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import sys
2+
import cowsay
3+
import six
4+
from bazel_tools.tools.python.runfiles import runfiles
5+
6+
7+
r = runfiles.Create()
8+
data_path = r.Rlocation("aspect_rules_py/py/tests/py-pex-binary/data.txt")
9+
10+
# strings on one line to test presence for all
11+
print(open(data_path).read()
12+
+ ","
13+
+ "/".join(cowsay.__file__.split("/")[-3:])
14+
+ ","
15+
+ "/".join(six.__file__.split("/")[-2:]))

py/tools/pex/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def __call__(self, parser, namespace, value, option_str=None):
159159
]
160160

161161
for dep in options.dependencies:
162-
dist = Distribution.load(dep + "/../")
162+
dist = Distribution.load(dep)
163163

164164
# TODO: explain which level of inferno is this!
165165
key = "%s-%s" % (dist.key, dist.version)

requirements.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ pytest
55
cowsay
66
ftfy==6.2.0
77
neptune==1.10.2
8+
six

requirements.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,7 @@ six==1.16.0 \
770770
--hash=sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926 \
771771
--hash=sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254
772772
# via
773+
# -r requirements.in
773774
# bravado
774775
# bravado-core
775776
# neptune

0 commit comments

Comments
 (0)