10000 Fix regression in pip parse for finding implicit namespace packages. … · lapointexavier/rules_python@fbbecae · GitHub
[go: up one dir, main page]
Skip to content

Commit fbbecae

Browse files
hrfullerJonathon Belotti
hrfuller
and
Jonathon Belotti
authored
Fix regression in pip parse for finding implicit namespace packages. (bazel-contrib#504)
* Fix regression in pip parse for finding implicit namespace packages. Pathlib.Path normalizes path names, which caused lookups of relative paths using the special '.' directory path to fail to find parents of standard packages. * use paths everywhere, add test for cwd case * refactor for mypy Co-authored-by: Jonathon Belotti <jonathon@canva.com>
1 parent 736b7ef commit fbbecae

File tree

5 files changed

+58
-27
lines changed

5 files changed

+58
-27
lines changed

‎python/pip_install/extract_wheels/lib/bazel.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ def extract_wheel(
184184
enable_implicit_namespace_pkgs: bool,
185185
incremental: bool = False,
186186
incremental_repo_prefix: Optional[str] = None,
187-
) -> str:
187+
) -> Optional[str]:
188188
"""Extracts wheel into given directory and creates py_library and filegroup targets.
189189
190190
Args:
@@ -250,5 +250,5 @@ def extract_wheel(
250250

251251
if not incremental:
252252
os.remove(whl.path)
253-
254-
return "//%s" % directory
253+
return f"//{directory}"
254+
return None

‎python/pip_install/extract_wheels/lib/namespace_pkgs.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
"""Utility functions to discover python package types"""
22
import os
3-
import pathlib # supported in >= 3.4
3+
from pathlib import Path # supported in >= 3.4
44
import textwrap
55
from typing import Set, List, Optional
66

77

88
def implicit_namespace_packages(
99
directory: str, ignored_dirnames: Optional[List[str]] = None
10-
) -> Set[str]:
10+
) -> Set[Path]:
1111
"""Discovers namespace packages implemented using the 'native namespace packages' method.
1212
1313
AKA 'implicit namespace packages', which has been supported since Python 3.3.
@@ -20,34 +20,36 @@ def implicit_namespace_packages(
2020
Returns:
2121
The set of directories found under root to be packages using the native namespace method.
2222
"""
23-
namespace_pkg_dirs: Set[str] = set()
24-
standard_pkg_dirs: Set[str] = set()
23+
namespace_pkg_dirs: Set[Path] = set()
24+
standard_pkg_dirs: Set[Path] = set()
25+
directory_path = Path(directory)
26+
ignored_dirname_paths: List[Path] = [Path(p) for p in ignored_dirnames or ()]
2527
# Traverse bottom-up because a directory can be a namespace pkg because its child contains module files.
26-
for dirpath, dirnames, filenames in os.walk(directory, topdown=False):
28+
for dirpath, dirnames, filenames in map(lambda t: (Path(t[0]), *t[1:]), os.walk(directory_path, topdown=False)):
2729
if "__init__.py" in filenames:
2830
standard_pkg_dirs.add(dirpath)
2931
continue
30-
elif ignored_dirnames:
31-
is_ignored_dir = dirpath in ignored_dirnames
32-
child_of_ignored_dir = any(d in pathlib.Path(dirpath).parents for d in ignored_dirnames)
32+
elif ignored_dirname_paths:
33+
is_ignored_dir = dirpath in ignored_dirname_paths
34+
child_of_ignored_dir = any(d in dirpath.parents for d in ignored_dirname_paths)
3335
if is_ignored_dir or child_of_ignored_dir:
3436
continue
3537

3638
dir_includes_py_modules = _includes_python_modules(filenames)
37-
parent_of_namespace_pkg = any(str(pathlib.Path(dirpath, d)) in namespace_pkg_dirs for d in dirnames)
38-
parent_of_standard_pkg = any(str(pathlib.Path(dirpath, d)) in standard_pkg_dirs for d in dirnames)
39+
parent_of_namespace_pkg = any(Path(dirpath, d) in namespace_pkg_dirs for d in dirnames)
40+
parent_of_standard_pkg = any(Path(dirpath, d) in standard_pkg_dirs for d in dirnames)
3941
parent_of_pkg = parent_of_namespace_pkg or parent_of_standard_pkg
4042
if (
4143
(dir_includes_py_modules or parent_of_pkg)
4244
and
4345
# The root of the directory should never be an implicit namespace
44-
dirpath != directory
46+
dirpath != directory_path
4547
):
4648
namespace_pkg_dirs.add(dirpath)
4749
return namespace_pkg_dirs
4850

4951

50-
def add_pkgutil_style_namespace_pkg_init(dir_path: str) -> None:
52+
def add_pkgutil_style_namespace_pkg_init(dir_path: Path) -> None:
5153
"""Adds 'pkgutil-style namespace packages' init file to the given directory
5254
5355
See: https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages
@@ -95,7 +97,7 @@ def _includes_python_modules(files: List[str]) -> bool:
9597
".pyd" # https://docs.python.org/3/faq/windows.html#is-a-pyd-file-the-same-as-a-dll
9698
}
9799
return any(
98-
pathlib.Path(f).suffix in module_suffixes
100+
Path(f).suffix in module_suffixes
99101
for f
100102
in files
101103
)

