Skip to content
Closed
66 changes: 44 additions & 22 deletions src/poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from pathlib import Path
from typing import Iterable
from warnings import warn

from poetry.core.packages.dependency import Dependency
from poetry.core.packages.utils.utils import is_python_project
Expand All @@ -12,6 +13,8 @@


class DirectoryDependency(Dependency):
_full_path: Path | None

def __init__(
self,
name: str,
Expand All @@ -24,26 +27,10 @@ def __init__(
) -> None:
self._path = path
self._base = base or Path.cwd()
self._full_path = path

if not self._path.is_absolute():
try:
self._full_path = self._base.joinpath(self._path).resolve()
except FileNotFoundError:
raise ValueError(f"Directory {self._path} does not exist")

self._full_path = None
self._develop = develop

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

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"
)
full_path = self._get_full_path(False, name)

super().__init__(
name,
Expand All @@ -52,20 +39,55 @@ def __init__(
optional=optional,
allows_prereleases=True,
source_type="directory",
source_url=self._full_path.as_posix(),
source_url=full_path.as_posix(),
extras=extras,
)

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

def _get_full_path(self, raise_if_invalid: bool, name: str) -> Path:
if self._full_path is not None:
return self._full_path
full_path = self._path
if not self._path.is_absolute():
try:
full_path = self._base.joinpath(self._path).resolve()
except FileNotFoundError:
msg = f"Source directory {self._path} for {name} does not exist"
if raise_if_invalid:
raise ValueError(msg)
warn(msg, category=UserWarning)
return full_path

if not full_path.exists():
msg = f"Source directory {self._path} for {name} does not exist"
if raise_if_invalid:
raise ValueError(msg)
warn(msg, category=UserWarning)
return full_path

if full_path.is_file():
raise ValueError(
f"Expected source for {name} to be a"
f" directory but {self._path} is a file"
)

if not is_python_project(full_path):
raise ValueError(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here. If I don't care for existence, I won't care if it's not a Python package.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd like to push back on this a bit. There's a real use case for creating a DirectoryDependency pointing to a path that does not exist: you deleted or renamed the dependency and are updating the lock file. I can't think of any use case where you'd want to point at an existing but invalid dependency, so in those cases I think failing fast is the best option.

f"The source directory {self._full_path} for {name}"
" does not seem to be a Python package",
)

self._full_path = full_path
return full_path

@property
def path(self) -> Path:
return self._path

@property
def full_path(self) -> Path:
return self._full_path
return self._get_full_path(True, self.name)

@property
def base(self) -> Path:
Expand All @@ -76,7 +98,7 @@ def develop(self) -> bool:
return self._develop

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

def is_directory(self) -> bool:
return True
Expand Down
54 changes: 38 additions & 16 deletions src/poetry/core/packages/file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@

from pathlib import Path
from typing import Iterable
from warnings import warn

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


class FileDependency(Dependency):
_full_path: Path | None

def __init__(
self,
name: str,
Expand All @@ -22,19 +25,8 @@ def __init__(
) -> None:
self._path = path
self._base = base or Path.cwd()
self._full_path = path

if not self._path.is_absolute():
try:
self._full_path = self._base.joinpath(self._path).resolve()
except FileNotFoundError:
raise ValueError(f"Directory {self._path} does not exist")

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

if self._full_path.is_dir():
raise ValueError(f"{self._path} is a directory, expected a file")
self._full_path = None
full_path = self._get_full_path(False, name)

super().__init__(
name,
Expand All @@ -43,10 +35,40 @@ def __init__(
optional=optional,
allows_prereleases=True,
source_type="file",
source_url=self._full_path.as_posix(),
source_url=full_path.as_posix(),
extras=extras,
)

def _get_full_path(self, raise_if_invalid: bool, name: str) -> Path:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adding name was necessary because .name doesn't get set until super().__init__ is called which already requires full_path.

if self._full_path is not None:
return self._full_path
Comment on lines +43 to +44
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This little bit of caching means that if we already checked in __init__ we don't need to check again. We could also cache the negative case but it doesn't seem worth the complexity IMO.

full_path = self._path
if not self._path.is_absolute():
try:
full_path = self._base.joinpath(self._path).resolve()
except FileNotFoundError:
msg = f"Source file {self._path} for {name} does not exist"
if raise_if_invalid:
raise ValueError(msg)
warn(msg, category=UserWarning)
return full_path

if not full_path.exists():
msg = f"Source file {self._path} for {name} does not exist"
if raise_if_invalid:
raise ValueError(msg)
warn(msg, category=UserWarning)
return full_path

if full_path.is_dir():
raise ValueError(
f"Expected source for {name} to be a"
f" file but {self._path} is a directory"
)

self._full_path = full_path
return full_path

@property
def base(self) -> Path:
return self._base
Expand All @@ -57,14 +79,14 @@ def path(self) -> Path:

@property
def full_path(self) -> Path:
return self._full_path
return self._get_full_path(True, self.name)

def is_file(self) -> bool:
return True

def hash(self, hash_name: str = "sha256") -> str:
h = hashlib.new(hash_name)
with self._full_path.open("rb") as fp:
with self.full_path.open("rb") as fp:
for content in iter(lambda: fp.read(io.DEFAULT_BUFFER_SIZE), b""):
h.update(content)

Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/non_python_project/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# missing build section
2 changes: 1 addition & 1 deletion tests/packages/test_directory_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

def test_directory_dependency_must_exist() -> None:
with pytest.raises(ValueError):
DirectoryDependency("demo", DIST_PATH / "invalid")
DirectoryDependency("demo", DIST_PATH / "invalid").full_path


def _test_directory_dependency_pep_508(
Expand Down
2 changes: 1 addition & 1 deletion tests/packages/test_file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

def test_file_dependency_wrong_path() -> None:
with pytest.raises(ValueError):
FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz")
FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz").full_path
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think these tests should be testing each individual failure mode, but this was good enough for years so I suppose it will do



def test_file_dependency_dir() -> None:
Expand Down
12 changes: 0 additions & 12 deletions tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,18 +298,6 @@ 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(
fixtures_dir / "project_with_invalid_dev_deps", with_groups=False
)


Comment on lines -301 to -312
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing this test since we actually want to be able to create a Poetry object with invalid path deps, we just want it to fail for certain operations (install, show, etc.).

def test_create_poetry_with_groups_and_legacy_dev() -> None:
poetry = Factory().create_poetry(
fixtures_dir / "project_with_groups_and_legacy_dev"
Expand Down