Skip to content

Commit

Permalink
dependencies: allow path dependencies with non-existent paths
Browse files Browse the repository at this point in the history
* print warning instead of raising an error in __init__ (allows `poetry lock --no-update` with missing locked path dependency that has been removed from pyproject.toml)
* introduce explicit `validate` method

Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
  • Loading branch information
radoering and adriangb committed Jan 22, 2023
1 parent 6cfe239 commit 549f7e2
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 39 deletions.
34 changes: 26 additions & 8 deletions src/poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import functools
import logging

from typing import TYPE_CHECKING
from typing import Iterable
Expand All @@ -13,6 +14,8 @@
if TYPE_CHECKING:
from pathlib import Path

logger = logging.getLogger(__name__)


class DirectoryDependency(PathDependency):
def __init__(
Expand All @@ -36,20 +39,35 @@ def __init__(
)
self._develop = develop

if self._full_path.is_file():
raise ValueError(f"{self._path} is a file, expected a directory")

if not is_python_project(self._full_path):
raise ValueError(
f"Directory {self._full_path} does not seem to be a Python package"
)

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

@property
def develop(self) -> bool:
return self._develop

def validate(self, *, raise_error: bool) -> bool:
if not super().validate(raise_error=raise_error):
return False

message = ""
if self._full_path.is_file():
message = (
f"{self._full_path} for {self.pretty_name} is a file,"
" expected a directory"
)
elif not is_python_project(self._full_path):
message = (
f"Directory {self._full_path} for {self.pretty_name} does not seem"
" to be a Python package"
)

if message:
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True

def _supports_poetry(self) -> bool:
return PyProjectTOML(self._full_path / "pyproject.toml").is_poetry_project()
17 changes: 16 additions & 1 deletion src/poetry/core/packages/file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import hashlib
import io
import logging
import warnings

from typing import TYPE_CHECKING
Expand All @@ -13,6 +14,8 @@
if TYPE_CHECKING:
from pathlib import Path

logger = logging.getLogger(__name__)


class FileDependency(PathDependency):
def __init__(
Expand All @@ -34,8 +37,20 @@ def __init__(
extras=extras,
)

def validate(self, *, raise_error: bool) -> bool:
if not super().validate(raise_error=raise_error):
return False

if self._full_path.is_dir():
raise ValueError(f"{self._path} is a directory, expected a file")
message = (
f"{self._full_path} for {self.pretty_name} is a directory,"
" expected a file"
)
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True

def hash(self, hash_name: str = "sha256") -> str:
warnings.warn(
Expand Down
18 changes: 15 additions & 3 deletions src/poetry/core/packages/path_dependency.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

import logging

from abc import ABC
from abc import abstractmethod
from pathlib import Path
Expand All @@ -9,6 +11,9 @@
from poetry.core.packages.utils.utils import path_to_url


logger = logging.getLogger(__name__)


class PathDependency(Dependency, ABC):
@abstractmethod
def __init__(
Expand All @@ -30,9 +35,6 @@ def __init__(
if not self._path.is_absolute():
self._full_path = self._base.joinpath(self._path).resolve()

if not self._full_path.exists():
raise ValueError(f"Path {self._path} does not exist")

super().__init__(
name,
"*",
Expand All @@ -43,6 +45,7 @@ def __init__(
source_url=self._full_path.as_posix(),
extras=extras,
)
self.validate(raise_error=False)

@property
def path(self) -> Path:
Expand All @@ -62,6 +65,15 @@ def is_file(self) -> bool:
def is_directory(self) -> bool:
return self._source_type == "directory"

def validate(self, *, raise_error: bool) -> bool:
if not self._full_path.exists():
message = f"Path {self._full_path} for {self.pretty_name} does not exist"
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True

@property
def base_pep_508_name(self) -> str:
requirement = self.pretty_name
Expand Down
34 changes: 25 additions & 9 deletions tests/masonry/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from contextlib import contextmanager
from pathlib import Path
from typing import TYPE_CHECKING
from typing import Iterator

import pytest
Expand All @@ -18,6 +19,10 @@
from tests.testutils import validate_wheel_contents


if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture


@contextmanager
def cwd(directory: str | Path) -> Iterator[None]:
prev = os.getcwd()
Expand Down Expand Up @@ -72,12 +77,15 @@ def test_build_wheel_with_bad_path_dev_dep_succeeds() -> None:
api.build_wheel(tmp_dir)


def test_build_wheel_with_bad_path_dep_fails() -> None:
with pytest.raises(ValueError) as err, temporary_directory() as tmp_dir, cwd(
def test_build_wheel_with_bad_path_dep_succeeds(caplog: LogCaptureFixture) -> None:
with temporary_directory() as tmp_dir, cwd(
os.path.join(fixtures, "with_bad_path_dep")
):
api.build_wheel(tmp_dir)
assert "does not exist" in str(err.value)
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message


@pytest.mark.skipif(
Expand Down Expand Up @@ -123,12 +131,15 @@ def test_build_sdist_with_bad_path_dev_dep_succeeds() -> None:
api.build_sdist(tmp_dir)


def test_build_sdist_with_bad_path_dep_fails() -> None:
with pytest.raises(ValueError) as err, temporary_directory() as tmp_dir, cwd(
def test_build_sdist_with_bad_path_dep_succeeds(caplog: LogCaptureFixture) -> None:
with temporary_directory() as tmp_dir, cwd(
os.path.join(fixtures, "with_bad_path_dep")
):
api.build_sdist(tmp_dir)
assert "does not exist" in str(err.value)
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message


def test_prepare_metadata_for_build_wheel() -> None:
Expand Down Expand Up @@ -209,12 +220,17 @@ def test_prepare_metadata_for_build_wheel_with_bad_path_dev_dep_succeeds() -> No
api.prepare_metadata_for_build_wheel(tmp_dir)


def test_prepare_metadata_for_build_wheel_with_bad_path_dep_succeeds() -> None:
with pytest.raises(ValueError) as err, temporary_directory() as tmp_dir, cwd(
def test_prepare_metadata_for_build_wheel_with_bad_path_dep_succeeds(
caplog: LogCaptureFixture,
) -> None:
with temporary_directory() as tmp_dir, cwd(
os.path.join(fixtures, "with_bad_path_dep")
):
api.prepare_metadata_for_build_wheel(tmp_dir)
assert "does not exist" in str(err.value)
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message


def test_build_editable_wheel() -> None:
Expand Down
42 changes: 38 additions & 4 deletions tests/packages/test_directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from pathlib import Path
from typing import TYPE_CHECKING
from typing import cast

import pytest
Expand All @@ -9,13 +10,46 @@
from poetry.core.packages.directory_dependency import DirectoryDependency


DIST_PATH = Path(__file__).parent.parent / "fixtures" / "git" / "github.com" / "demo"
if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture

DIST_PATH = Path(__file__).parent.parent / "fixtures" / "distributions"
SAMPLE_PROJECT = Path(__file__).parent.parent / "fixtures" / "sample_project"


def test_directory_dependency_must_exist() -> None:
with pytest.raises(ValueError):
DirectoryDependency("demo", DIST_PATH / "invalid")
def test_directory_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
dep = DirectoryDependency("demo", DIST_PATH / "invalid")
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message

with pytest.raises(ValueError, match="does not exist"):
dep.validate(raise_error=True)


def test_directory_dependency_is_file(caplog: LogCaptureFixture) -> None:
dep = DirectoryDependency("demo", DIST_PATH / "demo-0.1.0.tar.gz")
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "is a file" in record.message

with pytest.raises(ValueError, match="is a file"):
dep.validate(raise_error=True)


def test_directory_dependency_is_not_a_python_project(
caplog: LogCaptureFixture,
) -> None:
dep = DirectoryDependency("demo", DIST_PATH)
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "a Python package" in record.message

with pytest.raises(ValueError, match="not .* a Python package"):
dep.validate(raise_error=True)


def _test_directory_dependency_pep_508(
Expand Down
25 changes: 19 additions & 6 deletions tests/packages/test_file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@


if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture
from pytest_mock import MockerFixture

from poetry.core.version.markers import BaseMarker
Expand All @@ -20,14 +21,26 @@
TEST_FILE = "demo-0.1.0.tar.gz"


def test_file_dependency_wrong_path() -> None:
with pytest.raises(ValueError):
FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz")
def test_file_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
dep = FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz")
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message

with pytest.raises(ValueError, match="does not exist"):
dep.validate(raise_error=True)

def test_file_dependency_dir() -> None:
with pytest.raises(ValueError):
FileDependency("demo", DIST_PATH)

def test_file_dependency_is_directory(caplog: LogCaptureFixture) -> None:
dep = FileDependency("demo", DIST_PATH)
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "is a directory" in record.message

with pytest.raises(ValueError, match="is a directory"):
dep.validate(raise_error=True)


def test_default_hash() -> None:
Expand Down
21 changes: 13 additions & 8 deletions tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@


if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture

from poetry.core.packages.dependency import Dependency
from poetry.core.packages.vcs_dependency import VCSDependency

Expand Down Expand Up @@ -306,16 +308,19 @@ def test_create_poetry_omits_dev_dependencies_iff_with_dev_is_false() -> None:
assert any("dev" in r.groups for r in poetry.package.all_requires)


def test_create_poetry_fails_with_invalid_dev_dependencies_iff_with_dev_is_true() -> (
None
):
with pytest.raises(ValueError) as err:
Factory().create_poetry(fixtures_dir / "project_with_invalid_dev_deps")
assert "does not exist" in str(err.value)

Factory().create_poetry(
def test_create_poetry_with_invalid_dev_dependencies(caplog: LogCaptureFixture) -> None:
poetry = Factory().create_poetry(
fixtures_dir / "project_with_invalid_dev_deps", with_groups=False
)
assert not any("dev" in r.groups for r in poetry.package.all_requires)

assert not caplog.records
poetry = Factory().create_poetry(fixtures_dir / "project_with_invalid_dev_deps")
assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == "WARNING"
assert "does not exist" in record.message
assert any("dev" in r.groups for r in poetry.package.all_requires)


def test_create_poetry_with_groups_and_legacy_dev() -> None:
Expand Down

0 comments on commit 549f7e2

Please sign in to comment.