external tool upgrade: fixed width + alphabetical sort for Platforms (#23095)

This implements the -- I think -- consensus --position from #23045,
namely:
 * Lexicographic sort of platform
 * Fixed width platform: yes

Author: cburroughs

build-support/bin/external_tool_upgrade.py

@@ -35,7 +35,7 @@
     replace_class_variables,
 )
 from packaging.specifiers import InvalidSpecifier, SpecifierSet
-from packaging.version import Version
+from packaging.version import InvalidVersion, Version
 from tqdm import tqdm
 
 logger = logging.getLogger(__name__)
@@ -44,6 +44,13 @@
 # The ExternalToolVersion class is copied here to avoid depending on pants.
 # This makes it possible to run this as a standalone script with uv or use a
 # separate resolve in pants.
+
+# Max platform name width for consistent formatting. Hardcoded rather than
+# computed from pants.engine.platform.Platform to maintain independent resolves.
+# If Platform gains values, update this constant.
+PLATFORM_WIDTH = 12
+
+
 @dataclass(frozen=True)
 class ExternalToolVersion:
     version: str
@@ -53,7 +60,8 @@ class ExternalToolVersion:
     url_override: str | None = None
 
     def encode(self) -> str:
-        parts = [self.version, self.platform, self.sha256, str(self.filesize)]
+        padded_platform = self.platform.ljust(PLATFORM_WIDTH)
+        parts = [self.version, padded_platform, self.sha256, str(self.filesize)]
         if self.url_override:
             parts.append(self.url_override)
         return "|".join(parts)
@@ -66,6 +74,29 @@ def decode(cls, version_str: str) -> ExternalToolVersion:
         return cls(version, platform, sha256, int(filesize), url_override=url_override)
 
 
+def _version_key(version_str: str) -> tuple[int, Version | str]:
+    """Return a sort key for version strings, handling non-PEP 440 versions.
+
+    Valid PEP 440 versions get priority (1, ...) so they sort first with reverse=True. Invalid
+    versions get (0, ...) and fall back to string comparison.
+    """
+    try:
+        return (1, Version(version_str.lstrip("v")))
+    except InvalidVersion:
+        return (0, version_str)
+
+
+def sorted_by_version_and_platform(
+    versions: list[ExternalToolVersion],
+) -> list[ExternalToolVersion]:
+    """Sort by version descending, then platform alphabetically.
+
+    See https://github.com/pantsbuild/pants/issues/23045
+    """
+    by_platform = sorted(versions, key=lambda etv: etv.platform)
+    return sorted(by_platform, key=lambda etv: _version_key(etv.version), reverse=True)
+
+
 def format_string_to_regex(format_string: str) -> re.Pattern:
     r"""Converts a format string to a regex.
 
@@ -356,8 +387,7 @@ def main():
 
         fetched_versions = set(external_versions)
 
-        known_versions = list(existing_versions | fetched_versions)
-        known_versions.sort(key=lambda tu: (Version(tu.version), tu.platform), reverse=True)
+        known_versions = sorted_by_version_and_platform(list(existing_versions | fetched_versions))
 
         if args.version_constraint:
             # Only upgrade if the newest matching version is greater than current default

build-support/bin/external_tool_upgrade_test.py

@@ -3,7 +3,12 @@
 import doctest
 
 import external_tool_upgrade
-from external_tool_upgrade import ExternalToolVersion, filter_versions_by_constraint
+from external_tool_upgrade import (
+    PLATFORM_WIDTH,
+    ExternalToolVersion,
+    filter_versions_by_constraint,
+    sorted_by_version_and_platform,
+)
 from packaging.version import Version
 
 
@@ -151,3 +156,65 @@ def test_version_constraint_with_v_prefix_upgrades_correctly() -> None:
 
     result = _select_default_version_with_constraint(known_versions, current_default, constraint)
     assert result == "v5.0"
+
+
+def test_encode_pads_platform_to_fixed_width() -> None:
+    v = ExternalToolVersion("1.0", "linux_arm64", "abc123", 100)
+    encoded = v.encode()
+    parts = encoded.split("|")
+    assert parts[1] == "linux_arm64 "
+    assert len(parts[1]) == PLATFORM_WIDTH
+
+
+def test_encode_no_padding_needed_for_max_width_platform() -> None:
+    v = ExternalToolVersion("1.0", "macos_x86_64", "abc123", 100)
+    encoded = v.encode()
+    parts = encoded.split("|")
+    assert parts[1] == "macos_x86_64"
+    assert len(parts[1]) == PLATFORM_WIDTH
+
+
+def test_encode_decode_round_trip() -> None:
+    original = ExternalToolVersion("2.0", "linux_arm64", "abc123def456", 999)
+    decoded = ExternalToolVersion.decode(original.encode())
+    assert decoded == original
+
+
+def test_sorted_by_version_and_platform() -> None:
+    versions = [
+        ExternalToolVersion("2.0", "macos_x86_64", "a", 1),
+        ExternalToolVersion("1.0", "linux_arm64", "b", 2),
+        ExternalToolVersion("2.0", "linux_arm64", "c", 3),
+        ExternalToolVersion("1.0", "macos_x86_64", "d", 4),
+        ExternalToolVersion("2.0", "linux_x86_64", "e", 5),
+    ]
+    result = sorted_by_version_and_platform(versions)
+
+    expected_order = [
+        ("2.0", "linux_arm64"),
+        ("2.0", "linux_x86_64"),
+        ("2.0", "macos_x86_64"),
+        ("1.0", "linux_arm64"),
+        ("1.0", "macos_x86_64"),
+    ]
+    actual_order = [(v.version, v.platform) for v in result]
+    assert actual_order == expected_order
+
+
+def test_sorted_by_version_and_platform_with_non_pep440_versions() -> None:
+    versions = [
+        ExternalToolVersion("v2.1.0-M5-18-gfebf9838c", "linux_x86_64", "a", 1),
+        ExternalToolVersion("v2.1.24", "linux_x86_64", "b", 2),
+        ExternalToolVersion("v2.1.6", "linux_x86_64", "c", 3),
+        ExternalToolVersion("v2.0.16-169-g194ebc55c", "linux_x86_64", "d", 4),
+    ]
+    result = sorted_by_version_and_platform(versions)
+
+    expected_order = [
+        "v2.1.24",
+        "v2.1.6",
+        "v2.1.0-M5-18-gfebf9838c",
+        "v2.0.16-169-g194ebc55c",
+    ]
+    actual_order = [v.version for v in result]
+    assert actual_order == expected_order