Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

packages: unify python project check #377

Merged
merged 2 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/poetry/core/packages/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ def create_from_pep_508(
from poetry.core.packages.url_dependency import URLDependency
from poetry.core.packages.utils.link import Link
from poetry.core.packages.utils.utils import is_archive_file
from poetry.core.packages.utils.utils import is_installable_dir
from poetry.core.packages.utils.utils import is_python_project
from poetry.core.packages.utils.utils import is_url
from poetry.core.packages.utils.utils import path_to_url
from poetry.core.packages.utils.utils import strip_extras
Expand Down Expand Up @@ -521,10 +521,9 @@ def create_from_pep_508(
path_str = os.path.normpath(os.path.abspath(name))
p, extras = strip_extras(path_str)
if os.path.isdir(p) and (os.path.sep in name or name.startswith(".")):

if not is_installable_dir(p):
if not is_python_project(Path(name)):
raise ValueError(
f"Directory {name!r} is not installable. File 'setup.py' "
f"Directory {name!r} is not installable. File 'setup.[py|cfg]' "
"not found."
)
link = Link(path_to_url(p))
Expand Down
26 changes: 11 additions & 15 deletions src/poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
from __future__ import annotations

import functools

from pathlib import Path
from typing import TYPE_CHECKING
from typing import Iterable

from poetry.core.pyproject.toml import PyProjectTOML


if TYPE_CHECKING:
from poetry.core.semver.version_constraint import VersionConstraint

from poetry.core.packages.dependency import Dependency
from poetry.core.packages.utils.utils import is_python_project
from poetry.core.packages.utils.utils import path_to_url


Expand All @@ -23,7 +28,6 @@ def __init__(
develop: bool = False,
extras: Iterable[str] | None = None,
) -> None:
from poetry.core.pyproject.toml import PyProjectTOML

self._path = path
self._base = base or Path.cwd()
Expand All @@ -43,18 +47,7 @@ def __init__(
if self._full_path.is_file():
raise ValueError(f"{self._path} is a file, expected a directory")

# Checking content to determine actions
setup_py = self._full_path / "setup.py"
setup_cfg = self._full_path / "setup.cfg"
setuptools_project = setup_py.exists() or setup_cfg.exists()
pyproject = PyProjectTOML(self._full_path / "pyproject.toml")

self._supports_pep517 = (
setuptools_project or pyproject.is_build_system_defined()
)
self._supports_poetry = pyproject.is_poetry_project()

if not (self._supports_pep517 or self._supports_poetry):
if not is_python_project(self._full_path):
raise ValueError(
f"Directory {self._full_path} does not seem to be a Python package"
)
Expand All @@ -70,6 +63,9 @@ def __init__(
extras=extras,
)

# cache this function to avoid multiple IO reads and parsing
self.supports_poetry = functools.lru_cache(maxsize=1)(self._supports_poetry)

@property
def path(self) -> Path:
return self._path
Expand All @@ -86,8 +82,8 @@ def base(self) -> Path:
def develop(self) -> bool:
return self._develop

def supports_poetry(self) -> bool:
return self._supports_poetry
def _supports_poetry(self) -> bool:
return PyProjectTOML(self._full_path / "pyproject.toml").is_poetry_project()

def is_directory(self) -> bool:
return True
Expand Down
25 changes: 17 additions & 8 deletions src/poetry/core/packages/utils/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

import os
import functools
import posixpath
import re
import sys
Expand All @@ -14,6 +14,7 @@
from urllib.parse import urlsplit
from urllib.request import url2pathname

from poetry.core.pyproject.toml import PyProjectTOML
from poetry.core.semver.version_range import VersionRange
from poetry.core.version.markers import dnf

Expand Down Expand Up @@ -119,14 +120,22 @@ def strip_extras(path: str) -> tuple[str, str | None]:
return path_no_extras, extras


def is_installable_dir(path: str) -> bool:
"""Return True if `path` is a directory containing a setup.py file."""
if not os.path.isdir(path):
@functools.lru_cache(maxsize=None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how much of a hot path this is/if caching is a significant benefit here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is this is called every time a dir dependency gets cloned, this happens a few times during resolution. And since this requires IO figured it can't hurt.

def is_python_project(path: Path) -> bool:
"""Return true if the directory is a Python project"""
if not path.is_dir():
return False
setup_py = os.path.join(path, "setup.py")
if os.path.isfile(setup_py):
return True
return False

setup_py = path / "setup.py"
setup_cfg = path / "setup.cfg"
setuptools_project = setup_py.exists() or setup_cfg.exists()

pyproject = PyProjectTOML(path / "pyproject.toml")

supports_pep517 = setuptools_project or pyproject.is_build_system_defined()
supports_poetry = pyproject.is_poetry_project()

return supports_pep517 or supports_poetry


def is_archive_file(name: str) -> bool:
Expand Down
19 changes: 19 additions & 0 deletions tests/packages/utils/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from __future__ import annotations

from pathlib import Path

import pytest

from poetry.core.packages.utils.utils import convert_markers
from poetry.core.packages.utils.utils import get_python_constraint_from_marker
from poetry.core.packages.utils.utils import is_python_project
from poetry.core.semver.helpers import parse_constraint
from poetry.core.version.markers import parse_marker

Expand Down Expand Up @@ -132,3 +135,19 @@ def test_get_python_constraint_from_marker(marker: str, constraint: str) -> None
marker_parsed = parse_marker(marker)
constraint_parsed = parse_constraint(constraint)
assert get_python_constraint_from_marker(marker_parsed) == constraint_parsed


@pytest.mark.parametrize(
("fixture", "result"),
[
("simple_project", True),
("project_with_setup_cfg_only", True),
("project_with_setup", True),
("project_with_pep517_non_poetry", True),
("project_without_pep517", False),
("does_not_exist", False),
],
)
def test_package_utils_is_python_project(fixture: str, result: bool) -> None:
path = Path(__file__).parent.parent.parent / "fixtures" / fixture
assert is_python_project(path) == result