Fix regression in pip parse for finding implicit namespace packages. (#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>
diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py
index aabc316..ca1a8bb 100644
--- a/python/pip_install/extract_wheels/lib/bazel.py
+++ b/python/pip_install/extract_wheels/lib/bazel.py
@@ -184,7 +184,7 @@
enable_implicit_namespace_pkgs: bool,
incremental: bool = False,
incremental_repo_prefix: Optional[str] = None,
-) -> str:
+) -> Optional[str]:
"""Extracts wheel into given directory and creates py_library and filegroup targets.
Args:
@@ -250,5 +250,5 @@
if not incremental:
os.remove(whl.path)
-
- return "//%s" % directory
+ return f"//{directory}"
+ return None
diff --git a/python/pip_install/extract_wheels/lib/namespace_pkgs.py b/python/pip_install/extract_wheels/lib/namespace_pkgs.py
index da529de..b3177b3 100644
--- a/python/pip_install/extract_wheels/lib/namespace_pkgs.py
+++ b/python/pip_install/extract_wheels/lib/namespace_pkgs.py
@@ -1,13 +1,13 @@
"""Utility functions to discover python package types"""
import os
-import pathlib # supported in >= 3.4
+from pathlib import Path # supported in >= 3.4
import textwrap
from typing import Set, List, Optional
def implicit_namespace_packages(
directory: str, ignored_dirnames: Optional[List[str]] = None
-) -> Set[str]:
+) -> Set[Path]:
"""Discovers namespace packages implemented using the 'native namespace packages' method.
AKA 'implicit namespace packages', which has been supported since Python 3.3.
@@ -20,34 +20,36 @@
Returns:
The set of directories found under root to be packages using the native namespace method.
"""
- namespace_pkg_dirs: Set[str] = set()
- standard_pkg_dirs: Set[str] = set()
+ namespace_pkg_dirs: Set[Path] = set()
+ standard_pkg_dirs: Set[Path] = set()
+ directory_path = Path(directory)
+ ignored_dirname_paths: List[Path] = [Path(p) for p in ignored_dirnames or ()]
# Traverse bottom-up because a directory can be a namespace pkg because its child contains module files.
- for dirpath, dirnames, filenames in os.walk(directory, topdown=False):
+ for dirpath, dirnames, filenames in map(lambda t: (Path(t[0]), *t[1:]), os.walk(directory_path, topdown=False)):
if "__init__.py" in filenames:
standard_pkg_dirs.add(dirpath)
continue
- elif ignored_dirnames:
- is_ignored_dir = dirpath in ignored_dirnames
- child_of_ignored_dir = any(d in pathlib.Path(dirpath).parents for d in ignored_dirnames)
+ elif ignored_dirname_paths:
+ is_ignored_dir = dirpath in ignored_dirname_paths
+ child_of_ignored_dir = any(d in dirpath.parents for d in ignored_dirname_paths)
if is_ignored_dir or child_of_ignored_dir:
continue
dir_includes_py_modules = _includes_python_modules(filenames)
- parent_of_namespace_pkg = any(str(pathlib.Path(dirpath, d)) in namespace_pkg_dirs for d in dirnames)
- parent_of_standard_pkg = any(str(pathlib.Path(dirpath, d)) in standard_pkg_dirs for d in dirnames)
+ parent_of_namespace_pkg = any(Path(dirpath, d) in namespace_pkg_dirs for d in dirnames)
+ parent_of_standard_pkg = any(Path(dirpath, d) in standard_pkg_dirs for d in dirnames)
parent_of_pkg = parent_of_namespace_pkg or parent_of_standard_pkg
if (
(dir_includes_py_modules or parent_of_pkg)
and
# The root of the directory should never be an implicit namespace
- dirpath != directory
+ dirpath != directory_path
):
namespace_pkg_dirs.add(dirpath)
return namespace_pkg_dirs
-def add_pkgutil_style_namespace_pkg_init(dir_path: str) -> None:
+def add_pkgutil_style_namespace_pkg_init(dir_path: Path) -> None:
"""Adds 'pkgutil-style namespace packages' init file to the given directory
See: https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages
@@ -95,7 +97,7 @@
".pyd" # https://docs.python.org/3/faq/windows.html#is-a-pyd-file-the-same-as-a-dll
}
return any(
- pathlib.Path(f).suffix in module_suffixes
+ Path(f).suffix in module_suffixes
for f
in files
)
diff --git a/python/pip_install/extract_wheels/lib/namespace_pkgs_test.py b/python/pip_install/extract_wheels/lib/namespace_pkgs_test.py
index baec8b3..dca7026 100644
--- a/python/pip_install/extract_wheels/lib/namespace_pkgs_test.py
+++ b/python/pip_install/extract_wheels/lib/namespace_pkgs_test.py
@@ -1,7 +1,8 @@
+import os
import pathlib
import shutil
import tempfile
-from typing import Optional
+from typing import Optional, Set
import unittest
from python.pip_install.extract_wheels.lib import namespace_pkgs
@@ -32,6 +33,30 @@
class TestImplicitNamespacePackages(unittest.TestCase):
+
+ def assertPathsEqual(self, actual: Set[pathlib.Path], expected: Set[str]) -> None:
+ self.assertEqual(actual, {pathlib.Path(p) for p in expected})
+
+ def test_in_current_directory(self) -> None:
+ directory = TempDir()
+ directory.add_file("foo/bar/biz.py")
+ directory.add_file("foo/bee/boo.py")
+ directory.add_file("foo/buu/__init__.py")
+ directory.add_file("foo/buu/bii.py")
+ cwd = os.getcwd()
+ os.chdir(directory.root())
+ expected = {
+ "foo",
+ "foo/bar",
+ "foo/bee",
+ }
+ try:
+ actual = namespace_pkgs.implicit_namespace_packages(".")
+ self.assertPathsEqual(actual, expected)
+ finally:
+ os.chdir(cwd)
+ directory.remove()
+
def test_finds_correct_namespace_packages(self) -> None:
directory = TempDir()
directory.add_file("foo/bar/biz.py")
@@ -45,7 +70,7 @@
directory.root() + "/foo/bee",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_ignores_empty_directories(self) -> None:
directory = TempDir()
@@ -57,7 +82,7 @@
directory.root() + "/foo/bar",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_empty_case(self) -> None:
directory = TempDir()
@@ -87,7 +112,7 @@
directory.root() + "/foo/bar/biff",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_parent_child_relationship_of_namespace_and_standard_pkgs(self):
directory = TempDir()
@@ -99,7 +124,7 @@
directory.root() + "/foo/bar",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_parent_child_relationship_of_namespace_and_nested_standard_pkgs(self):
directory = TempDir()
@@ -115,7 +140,7 @@
directory.root() + "/fim",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_recognized_all_nonstandard_module_types(self):
directory = TempDir()
@@ -132,7 +157,7 @@
directory.root() + "/eff/jee",
}
actual = namespace_pkgs.implicit_namespace_packages(directory.root())
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
def test_skips_ignored_directories(self):
directory = TempDir()
@@ -147,7 +172,7 @@
directory.root(),
ignored_dirnames=[directory.root() + "/foo/boo"],
)
- self.assertEqual(actual, expected)
+ self.assertPathsEqual(actual, expected)
if __name__ == "__main__":
diff --git a/python/pip_install/extract_wheels/lib/whl_filegroup_test.py b/python/pip_install/extract_wheels/lib/whl_filegroup_test.py
index 84054b1..4fd0159 100644
--- a/python/pip_install/extract_wheels/lib/whl_filegroup_test.py
+++ b/python/pip_install/extract_wheels/lib/whl_filegroup_test.py
@@ -34,8 +34,10 @@
enable_implicit_namespace_pkgs=False,
incremental=incremental,
incremental_repo_prefix=incremental_repo_prefix
- )[2:] # Take off the leading // from the returned label.
+ )
+ # Take off the leading // from the returned label.
# Assert that the raw wheel ends up in the package.
+ generated_bazel_dir = generated_bazel_dir[2:] if not incremental else self.wheel_dir
self.assertIn(self.wheel_name, os.listdir(generated_bazel_dir))
with open("{}/BUILD.bazel".format(generated_bazel_dir)) as build_file:
build_file_content = build_file.read()
diff --git a/python/pip_install/parse_requirements_to_bzl/__init__.py b/python/pip_install/parse_requirements_to_bzl/__init__.py
index 66e6f5e..8802ef4 100644
--- a/python/pip_install/parse_requirements_to_bzl/__init__.py
+++ b/python/pip_install/parse_requirements_to_bzl/__init__.py
@@ -118,6 +118,8 @@
)
)
+def coerce_to_bool(option):
+ return str(option).lower() == 'true'
def main() -> None:
parser = argparse.ArgumentParser(
@@ -132,8 +134,8 @@
)
parser.add_argument(
"--quiet",
- type=bool,
- action="store",
+ type=coerce_to_bool,
+ default=True,
required=True,
help="Whether to print stdout / stderr from child repos.",
)
