From f133e9eda70ce60a32e047e2aa573f80ec686e5c Mon Sep 17 00:00:00 2001 From: stephsamson Date: Sun, 17 Nov 2019 17:08:15 +0100 Subject: [PATCH 1/2] Handle nested wildcards in package includes correctly. Fixes: #1379. Use compat `Path`. --- poetry/masonry/utils/package_include.py | 6 ++-- tests/masonry/utils/__init__.py | 0 .../utils/fixtures/with_includes/__init__.py | 0 .../utils/fixtures/with_includes/bar/baz.py | 0 .../extra_package/some_dir/foo.py | 0 .../extra_package/some_dir/quux.py | 0 tests/masonry/utils/test_package_include.py | 32 +++++++++++++++++++ 7 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 tests/masonry/utils/__init__.py create mode 100644 tests/masonry/utils/fixtures/with_includes/__init__.py create mode 100644 tests/masonry/utils/fixtures/with_includes/bar/baz.py create mode 100644 tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/foo.py create mode 100644 tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/quux.py create mode 100644 tests/masonry/utils/test_package_include.py diff --git a/poetry/masonry/utils/package_include.py b/poetry/masonry/utils/package_include.py index ee185497a61..cc284e40c1d 100644 --- a/poetry/masonry/utils/package_include.py +++ b/poetry/masonry/utils/package_include.py @@ -44,10 +44,10 @@ def check_elements(self): # type: () -> PackageInclude # Probably glob self._is_package = True - # The __init__.py file should be first + # Packages no longer need an __init__.py in python3 root = self._elements[0] - if root.name != "__init__.py": - raise ValueError("{} is not a package.".format(root)) + if root.name == "__init__.py": + pass self._package = root.parent.name else: diff --git a/tests/masonry/utils/__init__.py b/tests/masonry/utils/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/fixtures/with_includes/__init__.py b/tests/masonry/utils/fixtures/with_includes/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/fixtures/with_includes/bar/baz.py b/tests/masonry/utils/fixtures/with_includes/bar/baz.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/foo.py b/tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/foo.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/quux.py b/tests/masonry/utils/fixtures/with_includes/extra_package/some_dir/quux.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/test_package_include.py b/tests/masonry/utils/test_package_include.py new file mode 100644 index 00000000000..863e88100ba --- /dev/null +++ b/tests/masonry/utils/test_package_include.py @@ -0,0 +1,32 @@ +from poetry.masonry.utils.package_include import PackageInclude +from poetry.utils._compat import Path + + +fixtures_dir = Path(__file__).parent / "fixtures" +with_includes = fixtures_dir / "with_includes" + + +def test_package_include_with_multiple_dirs(): + pkg_include = PackageInclude(base=fixtures_dir, include="with_includes") + assert pkg_include.elements == [ + with_includes / "__init__.py", + with_includes / "bar", + with_includes / "bar/baz.py", + with_includes / "extra_package", + with_includes / "extra_package/some_dir", + with_includes / "extra_package/some_dir/foo.py", + with_includes / "extra_package/some_dir/quux.py", + ] + + +def test_package_include_with_simple_dir(): + pkg_include = PackageInclude(base=with_includes, include="bar") + assert pkg_include.elements == [with_includes / "bar/baz.py"] + + +def test_package_include_with_nested_dir(): + pkg_include = PackageInclude(base=with_includes, include="extra_package/**/*.py") + assert pkg_include.elements == [ + with_includes / "extra_package/some_dir/foo.py", + with_includes / "extra_package/some_dir/quux.py", + ] From 1d0e99d1c96a55b7950014f8a95fe2b4d49315d1 Mon Sep 17 00:00:00 2001 From: stephsamson Date: Mon, 6 Jan 2020 17:59:11 +0700 Subject: [PATCH 2/2] Ensure that when building sdists, subpackages with Python files (but not necessarily an __init__.py file) will be included. --- poetry/masonry/builders/sdist.py | 2 +- poetry/masonry/utils/package_include.py | 23 +++++++++++-------- .../complete/my_package/sub_pkg3/foo.py | 0 tests/masonry/builders/test_complete.py | 1 + tests/masonry/builders/test_sdist.py | 9 +++++++- .../with_includes/not_a_python_pkg/baz.txt | 0 tests/masonry/utils/test_package_include.py | 11 +++++++++ 7 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 tests/masonry/builders/fixtures/complete/my_package/sub_pkg3/foo.py create mode 100644 tests/masonry/utils/fixtures/with_includes/not_a_python_pkg/baz.txt diff --git a/poetry/masonry/builders/sdist.py b/poetry/masonry/builders/sdist.py index 7bb26c92ea6..c906f4c473d 100644 --- a/poetry/masonry/builders/sdist.py +++ b/poetry/masonry/builders/sdist.py @@ -237,7 +237,7 @@ def find_nearest_pkg(rel_path): if from_top_level == ".": continue - is_subpkg = "__init__.py" in filenames + is_subpkg = any([filename.endswith(".py") for filename in filenames]) if is_subpkg: subpkg_paths.add(from_top_level) parts = from_top_level.split(os.sep) diff --git a/poetry/masonry/utils/package_include.py b/poetry/masonry/utils/package_include.py index cc284e40c1d..42ecbbc1fd0 100644 --- a/poetry/masonry/utils/package_include.py +++ b/poetry/masonry/utils/package_include.py @@ -12,7 +12,6 @@ def __init__(self, base, include, formats=None, source=None): base = base / source super(PackageInclude, self).__init__(base, include, formats=formats) - self.check_elements() @property @@ -35,6 +34,8 @@ def refresh(self): # type: () -> PackageInclude return self.check_elements() def check_elements(self): # type: () -> PackageInclude + root = self._elements[0] + if not self._elements: raise ValueError( "{} does not contain any element".format(self._base / self._include) @@ -44,20 +45,24 @@ def check_elements(self): # type: () -> PackageInclude # Probably glob self._is_package = True - # Packages no longer need an __init__.py in python3 - root = self._elements[0] - if root.name == "__init__.py": - pass + # Packages no longer need an __init__.py in python3, but there must + # at least be one .py file for it to be considered a package + if not any([element.suffix == ".py" for element in self._elements]): + raise ValueError("{} is not a package.".format(root.name)) self._package = root.parent.name else: - if self._elements[0].is_dir(): + if root.is_dir(): # If it's a directory, we include everything inside it - self._package = self._elements[0].name - self._elements = sorted(list(self._elements[0].glob("**/*"))) + self._package = root.name + self._elements = sorted(list(root.glob("**/*"))) + + if not any([element.suffix == ".py" for element in self._elements]): + raise ValueError("{} is not a package.".format(root.name)) + self._is_package = True else: - self._package = self._elements[0].stem + self._package = root.stem self._is_module = True return self diff --git a/tests/masonry/builders/fixtures/complete/my_package/sub_pkg3/foo.py b/tests/masonry/builders/fixtures/complete/my_package/sub_pkg3/foo.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/builders/test_complete.py b/tests/masonry/builders/test_complete.py index 05ae2e17824..f4adf5442d7 100644 --- a/tests/masonry/builders/test_complete.py +++ b/tests/masonry/builders/test_complete.py @@ -265,6 +265,7 @@ def test_complete_no_vcs(): "my_package/sub_pkg1/__init__.py", "my_package/sub_pkg2/__init__.py", "my_package/sub_pkg2/data2/data.json", + "my_package/sub_pkg3/foo.py", "my_package-1.2.3.dist-info/entry_points.txt", "my_package-1.2.3.dist-info/LICENSE", "my_package-1.2.3.dist-info/WHEEL", diff --git a/tests/masonry/builders/test_sdist.py b/tests/masonry/builders/test_sdist.py index 4609c331990..fdc9f943d03 100644 --- a/tests/masonry/builders/test_sdist.py +++ b/tests/masonry/builders/test_sdist.py @@ -124,6 +124,7 @@ def test_make_setup(): "my_package", "my_package.sub_pkg1", "my_package.sub_pkg2", + "my_package.sub_pkg3", ] assert ns["install_requires"] == ["cachy[msgpack]>=0.2.0,<0.3.0", "cleo>=0.6,<0.7"] assert ns["entry_points"] == { @@ -178,6 +179,7 @@ def test_find_files_to_add(): Path("my_package/sub_pkg1/__init__.py"), Path("my_package/sub_pkg2/__init__.py"), Path("my_package/sub_pkg2/data2/data.json"), + Path("my_package/sub_pkg3/foo.py"), Path("pyproject.toml"), ] ) @@ -213,7 +215,12 @@ def test_find_packages(): pkg_dir, packages, pkg_data = builder.find_packages(include) assert pkg_dir is None - assert packages == ["my_package", "my_package.sub_pkg1", "my_package.sub_pkg2"] + assert packages == [ + "my_package", + "my_package.sub_pkg1", + "my_package.sub_pkg2", + "my_package.sub_pkg3", + ] assert pkg_data == { "": ["*"], "my_package": ["data1/*"], diff --git a/tests/masonry/utils/fixtures/with_includes/not_a_python_pkg/baz.txt b/tests/masonry/utils/fixtures/with_includes/not_a_python_pkg/baz.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/masonry/utils/test_package_include.py b/tests/masonry/utils/test_package_include.py index 863e88100ba..a79ff96f03c 100644 --- a/tests/masonry/utils/test_package_include.py +++ b/tests/masonry/utils/test_package_include.py @@ -1,3 +1,5 @@ +import pytest + from poetry.masonry.utils.package_include import PackageInclude from poetry.utils._compat import Path @@ -16,6 +18,8 @@ def test_package_include_with_multiple_dirs(): with_includes / "extra_package/some_dir", with_includes / "extra_package/some_dir/foo.py", with_includes / "extra_package/some_dir/quux.py", + with_includes / "not_a_python_pkg", + with_includes / "not_a_python_pkg/baz.txt", ] @@ -30,3 +34,10 @@ def test_package_include_with_nested_dir(): with_includes / "extra_package/some_dir/foo.py", with_includes / "extra_package/some_dir/quux.py", ] + + +def test_package_include_with_no_python_files_in_dir(): + with pytest.raises(ValueError) as e: + PackageInclude(base=with_includes, include="not_a_python_pkg") + + assert str(e.value) == "not_a_python_pkg is not a package."