Merge pull request #1047 from LalatenduMohanty/add_docstrings_non_trivial_code

docs: add docstrings and comments to complex functions

Author: mergify[bot]

src/fromager/bootstrapper.py

@@ -1078,6 +1078,9 @@ def _look_for_existing_wheel(
         if not wheel_filename:
             return None, None
 
+        # Re-validate build tag from the actual wheel metadata because
+        # finders.find_wheel matches by filename prefix, which may not
+        # enforce an exact build tag match.
         _, _, build_tag, _ = wheels.extract_info_from_wheel_file(req, wheel_filename)
         if expected_build_tag and expected_build_tag != build_tag:
             logger.info(
@@ -1185,7 +1188,12 @@ def _unpack_metadata_from_wheel(
             return None
 
     def _resolve_version_from_git_url(self, req: Requirement) -> tuple[str, Version]:
-        "Return path to the cloned git repository and the package version."
+        """Resolve source path and version from a ``git+`` URL.
+
+        Parses the URL for an ``@ref`` version hint. If the ref is a valid
+        version, reuses an existing clone when possible. Otherwise, clones
+        the repo and extracts the version from package metadata.
+        """
 
         if not req.url:
             raise ValueError(f"unable to resolve from URL with no URL in {req}")

src/fromager/dependencies.py

@@ -81,6 +81,8 @@ def _filter_requirements(
     for r in requirements:
         if not isinstance(r, Requirement):
             r = Requirement(r)
+        # Use the parent's extras for marker evaluation because PEP 508
+        # markers like `extra == "foo"` refer to the parent's install extras.
         if requirements_file.evaluate_marker(req, r, req.extras):
             requires.add(r)
         else:
@@ -529,7 +531,13 @@ def get_build_backend_hook_caller(
     build_env: build_environment.BuildEnvironment,
     log_filename: str | None = None,
 ) -> pyproject_hooks.BuildBackendHookCaller:
-    """Create pyproject_hooks build backend caller"""
+    """Create a ``BuildBackendHookCaller`` with merged environment variables.
+
+    Environment variables are merged from three sources, where later
+    sources override earlier ones: hook-provided ``extra_environ``,
+    package-specific ``override_environ``, and ``build_env`` virtualenv
+    variables.
+    """
 
     def _run_hook_with_extra_environ(
         cmd: typing.Sequence[str],

src/fromager/dependency_graph.py

@@ -169,6 +169,8 @@ def _traverse_install_requirements(
         start_edges: list[DependencyEdge],
         visited: set[str],
     ) -> typing.Iterable[DependencyEdge]:
+        # Depth-first walk of install requirement edges, skipping nodes
+        # already visited to avoid loops in the dependency graph.
         for edge in start_edges:
             if edge.key in visited:
                 continue
@@ -220,6 +222,9 @@ def from_dict(
         graph_dict: dict[str, dict[str, typing.Any]],
     ) -> DependencyGraph:
         graph = cls()
+        # Stack-based DFS to reconstruct graph from serialized dict,
+        # skipping nodes already visited to avoid processing shared
+        # dependencies twice.
         stack = [ROOT]
         visited = set()
         while stack:

src/fromager/external_commands.py

@@ -58,7 +58,13 @@ def run(
     log_filename: str | None = None,
     stdin: TextIOWrapper | None = None,
 ) -> str:
-    """Call the subprocess while logging output"""
+    """Run a subprocess with optional network isolation and structured logging.
+
+    Captures output to a log file or in-memory pipe and prefixes each
+    line with the current package name for easier searching. Raises
+    ``NetworkIsolationError`` instead of ``CalledProcessError`` when the
+    failure output indicates a network access problem.
+    """
     if extra_environ is None:
         extra_environ = {}
     env = os.environ.copy()

src/fromager/finders.py

@@ -40,6 +40,14 @@ def find_sdist(
     req: Requirement,
     dist_version: str,
 ) -> pathlib.Path | None:
+    """Find an sdist archive in downloads_dir for the given requirement.
+
+    First checks for a plugin-provided exact filename. If no plugin match,
+    tries four naming conventions (underscore-normalized, canonical, original,
+    and dotted) with case-insensitive comparison to handle inconsistent
+    sdist naming across the Python ecosystem. Case-insensitive globbing
+    is not available before Python 3.12.
+    """
     sdist_file_name = overrides.find_and_invoke(
         req.name,
         "expected_source_archive_name",
@@ -95,6 +103,13 @@ def find_wheel(
     dist_version: str,
     build_tag: BuildTag = (),
 ) -> pathlib.Path | None:
+    """Find a wheel file in downloads_dir for the given requirement.
+
+    Tries four naming conventions (PEP 427 transformed, canonical, original,
+    and dotted), each suffixed with the build tag when present. Uses
+    case-insensitive ``startswith`` matching rather than exact base comparison
+    because wheel filenames include platform/Python tags after the version.
+    """
     filename_prefix = _dist_name_to_filename(req.name)
     canonical_name = canonicalize_name(req.name)
     # if build tag is 0 then we can ignore to handle non tagged wheels for backward compatibility
@@ -140,6 +155,15 @@ def find_source_dir(
     req: Requirement,
     dist_version: str,
 ) -> pathlib.Path | None:
+    """Find the unpacked source directory for a requirement.
+
+    Tries three strategies in order:
+    1. Plugin override providing the exact directory name
+    2. Derive from the sdist archive name (strip extension), assuming the
+       unpacked layout is ``<base>/<base>/`` (double-nested)
+    3. Fall back to four naming conventions with case-insensitive matching,
+       returning ``<match>/<match>/`` (same double-nested convention)
+    """
     sdir_name_func = overrides.find_override_method(
         req.name, "expected_source_directory_name"
     )

src/fromager/pyproject.py

@@ -93,6 +93,12 @@ def _default_build_system(self, doc: tomlkit.TOMLDocument) -> TomlDict:
         return build_system
 
     def _update_build_requires(self, build_system: TomlDict) -> None:
+        """Merge, remove, and deduplicate build requirements.
+
+        Starts with setuptools as a default, adds the existing requirements,
+        removes unwanted ones, then applies updates. Only writes back if
+        the resulting set differs from the original.
+        """
         old_requires = build_system[BUILD_REQUIRES]
         # always include setuptools
         req_map: dict[NormalizedName, list[Requirement]] = {

src/fromager/requirements_file.py

@@ -65,6 +65,8 @@ def evaluate_marker(
     if not extras:
         marker_envs = [default_env]
     else:
+        # Evaluate once per extra because PEP 508 markers like `extra == "foo"`
+        # can only match one extra value at a time.
         marker_envs = [default_env.copy() | {"extra": e} for e in extras]
 
     for marker_env in marker_envs:

src/fromager/resolver.py

@@ -222,7 +222,13 @@ def get_project_from_pypi(
     *,
     override_download_url: str | None = None,
 ) -> Candidates:
-    """Return candidates created from the project name and extras."""
+    """Fetch and filter package candidates from a PyPI-compatible server.
+
+    Filters the project's package list by: project status, filename
+    validity, package type (sdist/wheel), yanked status, Python version
+    compatibility, and platform tags. Can substitute
+    ``override_download_url`` into each candidate's URL.
+    """
     found_candidates: set[str] = set()
     ignored_candidates: set[str] = set()
     logger.debug("%s: getting available versions from %s", project, sdist_server_url)
@@ -806,6 +812,13 @@ def cache_key(self) -> str:
         raise NotImplementedError("GenericProvider does not implement caching")
 
     def find_candidates(self, identifier: typing.Any) -> Candidates:
+        """Find matching candidates from the version source.
+
+        Accepts three input formats from _version_source:
+        1. Candidate objects (used directly)
+        2. (url, Version) tuples
+        3. (url, str) tuples (version parsed via _match_function)
+        """
         candidates: list[Candidate] = []
         version: Version | None
         for item in self._version_source(identifier):

src/fromager/sources.py

@@ -295,6 +295,12 @@ def download_url(
     url: str,
     destination_filename: str | None = None,
 ) -> pathlib.Path:
+    """Download a URL to destination_dir, returning the local path.
+
+    Returns immediately if the file already exists. Downloads to a
+    temporary file first and renames it on success to avoid leaving
+    partial files. Retries on transient network errors.
+    """
     basename = (
         destination_filename
         if destination_filename
@@ -500,6 +506,13 @@ def prepare_source(
     source_filename: pathlib.Path,
     version: Version,
 ) -> pathlib.Path:
+    """Unpack and prepare source for building.
+
+    Git URL sources skip the plugin system and are prepared directly.
+    Non-git sources go through ``find_and_invoke`` which may call a
+    package-specific override. The plugin may return a ``Path`` or a
+    ``(Path, bool)`` tuple; both forms are handled.
+    """
     if req.url:
         logger.info(
             "preparing source cloned from %s into %s, ignoring any plugins",

src/fromager/vendor_rust.py

@@ -56,7 +56,7 @@ def _cargo_shrink(crate_dir: pathlib.Path) -> None:
         for filename in crate_dir.glob(pattern):
             logger.debug("removing '%s'", filename.relative_to(crate_dir.parent))
             filename.unlink()
-            filename.touch()  # create empty file
+            filename.touch()  # keep empty placeholder so cargo doesn't error on missing files
             removed_files.append(str(filename.relative_to(crate_dir)))
 
     # update checksums