Merge pull request python-wheel-build#534 from dhellmann/fix-issues-w…

…riting-constraints fix some issues writing the constraints file during bootstrap
tiran · Jan 21, 2025 · c84ee7a · c84ee7a
2 parents 8ade718 + a925786
commit c84ee7a
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 9 deletions.
diff --git a/src/fromager/commands/bootstrap.py b/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()):

diff --git a/tests/test_bootstrap.py b/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": "a==2.7.0",
+                    "req_type": "toplevel",
+                    "req": "a==2.7.0",
+                },
+                {
+                    "key": "b==0.26.1",
+                    "req_type": "toplevel",
+                    "req": "b==0.26.1",
+                },
+            ],
+        },
+        "b==0.26.1": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "0.26.1",
+            "canonicalized_name": "b",
+            "edges": [],
+        },
+        "b==0.26.2": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "0.26.2",
+            "canonicalized_name": "b",
+            "edges": [],
+        },
+        "a==2.7.0": {
+            "download_url": "",
+            "pre_built": False,
+            "version": "2.7.0",
+            "canonicalized_name": "a",
+            "edges": [
+                {
+                    "key": "b==0.26.2",
+                    "req_type": "install",
+                    "req": "b<0.27.0,>=0.26.1",
+                },
+            ],
+        },
+    }
+    graph = dependency_graph.DependencyGraph.from_dict(raw_graph)
+    assert bootstrap.write_constraints_file(graph, buffer)
+    expected = textwrap.dedent("""
+        a==2.7.0
+        # NOTE: fromager selected b==0.26.2 from: ['0.26.1', '0.26.2']
+        b==0.26.2
+        """).lstrip()
+    assert expected == buffer.getvalue()