docs: add docstrings and comments to complex functions

Add docstrings to functions and inline comments to  non-obvious
code patterns across various files. Only targets genuinely complex
logic that would be hard for new contributors to understand.

Closes: #1046
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>

Author: LalatenduMohanty

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(
@@ -1181,7 +1184,15 @@ 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.
+
+        Strips the ``git+`` prefix, then checks for an ``@ref`` suffix in
+        the URL path. If the ref parses as a valid Python version, reuses
+        an existing clone when possible. Otherwise, clones to a temp
+        directory and extracts the version from package metadata. The clone
+        is moved to the standard work directory layout
+        (``<name>-<version>/<name>-<version>/``) once the version is known.
+        """
 
         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,14 @@ 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.
+
+    The returned hook caller uses a custom runner that merges three sources
+    of environment variables (in increasing priority):
+    1. ``extra_environ`` passed by the hook caller itself
+    2. ``override_environ`` from package-specific settings
+    3. virtualenv variables from ``build_env``
+    """
 
     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,14 @@ 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 via a log file (when log_filename is set) or an in-memory
+    pipe, then formats continuation lines with the current package prefix for
+    greppability. On failure, raises ``NetworkIsolationError`` instead of
+    ``CalledProcessError`` when the output contains network-related error
+    strings, so callers can distinguish network isolation failures.
+    """
     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,13 @@ 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.
+
+        Builds a map keyed by normalized package name (seeded with setuptools
+        as a default), overlays existing requirements, removes unwanted ones,
+        then applies update requirements. Only writes back if the resulting
+        set differs from the original, ignoring order.
+        """
         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,14 @@ 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.
+
+    Applies a multi-stage filtering pipeline to the project's package list:
+    PEP 792 status check -> legacy filename rejection -> sdist/wheel type
+    filter -> PEP 592 yanked check -> Python version compatibility ->
+    platform tag matching (with ``ignore_platform`` fallback). Optionally
+    substitutes ``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 +813,10 @@ def cache_key(self) -> str:
         raise NotImplementedError("GenericProvider does not implement caching")
 
     def find_candidates(self, identifier: typing.Any) -> Candidates:
+        # 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,13 @@ 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 ``.tmp``
+    file first, then atomically renames on success to avoid leaving partial
+    files. A nested function applies retry logic for transient network errors;
+    the outer function ensures the temp file is cleaned up on any failure.
+    """
     basename = (
         destination_filename
         if destination_filename
@@ -500,6 +507,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