fix some issues writing the constraints file during bootstrap

We're seeing some cases downstream where fromager
fails with an exception because it tries to remove
something from the unresolved dependency list when
the item isn't in the list. This seems to be
caused by having multiple candidate versions for a
package where both versions could be used by all
consumers.

This change resolves the issue by stopping the
resolution loop when we find a version of a
package that can work for all consumers. The loop
now iterates from the newest version to the
oldest, to ensure we choose the newest package
that works.

There is also some error handling in case this fix
is not sufficient to eliminate the original
ValueError.

I also noticed that a log message that we supposed
to print debug info about the candidate versions
was using the wrong list variable so it was always
printing the same values, regardless of the
package. I cleaned that up and added additional
debug logging.

Author: dhellmann

src/fromager/commands/bootstrap.py

@@ -219,7 +219,7 @@ def write_constraints_file(
         # Make copy of the original list and loop over unresolved dependencies
         for dep_name, nodes in unresolved_dependencies[:]:
             # Track which versions can be used by which parent requirement.
-            usable_versions: dict[str, list[Version]] = {}
+            usable_versions: dict[Version, list[Version]] = {}
             # Track how many total users of a requirement (by name) there are so we
             # can tell later if any version can be used by all of them.
             user_counter = 0
@@ -245,7 +245,7 @@ def write_constraints_file(
                     for matching_version in parent_edge.req.specifier.filter(
                         dep_versions
                     ):
-                        usable_versions.setdefault(str(matching_version), []).append(
+                        usable_versions.setdefault(matching_version, []).append(
                             parent_edge.destination_node.version
                         )
                     user_counter += 1
@@ -254,18 +254,34 @@ def write_constraints_file(
             # and output that if we find it. Otherwise, include a warning and report
             # all versions so a human reading the file can make their own decision
             # about how to resolve the conflict.
-            for v, users in usable_versions.items():
-                if len(users) == user_counter:
-                    version_strs = [str(v) for v in sorted(versions)]
+            for v, users in reversed(sorted(usable_versions.items())):
+                if len(users) != user_counter:
                     logger.debug(
-                        "%s: selecting %s from multiple candidates %s",
+                        "%s: version %s is useable by %d of %d consumers, skipping it",
                         dep_name,
                         v,
-                        version_strs,
+                        len(users),
+                        user_counter,
                     )
-                    resolved[dep_name] = Version(v)
-                    resolved_something = True
+                    continue
+                version_strs = [str(v) for v in reversed(sorted(dep_versions))]
+                logger.debug(
+                    "%s: selecting %s from multiple candidates %s",
+                    dep_name,
+                    v,
+                    version_strs,
+                )
+                resolved[dep_name] = v
+                resolved_something = True
+                try:
                     unresolved_dependencies.remove((dep_name, nodes))
+                except ValueError:
+                    logger.debug(
+                        "%s: %s not in unresolved dependencies list, ignoring",
+                        dep_name,
+                        (dep_name, nodes),
+                    )
+                break
 
     # Write resolved versions to constraints file
     for dep_name, resolved_version in sorted(resolved.items()):

tests/test_bootstrap.py

@@ -261,3 +261,62 @@ def test_write_constraints_file_duplicates():
         d==1.0
         """).lstrip()
     assert expected == buffer.getvalue()
+
+
+def test_write_constraints_file_multiples():
+    buffer = io.StringIO()
+    raw_graph = {
+        "": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "0",
+            "canonicalized_name": "",
+            "edges": [
+                {
+                    "key": "docling==2.7.0",
+                    "req_type": "toplevel",
+                    "req": "docling==2.7.0",
+                },
+                {
+                    "key": "deepsearch-glm==0.26.1",
+                    "req_type": "toplevel",
+                    "req": "deepsearch-glm==0.26.1",
+                },
+            ],
+        },
+        "deepsearch-glm==0.26.1": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "0.26.1",
+            "canonicalized_name": "deepsearch-glm",
+            "edges": [],
+        },
+        "deepsearch-glm==0.26.2": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "0.26.2",
+            "canonicalized_name": "deepsearch-glm",
+            "edges": [],
+        },
+        "docling==2.7.0": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "2.7.0",
+            "canonicalized_name": "docling",
+            "edges": [
+                {
+                    "key": "deepsearch-glm==0.26.2",
+                    "req_type": "install",
+                    "req": "deepsearch-glm<0.27.0,>=0.26.1",
+                },
+            ],
+        },
+    }
+    graph = dependency_graph.DependencyGraph.from_dict(raw_graph)
+    assert bootstrap.write_constraints_file(graph, buffer)
+    expected = textwrap.dedent("""
+        # NOTE: fromager selected deepsearch-glm==0.26.2 from: ['0.26.1', '0.26.2']
+        deepsearch-glm==0.26.2
+        docling==2.7.0
+        """).lstrip()
+    assert expected == buffer.getvalue()