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

Use strict optional checking in req_install.py #11379

Merged
merged 9 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
Empty file.
33 changes: 20 additions & 13 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# The following comment should be removed at some point in the future.
# mypy: strict-optional=False

import functools
import logging
import os
Expand Down Expand Up @@ -244,6 +241,7 @@ def supports_pyproject_editable(self) -> bool:

@property
def specifier(self) -> SpecifierSet:
assert self.req is not None
return self.req.specifier

@property
Expand All @@ -257,7 +255,8 @@ def is_pinned(self) -> bool:

For example, some-package==1.2 is pinned; some-package>1.2 is not.
"""
specifiers = self.specifier
assert self.req is not None
specifiers = self.req.specifier
return len(specifiers) == 1 and next(iter(specifiers)).operator in {"==", "==="}

def match_markers(self, extras_requested: Optional[Iterable[str]] = None) -> bool:
Expand Down Expand Up @@ -305,6 +304,7 @@ def hashes(self, trust_internet: bool = True) -> Hashes:
else:
link = None
if link and link.hash:
assert link.hash_name is not None
good_hashes.setdefault(link.hash_name, []).append(link.hash)
return Hashes(good_hashes)

Expand All @@ -314,6 +314,7 @@ def from_path(self) -> Optional[str]:
return None
s = str(self.req)
if self.comes_from:
comes_from: Optional[str]
uranusjr marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(self.comes_from, str):
comes_from = self.comes_from
else:
Expand Down Expand Up @@ -345,7 +346,7 @@ def ensure_build_location(

# When parallel builds are enabled, add a UUID to the build directory
# name so multiple builds do not interfere with each other.
dir_name: str = canonicalize_name(self.name)
dir_name: str = canonicalize_name(self.req.name)
uranusjr marked this conversation as resolved.
Show resolved Hide resolved
if parallel_builds:
dir_name = f"{dir_name}_{uuid.uuid4().hex}"

Expand Down Expand Up @@ -388,6 +389,7 @@ def _set_requirement(self) -> None:
)

def warn_on_mismatching_name(self) -> None:
assert self.req is not None
metadata_name = canonicalize_name(self.metadata["Name"])
if canonicalize_name(self.req.name) == metadata_name:
# Everything is fine.
Expand Down Expand Up @@ -457,6 +459,7 @@ def is_wheel_from_cache(self) -> bool:
# Things valid for sdists
@property
def unpacked_source_directory(self) -> str:
assert self.source_dir, f"No source dir for {self}"
Copy link
Contributor Author

@hauntsaninja hauntsaninja Jun 20, 2023

Choose a reason for hiding this comment

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

I guess this is one place where we could raise an AssertionError where the existing code would not fail: if self.source_dir was an empty string. But assert self.source_dir appears many other times in this file, so I think is okay?

return os.path.join(
self.source_dir, self.link and self.link.subdirectory_fragment or ""
)
Expand Down Expand Up @@ -543,7 +546,7 @@ def prepare_metadata(self) -> None:
Under PEP 517 and PEP 660, call the backend hook to prepare the metadata.
Under legacy processing, call setup.py egg-info.
"""
assert self.source_dir
assert self.source_dir, f"No source dir for {self}"
details = self.name or f"from {self.link}"

if self.use_pep517:
Expand Down Expand Up @@ -592,18 +595,20 @@ def get_dist(self) -> BaseDistribution:
if self.metadata_directory:
return get_directory_distribution(self.metadata_directory)
elif self.local_file_path and self.is_wheel:
assert self.req is not None
return get_wheel_distribution(
FilesystemWheel(self.local_file_path), canonicalize_name(self.name)
FilesystemWheel(self.local_file_path),
canonicalize_name(self.req.name),
)
raise AssertionError(
f"InstallRequirement {self} has no metadata directory and no wheel: "
f"can't make a distribution."
)

def assert_source_matches_version(self) -> None:
assert self.source_dir
assert self.source_dir, f"No source dir for {self}"
version = self.metadata["version"]
if self.req.specifier and version not in self.req.specifier:
if self.req and self.req.specifier and version not in self.req.specifier:
logger.warning(
"Requested %s, but installing version %s",
self,
Expand Down Expand Up @@ -696,9 +701,10 @@ def _clean_zip_name(name: str, prefix: str) -> str:
name = name.replace(os.path.sep, "/")
return name

assert self.req is not None
path = os.path.join(parentdir, path)
name = _clean_zip_name(path, rootdir)
return self.name + "/" + name
return self.req.name + "/" + name

def archive(self, build_dir: Optional[str]) -> None:
"""Saves archive to provided build_dir.
Expand Down Expand Up @@ -777,8 +783,9 @@ def install(
use_user_site: bool = False,
pycompile: bool = True,
) -> None:
assert self.req is not None
scheme = get_scheme(
self.name,
self.req.name,
user=use_user_site,
home=home,
root=root,
Expand All @@ -792,7 +799,7 @@ def install(
prefix=prefix,
home=home,
use_user_site=use_user_site,
name=self.name,
name=self.req.name,
setup_py_path=self.setup_py_path,
isolated=self.isolated,
build_env=self.build_env,
Expand All @@ -805,7 +812,7 @@ def install(
assert self.local_file_path

install_wheel(
self.name,
self.req.name,
self.local_file_path,
scheme=scheme,
req_description=str(self.req),
Expand Down