fix(packages): stop a non-numeric version taking down registry lookup

docs/book/book.md

@@ -935,6 +935,23 @@ Request: freertos >= 10.5, mbedtls ^3.0
 +-------------------+
 ```
 
+#### Version ordering
+
+A request without a version resolves to the highest version of that package in
+the registry. `version` is a free-form string — the recipe format does not
+require dotted integers — so the ordering is defined as follows:
+
+| Rule | Example |
+|---|---|
+| A leading `v` or `V` is ignored | `v2.9.3` ranks with `2.9.3` |
+| All-digit components compare numerically | `1.10.0` > `1.9.0` |
+| Any other component compares as text, below any numeric one | `1.x` < `1.0` |
+| A `-` or `+` suffix ranks below the same version without one | `3.6.0-rc1` < `3.6.0` |
+
+Every version string has a place in this order, including ones that carry no
+numbers at all (`main`), so one unusual recipe cannot break lookup for the
+packages around it.
+
 ---
 
 ## Chapter 13: SDK Generation

ebuild/packages/registry.py

@@ -9,11 +9,64 @@
 
 from __future__ import annotations
 
+import re
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Dict, List, Optional, Tuple
 
 from ebuild.packages.recipe import PackageRecipe, RecipeError, load_recipe
 
+# Everything from the first '-' or '+' is a suffix: a pre-release tag
+# ("3.6.0-rc1") or build metadata ("1.3.1+patch2").
+_SUFFIX_SPLIT = re.compile(r"[-+]")
+
+_ComponentKey = Tuple[int, int, str]
+
+
+def _component_key(component: str) -> _ComponentKey:
+    """Order one dot-separated component of a version string.
+
+    Numeric components compare numerically, so 1.10.0 still sorts above
+    1.9.0. Anything else compares as text and ranks below any numeric
+    component, which keeps the ordering total without inventing a meaning
+    for identifiers the recipe format does not define.
+    """
+    if component.isdigit():
+        return (1, int(component), "")
+    return (0, 0, component)
+
+
+def version_sort_key(version: str) -> tuple:
+    """Sort key for a package version string.
+
+    ``PackageRecipe.validate()`` accepts any non-empty version, and real
+    embedded recipes use more than dotted integers: a leading ``v``
+    (``v2.9.3``, littlefs's own tag format), pre-release tags
+    (``3.6.0-rc1``) and build metadata (``1.3.1+patch2``). Ordering used to
+    be ``[int(x) for x in version.split('.')]``, which raised ValueError on
+    every one of them -- and did so from ``get()``, ``list_packages()`` and
+    ``list_all_versions()``, so a single such recipe anywhere in the
+    registry took down package lookup for the whole project.
+
+    Ordering rules:
+      * an optional leading ``v`` or ``V`` is ignored;
+      * the release part is compared component by component, numerically
+        where a component is all digits;
+      * a version carrying a pre-release or build suffix sorts below the
+        otherwise-equal version without one, so 3.6.0-rc1 < 3.6.0;
+      * nothing raises -- any string has a place in the order.
+    """
+    text = version.strip()
+    if text[:1] in ("v", "V"):
+        text = text[1:]
+
+    parts = _SUFFIX_SPLIT.split(text, maxsplit=1)
+    release = tuple(_component_key(c) for c in parts[0].split("."))
+
+    if len(parts) == 1:
+        return (release, 1, ())
+    suffix = tuple(_component_key(c) for c in re.split(r"[.\-+]", parts[1]))
+    return (release, 0, suffix)
+
 
 class PackageRegistry:
     """Registry of available package recipes.
@@ -74,8 +127,7 @@ def get(self, name: str, version: Optional[str] = None) -> Optional[PackageRecip
         if version:
             return versions.get(version)
 
-        latest_version = sorted(versions.keys(), key=lambda v: [int(x) for x in v.split('.')])[-1]
-        return versions[latest_version]
+        return versions[max(versions, key=version_sort_key)]
 
     def has(self, name: str, version: Optional[str] = None) -> bool:
         """Check if a recipe exists."""
@@ -86,20 +138,13 @@ def list_packages(self) -> List[PackageRecipe]:
         result = []
         for name in sorted(self._recipes.keys()):
             versions = self._recipes[name]
-            latest = sorted(versions.keys(), key=lambda v: [int(x) for x in v.split('.')])[-1]
-            result.append(versions[latest])
+            result.append(versions[max(versions, key=version_sort_key)])
         return result
 
     def list_all_versions(self, name: str) -> List[PackageRecipe]:
         """Return all versions of a package."""
         versions = self._recipes.get(name, {})
-        return [
-                versions[v]
-                for v in sorted(
-                    versions.keys(),
-                    key=lambda v: [int(x) for x in v.split(".")],
-                 )
-              ]
+        return [versions[v] for v in sorted(versions, key=version_sort_key)]
 
     @property
     def package_count(self) -> int:

tests/ebuild/test_package_registry.py

@@ -1,5 +1,7 @@
+import pytest
+
 from ebuild.packages.recipe import PackageRecipe
-from ebuild.packages.registry import PackageRegistry
+from ebuild.packages.registry import PackageRegistry, version_sort_key
 
 
 def make_recipe(version: str) -> PackageRecipe:
@@ -10,6 +12,13 @@ def make_recipe(version: str) -> PackageRecipe:
     )
 
 
+def registry_with(*versions: str) -> PackageRegistry:
+    registry = PackageRegistry()
+    for version in versions:
+        registry._register(make_recipe(version))
+    return registry
+
+
 def test_list_all_versions_uses_numeric_version_order():
     registry = PackageRegistry()
 
@@ -24,3 +33,84 @@ def test_list_all_versions_uses_numeric_version_order():
         "1.9.0",
         "1.10.0",
     ]
+
+
+# --- Versions that are not dotted integers -----------------------------------
+#
+# PackageRecipe.validate() accepts any non-empty version string, so these all
+# load and register. Ordering used to be [int(x) for x in v.split('.')], which
+# raised ValueError on every one of them.
+
+
+@pytest.mark.parametrize(
+    "version",
+    [
+        "v2.9.3",        # littlefs publishes its releases with a leading v
+        "3.6.0-rc1",     # pre-release tag
+        "1.3.1+patch2",  # build metadata
+        "2024.06",       # date-stamped release
+        "main",          # a branch, not a release
+        "",              # degenerate, but reachable through _register()
+    ],
+)
+def test_lookup_survives_a_non_numeric_version(version):
+    registry = registry_with(version)
+
+    assert registry.get("demo").version == version
+    assert [r.version for r in registry.list_packages()] == [version]
+    assert [r.version for r in registry.list_all_versions("demo")] == [version]
+
+
+def test_one_odd_version_does_not_break_lookup_of_the_rest():
+    """A single unparseable version used to take down the whole registry.
+
+    get() with no version scans every version of the package, and
+    list_packages() scans every package -- which the resolver calls to build
+    its 'package not found' message. One recipe with a 'v' prefix therefore
+    turned an ordinary lookup anywhere in the project into a ValueError.
+    """
+    registry = registry_with("1.0.0", "v9.9.9", "1.2.0")
+
+    assert registry.get("demo").version == "v9.9.9"
+    assert registry.get("demo", "1.2.0").version == "1.2.0"
+    assert len(registry.list_all_versions("demo")) == 3
+
+
+def test_leading_v_does_not_change_precedence():
+    registry = registry_with("v2.9.3", "2.10.0")
+
+    assert registry.get("demo").version == "2.10.0"
+
+
+def test_prerelease_sorts_below_its_release():
+    registry = registry_with("3.6.0", "3.6.0-rc1", "3.6.0-rc2")
+
+    assert [r.version for r in registry.list_all_versions("demo")] == [
+        "3.6.0-rc1",
+        "3.6.0-rc2",
+        "3.6.0",
+    ]
+    assert registry.get("demo").version == "3.6.0"
+
+
+def test_build_metadata_does_not_outrank_the_next_release():
+    registry = registry_with("1.3.1+patch2", "1.3.2")
+
+    assert registry.get("demo").version == "1.3.2"
+
+
+def test_version_ordering_is_total_and_never_raises():
+    """Every pair must be comparable, in both directions, without raising."""
+    versions = [
+        "1.0.0", "1.0", "1.0.1", "v1.0.1", "2024.06", "1.0.0-rc1",
+        "1.0.0+meta", "main", "", "1.0.0-alpha.1", "10.0.0",
+    ]
+    keys = [version_sort_key(v) for v in versions]
+
+    for left in keys:
+        for right in keys:
+            assert (left < right) or (left >= right)
+
+    assert sorted(versions, key=version_sort_key) == sorted(
+        versions, key=version_sort_key
+    )