From 1d0c316a4cb0b9228a5f3d396338bd2f174b5f5f Mon Sep 17 00:00:00 2001 From: stephsamson Date: Fri, 27 Mar 2020 19:33:26 +0100 Subject: [PATCH] Allow inline tables in includes to be similarly functional to the `package` directive. Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder. Resolves: #8 --- poetry/core/factory.py | 13 +- poetry/core/json/schemas/poetry-schema.json | 49 +++++-- poetry/core/masonry/builders/builder.py | 125 ++++++++++++++---- poetry/core/masonry/builders/sdist.py | 42 +++++- poetry/core/masonry/builders/wheel.py | 43 +----- poetry/core/masonry/utils/module.py | 4 +- .../with_include_inline_table/pyproject.toml | 47 +++++++ .../src/src_package/__init__.py | 0 .../tests/__init__.py | 0 .../tests/test_foo/test.py | 0 .../with_include_inline_table/wheel_only.txt | 0 tests/masonry/builders/test_sdist.py | 40 +++++- tests/masonry/builders/test_wheel.py | 13 ++ tests/test_factory.py | 5 +- 14 files changed, 297 insertions(+), 84 deletions(-) create mode 100644 tests/masonry/builders/fixtures/with_include_inline_table/pyproject.toml create mode 100644 tests/masonry/builders/fixtures/with_include_inline_table/src/src_package/__init__.py create mode 100644 tests/masonry/builders/fixtures/with_include_inline_table/tests/__init__.py create mode 100644 tests/masonry/builders/fixtures/with_include_inline_table/tests/test_foo/test.py create mode 100644 tests/masonry/builders/fixtures/with_include_inline_table/wheel_only.txt diff --git a/poetry/core/factory.py b/poetry/core/factory.py index 323481aa1..6bb229a7f 100644 --- a/poetry/core/factory.py +++ b/poetry/core/factory.py @@ -115,7 +115,18 @@ def create_poetry(self, cwd=None): # type: (Optional[Path]) -> Poetry package.build_config = build or {} if "include" in local_config: - package.include = local_config["include"] + package.include = [] + + for include in local_config["include"]: + if not isinstance(include, dict): + include = {"path": include} + + formats = include.get("format", []) + if formats and not isinstance(formats, list): + formats = [formats] + include["format"] = formats + + package.include.append(include) if "exclude" in local_config: package.exclude = local_config["exclude"] diff --git a/poetry/core/json/schemas/poetry-schema.json b/poetry/core/json/schemas/poetry-schema.json index e3e7341b8..af5d7ee0b 100644 --- a/poetry/core/json/schemas/poetry-schema.json +++ b/poetry/core/json/schemas/poetry-schema.json @@ -73,26 +73,43 @@ ], "properties": { "include": { - "type": "string", - "description": "What to include in the package." + "$ref": "#/definitions/include-path" }, "from": { "type": "string", "description": "Where the source directory of the package resides." }, "format": { - "oneOf": [ - {"type": "string"}, - {"type": "array", "items": {"type": "string"}} - ], - "description": "The format(s) for which the package must be included." + "$ref": "#/definitions/package-formats" } } } }, "include": { "type": "array", - "description": "A list of files and folders to include." + "description": "A list of files and folders to include.", + "items": { + "anyOf": [ + { + "$ref": "#/definitions/include-path" + }, + { + "type": "object", + "additionalProperties": false, + "required": [ + "path" + ], + "properties": { + "path": { + "$ref": "#/definitions/include-path" + }, + "format": { + "$ref": "#/definitions/package-formats" + } + } + } + ] + } }, "exclude": { "type": "array", @@ -189,6 +206,22 @@ "type": "string" } }, + "include-path": { + "type": "string", + "description": "Path to file or directory to include." + }, + "package-format": { + "type": "string", + "enum": ["sdist", "wheel"], + "description": "A Python packaging format." + }, + "package-formats": { + "oneOf": [ + {"$ref": "#/definitions/package-format"}, + {"type": "array", "items": {"$ref": "#/definitions/package-format"}} + ], + "description": "The format(s) for which the package must be included." + }, "dependencies": { "type": "object", "patternProperties": { diff --git a/poetry/core/masonry/builders/builder.py b/poetry/core/masonry/builders/builder.py index d903b3a6a..cdb85791b 100644 --- a/poetry/core/masonry/builders/builder.py +++ b/poetry/core/masonry/builders/builder.py @@ -6,6 +6,7 @@ from collections import defaultdict from contextlib import contextmanager +from typing import Optional from typing import Set from typing import Union @@ -37,7 +38,7 @@ class Builder(object): def __init__( self, poetry, ignore_packages_formats=False - ): # type: ("Poetry", bool) -> None + ): # type: ("Poetry", "Env", "IO", bool) -> None self._poetry = poetry self._package = poetry.package self._path = poetry.file.parent @@ -60,12 +61,27 @@ def __init__( packages.append(p) + includes = [] + for include in self._package.include: + formats = include.get("format", []) + + if ( + formats + and self.format + and self.format not in formats + and not ignore_packages_formats + ): + continue + + includes.append(include) + self._module = Module( self._package.name, self._path.as_posix(), packages=packages, - includes=self._package.include, + includes=includes, ) + self._meta = Metadata.from_package(self._package) def build(self): @@ -113,23 +129,53 @@ def is_excluded(self, filepath): # type: (Union[str, Path]) -> bool return False - def find_files_to_add(self, exclude_build=True): # type: (bool) -> list + def find_files_to_add( + self, exclude_build=True + ): # type: (bool) -> Set[BuildIncludeFile] """ Finds all files to add to the tarball """ - to_add = [] + to_add = set() for include in self._module.includes: + include.refresh() + formats = include.formats or ["sdist"] + for file in include.elements: if "__pycache__" in str(file): continue if file.is_dir(): + if self.format in formats: + for current_file in file.glob("**/*"): + include_file = BuildIncludeFile( + path=current_file, source_root=self._path + ) + + if ( + include_file.relative_to_source_root() not in to_add + and not current_file.is_dir() + and not self.is_excluded( + include_file.relative_to_source_root() + ) + ): + to_add.add(include_file) continue - file = file.relative_to(self._path) + if ( + isinstance(include, PackageInclude) + and include.source + and self.format == "wheel" + ): + source_root = include.base + else: + source_root = self._path + + include_file = BuildIncludeFile(path=file, source_root=source_root) - if self.is_excluded(file) and isinstance(include, PackageInclude): + if self.is_excluded( + include_file.relative_to_source_root() + ) and isinstance(include, PackageInclude): continue if file.suffix == ".pyc": @@ -140,31 +186,19 @@ def find_files_to_add(self, exclude_build=True): # type: (bool) -> list continue logger.debug(" - Adding: {}".format(str(file))) - to_add.append(file) - - # Include project files - logger.debug(" - Adding: pyproject.toml") - to_add.append(Path("pyproject.toml")) - - # If a license file exists, add it - for license_file in self._path.glob("LICENSE*"): - logger.debug(" - Adding: {}".format(license_file.relative_to(self._path))) - to_add.append(license_file.relative_to(self._path)) - - # If a README is specified we need to include it - # to avoid errors - if "readme" in self._poetry.local_config: - readme = self._path / self._poetry.local_config["readme"] - if readme.exists(): - logger.debug(" - Adding: {}".format(readme.relative_to(self._path))) - to_add.append(readme.relative_to(self._path)) - - # If a build script is specified and explicitely required + to_add.add(include_file) + + # If a build script is specified and explicitly required # we add it to the list of files if self._package.build_script and not exclude_build: - to_add.append(Path(self._package.build_script)) + to_add.add( + BuildIncludeFile( + path=self._path / Path(self._package.build_script), + source_root=self._path, + ) + ) - return sorted(to_add) + return to_add def get_metadata_content(self): # type: () -> bytes content = METADATA_BASE.format( @@ -268,3 +302,38 @@ def temporary_directory(cls, *args, **kwargs): yield name shutil.rmtree(name) + + +class BuildIncludeFile: + def __init__( + self, + path, # type: Path + source_root=None, # type: Optional[Path] + ): + """ + :param path: a full path to the file to be included + :param source_root: the root path to resolve to + """ + self.path = Path(path).resolve() + self.source_root = None if not source_root else Path(source_root).resolve() + if not self.path.is_absolute() and self.source_root: + self.path = (self.source_root / self.path).resolve() + + def __eq__(self, other): # type: (Union[BuildIncludeFile, Path]) -> bool + if hasattr(other, "path"): + return self.path == other.path + return self.path == other + + def __ne__(self, other): # type: (Union[BuildIncludeFile, Path]) -> bool + return not self.__eq__(other) + + def __hash__(self): + return hash(self.path) + + def __repr__(self): # type: () -> str + return str(self.path) + + def relative_to_source_root(self): # type(): -> Path + if self.source_root is not None: + return self.path.relative_to(self.source_root) + return self.path diff --git a/poetry/core/masonry/builders/sdist.py b/poetry/core/masonry/builders/sdist.py index eb456c378..f3c551828 100644 --- a/poetry/core/masonry/builders/sdist.py +++ b/poetry/core/masonry/builders/sdist.py @@ -13,6 +13,7 @@ from posixpath import join as pjoin from pprint import pformat from typing import Iterator +from typing import List from poetry.core.utils._compat import Path from poetry.core.utils._compat import decode @@ -22,6 +23,7 @@ from ..utils.helpers import normalize_file_permissions from ..utils.package_include import PackageInclude from .builder import Builder +from .builder import BuildIncludeFile SETUP = """\ @@ -74,15 +76,15 @@ def build(self, target_dir=None): # type: (Path) -> Path files_to_add = self.find_files_to_add(exclude_build=False) - for relpath in files_to_add: - path = self._path / relpath + for file in files_to_add: tar_info = tar.gettarinfo( - str(path), arcname=pjoin(tar_dir, str(relpath)) + str(file.path), + arcname=pjoin(tar_dir, str(file.relative_to_source_root())), ) tar_info = self.clean_tarinfo(tar_info) if tar_info.isreg(): - with path.open("rb") as f: + with file.path.open("rb") as f: tar.addfile(tar_info, f) else: tar.addfile(tar_info) # Symlinks & ? @@ -105,7 +107,6 @@ def build(self, target_dir=None): # type: (Path) -> Path gz.close() logger.info(" - Built {}".format(target.name)) - return target def build_setup(self): # type: () -> bytes @@ -302,6 +303,37 @@ def find_nearest_pkg(rel_path): return pkgdir, sorted(packages), pkg_data + def find_files_to_add( + self, exclude_build=False + ): # type: (bool) -> List[BuildIncludeFile] + to_add = super(SdistBuilder, self).find_files_to_add(exclude_build) + + # Include project files + logger.debug(" - Adding: pyproject.toml") + to_add.add( + BuildIncludeFile( + path=self._path / Path("pyproject.toml"), source_root=self._path + ) + ) + + # If a license file exists, add it + for license_file in self._path.glob("LICENSE*"): + license_file = BuildIncludeFile(path=license_file, source_root=self._path) + logger.debug(" - Adding: {}".format(license_file.relative_to_source_root())) + to_add.add(license_file) + + # If a README is specified we need to include it to avoid errors + if "readme" in self._poetry.local_config: + readme = BuildIncludeFile( + path=self._path / self._poetry.local_config["readme"], + source_root=self._path, + ) + if readme.path.exists(): + logger.debug(" - Adding: {}".format(readme.relative_to_source_root())) + to_add.add(readme) + + return sorted(list(to_add), key=lambda x: x.relative_to_source_root()) + @classmethod def convert_dependencies(cls, package, dependencies): main = [] diff --git a/poetry/core/masonry/builders/wheel.py b/poetry/core/masonry/builders/wheel.py index 16eec2c2a..da7129c9f 100644 --- a/poetry/core/masonry/builders/wheel.py +++ b/poetry/core/masonry/builders/wheel.py @@ -22,7 +22,6 @@ from ..utils.helpers import escape_name from ..utils.helpers import escape_version from ..utils.helpers import normalize_file_permissions -from ..utils.package_include import PackageInclude from .builder import Builder from .sdist import SdistBuilder @@ -130,47 +129,13 @@ def _run_build_command(self, setup): [sys.executable, str(setup), "build", "-b", str(self._path / "build")] ) - def _copy_module(self, wheel): - - to_add = [] - - for include in self._module.includes: - if include.formats and "wheel" not in include.formats: - continue - - include.refresh() - - for file in include.elements: - if "__pycache__" in str(file): - continue - - if file.is_dir(): - continue - - if isinstance(include, PackageInclude) and include.source: - rel_file = file.relative_to(include.base) - else: - rel_file = file.relative_to(self._path) - - if self.is_excluded(rel_file.as_posix()) and isinstance( - include, PackageInclude - ): - continue - - if file.suffix == ".pyc": - continue - - if (file, rel_file) in to_add: - # Skip duplicates - continue - - logger.debug(" - Adding: {}".format(str(file))) - to_add.append((file, rel_file)) + def _copy_module(self, wheel): # type: (zipfile.ZipFile) -> None + to_add = self.find_files_to_add() # Walk the files and compress them, # sorting everything so the order is stable. - for full_path, rel_path in sorted(to_add, key=lambda x: x[1]): - self._add_file(wheel, full_path, rel_path) + for file in sorted(list(to_add), key=lambda x: x.path): + self._add_file(wheel, file.path, file.relative_to_source_root()) def _write_metadata(self, wheel): if ( diff --git a/poetry/core/masonry/utils/module.py b/poetry/core/masonry/utils/module.py index bde49bf3f..c1de7c530 100644 --- a/poetry/core/masonry/utils/module.py +++ b/poetry/core/masonry/utils/module.py @@ -74,7 +74,9 @@ def __init__(self, name, directory=".", packages=None, includes=None): ) for include in includes: - self._includes.append(Include(self._path, include)) + self._includes.append( + Include(self._path, include["path"], formats=include["format"]) + ) @property def name(self): # type: () -> str diff --git a/tests/masonry/builders/fixtures/with_include_inline_table/pyproject.toml b/tests/masonry/builders/fixtures/with_include_inline_table/pyproject.toml new file mode 100644 index 000000000..95b0949c6 --- /dev/null +++ b/tests/masonry/builders/fixtures/with_include_inline_table/pyproject.toml @@ -0,0 +1,47 @@ +[tool.poetry] +name = "with-include" +version = "1.2.3" +description = "Some description." +authors = [ + "Sébastien Eustace " +] +license = "MIT" + +homepage = "https://python-poetry.org/" +repository = "https://github.com/python-poetry/poetry" +documentation = "https://python-poetry.org/docs" + +keywords = ["packaging", "dependency", "poetry"] + +classifiers = [ + "Topic :: Software Development :: Build Tools", + "Topic :: Software Development :: Libraries :: Python Modules" +] + +packages = [ + { include = "src_package", from = "src"}, +] + +include = [ + { path = "tests", format = "sdist" }, + { path = "wheel_only.txt", format = "wheel" } +] + + +# Requirements +[tool.poetry.dependencies] +python = "^3.6" +cleo = "^0.6" +cachy = { version = "^0.2.0", extras = ["msgpack"] } + +pendulum = { version = "^1.4", optional = true } + +[tool.poetry.dev-dependencies] +pytest = "~3.4" + +[tool.poetry.extras] +time = ["pendulum"] + +[tool.poetry.scripts] +my-script = "my_package:main" +my-2nd-script = "my_package:main2" diff --git a/tests/masonry/builders/fixtures/with_include_inline_table/src/src_package/__init__.py b/tests/masonry/builders/fixtures/with_include_inline_table/src/src_package/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/masonry/builders/fixtures/with_include_inline_table/tests/__init__.py b/tests/masonry/builders/fixtures/with_include_inline_table/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/masonry/builders/fixtures/with_include_inline_table/tests/test_foo/test.py b/tests/masonry/builders/fixtures/with_include_inline_table/tests/test_foo/test.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/masonry/builders/fixtures/with_include_inline_table/wheel_only.txt b/tests/masonry/builders/fixtures/with_include_inline_table/wheel_only.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tests/masonry/builders/test_sdist.py b/tests/masonry/builders/test_sdist.py index 3af32d252..51bc50444 100644 --- a/tests/masonry/builders/test_sdist.py +++ b/tests/masonry/builders/test_sdist.py @@ -161,7 +161,7 @@ def test_find_files_to_add(): poetry = Factory().create_poetry(project("complete")) builder = SdistBuilder(poetry) - result = builder.find_files_to_add() + result = [f.relative_to_source_root() for f in builder.find_files_to_add()] assert sorted(result) == sorted( [ @@ -494,6 +494,44 @@ def test_proper_python_requires_if_three_digits_precision_version_specified(): assert parsed["Requires-Python"] == "==2.7.15" +def test_includes(): + poetry = Factory().create_poetry(project("with-include")) + + builder = SdistBuilder(poetry) + + builder.build() + + sdist = fixtures_dir / "with-include" / "dist" / "with-include-1.2.3.tar.gz" + + assert sdist.exists() + + with tarfile.open(str(sdist), "r") as tar: + assert "with-include-1.2.3/extra_dir/vcs_excluded.txt" in tar.getnames() + assert "with-include-1.2.3/notes.txt" in tar.getnames() + + +def test_includes_with_inline_table(): + poetry = Factory().create_poetry(project("with_include_inline_table")) + + builder = SdistBuilder(poetry) + + builder.build() + + sdist = ( + fixtures_dir + / "with_include_inline_table" + / "dist" + / "with-include-1.2.3.tar.gz" + ) + + assert sdist.exists() + + with tarfile.open(str(sdist), "r") as tar: + assert "with-include-1.2.3/wheel_only.txt" not in tar.getnames() + assert "with-include-1.2.3/tests/__init__.py" in tar.getnames() + assert "with-include-1.2.3/tests/test_foo/test.py" in tar.getnames() + + def test_excluded_subpackage(): poetry = Factory().create_poetry(project("excluded_subpackage")) diff --git a/tests/masonry/builders/test_wheel.py b/tests/masonry/builders/test_wheel.py index 294047a6c..2fd81bc23 100644 --- a/tests/masonry/builders/test_wheel.py +++ b/tests/masonry/builders/test_wheel.py @@ -157,6 +157,19 @@ def test_dist_info_file_permissions(): ) +def test_wheel_includes_inline_table(): + module_path = fixtures_dir / "with_include_inline_table" + WheelBuilder.make(Factory().create_poetry(module_path)) + + whl = module_path / "dist" / "with_include-1.2.3-py3-none-any.whl" + + assert whl.exists() + + with zipfile.ZipFile(str(whl)) as z: + assert "wheel_only.txt" in z.namelist() + assert "notes.txt" not in z.namelist() + + @pytest.mark.parametrize( "package", ["pep_561_stub_only", "pep_561_stub_only_partial", "pep_561_stub_only_src"], diff --git a/tests/test_factory.py b/tests/test_factory.py index af15b627f..aafafea60 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -139,7 +139,10 @@ def test_create_poetry_with_packages_and_includes(): {"include": "src_package", "from": "src"}, ] - assert package.include == ["extra_dir/vcs_excluded.txt", "notes.txt"] + assert package.include == [ + {"path": "extra_dir/vcs_excluded.txt", "format": []}, + {"path": "notes.txt", "format": []}, + ] def test_create_poetry_with_multi_constraints_dependency():