‎python/pip_install/extract_wheels/lib/namespace_pkgs_test.py

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1+
import os
12
import pathlib
23
import shutil
34
import tempfile
4-
from typing import Optional
5+
from typing import Optional, Set
56
import unittest
67

78
from python.pip_install.extract_wheels.lib import namespace_pkgs
@@ -32,6 +33,30 @@ def remove(self) -> None:
3233

3334

3435
class TestImplicitNamespacePackages(unittest.TestCase):
36+
37+
def assertPathsEqual(self, actual: Set[pathlib.Path], expected: Set[str]) -> None:
38+
self.assertEqual(actual, {pathlib.Path(p) for p in expected})
39+
40+
def test_in_current_directory(self) -> None:
41+
directory = TempDir()
42+
directory.add_file("foo/bar/biz.py")
43+
directory.add_file("foo/bee/boo.py")
44+
directory.add_file("foo/buu/__init__.py")
45+
directory.add_file("foo/buu/bii.py")
46+
cwd = os.getcwd()
47+
os.chdir(directory.root())
48+
expected = {
49+
"foo",
50+
"foo/bar",
51+
"foo/bee",
52+
}
53+
try:
54+
actual = namespace_pkgs.implicit_namespace_packages(".")
55+
self.assertPathsEqual(actual, expected)
56+
finally:
57+
os.chdir(cwd)
58+
directory.remove()
59+
3560
def test_finds_correct_namespace_packages(self) -> None:
3661
directory = TempDir()
3762
directory.add_file("foo/bar/biz.py")
@@ -45,7 +70,7 @@ def test_finds_correct_namespace_packages(self) -> None:
4570
directory.root() + "/foo/bee",
4671
}
4772
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
48-
self.assertEqual(actual, expected)
73+
self.assertPathsEqual(actual, expected)
4974

5075
def test_ignores_empty_directories(self) -> None:
5176
directory = TempDir()
@@ -57,7 +82,7 @@ def test_ignores_empty_directories(self) -> None:
5782
directory.root() + "/foo/bar",
5883
}
5984
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
60-
self.assertEqual(actual, expected)
85+
self.assertPathsEqual(actual, expected)
6186

6287
def test_empty_case(self) -> None:
6388
directory = TempDir()
@@ -87,7 +112,7 @@ def test_parent_child_relationship_of_namespace_pkgs(self):
87112
directory.root() + "/foo/bar/biff",
88113
}
89114
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
90-
self.assertEqual(actual, expected)
115+
self.assertPathsEqual(actual, expected)
91116

92117
def test_parent_child_relationship_of_namespace_and_standard_pkgs(self):
93118
directory = TempDir()
@@ -99,7 +124,7 @@ def test_parent_child_relationship_of_namespace_and_standard_pkgs(self):
99124
directory.root() + "/foo/bar",
100125
}
101126
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
102-
self.assertEqual(actual, expected)
127+
self.assertPathsEqual(actual, expected)
103128

104129
def test_parent_child_relationship_of_namespace_and_nested_standard_pkgs(self):
105130
directory = TempDir()
@@ -115,7 +140,7 @@ def test_parent_child_relationship_of_namespace_and_nested_standard_pkgs(self):
115140
directory.root() + "/fim",
116141
}
117142
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
118-
self.assertEqual(actual, expected)
143+
self.assertPathsEqual(actual, expected)
119144

120145
def test_recognized_all_nonstandard_module_types(self):
121146
directory = TempDir()
@@ -132,7 +157,7 @@ def test_recognized_all_nonstandard_module_types(self):
132157
directory.root() + "/eff/jee",
133158
}
134159
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
135-
self.assertEqual(actual, expected)
160+
self.assertPathsEqual(actual, expected)
136161

137162
def test_skips_ignored_directories(self):
138163
directory = TempDir()
@@ -147,7 +172,7 @@ def test_skips_ignored_directories(self):
147172
directory.root(),
148173
ignored_dirnames=[directory.root() + "/foo/boo"],
149174
)
150-
self.assertEqual(actual, expected)
175+
self.assertPathsEqual(actual, expected)
151176

152177

153178
if __name__ == "__main__":

‎python/pip_install/extract_wheels/lib/whl_filegroup_test.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ def _run(
3434
enable_implicit_namespace_pkgs=False,
3535
incremental=incremental,
3636
incremental_repo_prefix=incremental_repo_prefix
37-
)[2:] # Take off the leading // from the returned label.
37+
)
38+
# Take off the leading // from the returned label.
3839
# Assert that the raw wheel ends up in the package.
40+
generated_bazel_dir = generated_bazel_dir[2:] if not incremental else self.wheel_dir
3941
self.assertIn(self.wheel_name, os.listdir(generated_bazel_dir))
4042
with open("{}/BUILD.bazel".format(generated_bazel_dir)) as build_file:
4143
build_file_content = build_file.read()

‎python/pip_install/parse_requirements_to_bzl/__init__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ def install_deps():
118118
)
119119
)
120120

121+
def coerce_to_bool(option):
122+
return str(option).lower() == 'true'
121123

122124
def main() -> None:
123125
parser = argparse.ArgumentParser(
@@ -132,8 +134,8 @@ def main() -> None:
132134
)
133135
parser.add_argument(
134136
"--quiet",
135-
type=bool,
136-
action="store",
137+
type=coerce_to_bool,
138+
default=True,
137139
required=True,
138140
help="Whether to print stdout / stderr from child repos.",
139141
)

0 commit comments

Comments
 (0)
0