Consider OS version when matching artifacts to platforms

src/Platforms.jl

@@ -19,28 +19,33 @@ nbits(::AnyPlatform) = nbits(default_host_platform)
 proc_family(::AnyPlatform) = "any"
 Base.show(io::IO, ::AnyPlatform) = print(io, "AnyPlatform")
 
+function _agnostic(p::Platform, keeps)
+    filtered_tags = (Symbol(k) => convert(String, v) for (k, v) in tags(p) if k ∈ keeps)
+    return Platform(arch(p)::String, os(p)::String; filtered_tags...)
+end
+_agnostic(p::AnyPlatform, _) = p
+
 """
     abi_agnostic(p::AbstractPlatform)
 
 Strip out any tags that are not the basic annotations like `libc` and `call_abi`.
 """
-function abi_agnostic(p::Platform)
-    keeps = ("libc", "call_abi", "os_version")
-    filtered_tags = Dict{Symbol,String}(Symbol(k) => v for (k, v) in tags(p) if k ∈ keeps)
-    return Platform(arch(p)::String, os(p)::String; filtered_tags...)
-end
-abi_agnostic(p::AnyPlatform) = p
+abi_agnostic(p::AbstractPlatform) = _agnostic(p, ("libc", "call_abi", "os_version"))
 
 """
-    abi_agnostic(p::AbstractPlatform)
+    libabi_agnostic(p::AbstractPlatform)
 
 Like `abi_agnostic`, but keep the sanitizer ABI tags.
 """
-function libabi_agnostic(p::Platform)
-    keeps = ("libc", "call_abi", "os_version", "sanitize")
-    filtered_tags = Dict{Symbol,String}(Symbol(k) => v for (k, v) in tags(p) if k ∈ keeps)
-    return Platform(arch(p)::String, os(p)::String; filtered_tags...)
-end
+libabi_agnostic(p::AbstractPlatform) = _agnostic(p, ("libc", "call_abi", "os_version", "sanitize"))
+
+"""
+    os_version_agnostic(p::AbstractPlatform)
+
+Strip out an `os_version` tag if it exists.
+"""
+os_version_agnostic(p::AbstractPlatform) =
+    _agnostic(p, filter(!in(("arch", "os", "os_version")), keys(tags(p))))
 
 platforms_match_with_sanitize(a::AbstractPlatform, b::AbstractPlatform) =
     platforms_match(a, b) && sanitize(a) == sanitize(b)

src/Rootfs.jl

@@ -46,6 +46,11 @@ struct CompilerShard
             target = abi_agnostic(target)
         end
 
+        # If this compiler shard has an os_version, that should be interpreted as the bound it is.
+        if target !== nothing && os_version(target::Platform) !== nothing
+            set_compare_strategy!(target::Platform, "os_version", compare_version_cap)
+        end
+
         # Construct our shiny new CompilerShard object
         return new(name, version, target, host, archive_type)
     end
@@ -59,6 +64,37 @@ function Base.:(==)(a::CompilerShard, b::CompilerShard)
            a.archive_type == b.archive_type
 end
 
+# For a `CompilerShard` to match a `Platform`, the shard's target with OS version stripped
+# must match, and the OS version if present must be less than or equal to the platform's.
+# This is effectively `platforms_match` with a `compare_version_cap` comparison strategy for
+# OS version, except it works "correctly" (IMO) when both arguments have that strategy set.
+# It also ignores any strategy that's set.
+function Base.BinaryPlatforms.platforms_match(cs::CompilerShard, p::AbstractPlatform)
+    if cs.target === nothing
+        return platforms_match(cs.host, p)
+    elseif !platforms_match(os_version_agnostic(cs.target), os_version_agnostic(p))
+        return false
+    else
+        sv = os_version(cs.target)
+        pv = os_version(p)
+        return sv === nothing || pv === nothing || pv >= sv
+    end
+end
+
+Base.BinaryPlatforms.platforms_match(p::AbstractPlatform, cs::CompilerShard) = platforms_match(cs, p)
+
+function Base.BinaryPlatforms.platforms_match(a::CompilerShard, b::CompilerShard)
+    if !platforms_match(a.host, b.host)
+        return false
+    elseif a.target === b.target === nothing
+        return true
+    elseif a.target !== nothing && b.target !== nothing
+        return platforms_match(a.target, b.target)
+    else
+        return false
+    end
+end
+
 """
     artifact_name(cs::CompilerShard)
 
@@ -78,11 +114,10 @@ function artifact_name(cs::CompilerShard)
     return "$(cs.name)$(target_str).v$(cs.version).$(triplet(cs.host)).$(ext)"
 end
 
-# The inverse of `artifact_name(cs)`
-function CompilerShard(art_name::String)
+function Base.tryparse(::Type{CompilerShard}, art_name::AbstractString)
     m = match(r"^([^-]+)(?:-(.+))?\.(v[\d\.]+(?:-[^\.]+)?)\.([^0-9].+-.+)\.(\w+)", art_name)
     if m === nothing
-        error("Unable to parse '$(art_name)'")
+        return nothing
     end
     return CompilerShard(
         m.captures[1],
@@ -93,6 +128,15 @@ function CompilerShard(art_name::String)
     )
 end
 
+# The inverse of `artifact_name(cs)`
+function CompilerShard(art_name::String)
+    cs = tryparse(CompilerShard, art_name)
+    if cs === nothing
+        error("Unable to parse '$(art_name)'")
+    end
+    return cs
+end
+
 const ALL_SHARDS = Ref{Union{Vector{CompilerShard},Nothing}}(nothing)
 function all_compiler_shards()::Vector{CompilerShard}
     if ALL_SHARDS[] === nothing
@@ -109,17 +153,10 @@ function all_compiler_shards()::Vector{CompilerShard}
             end
         end
         for name in names
-            cs = try
-                CompilerShard(name)
-            catch
-                continue
-            end
-
-            # If this compiler shard has an os_version, that should be interpreted as the bound it is.
-            if cs.target !== nothing && os_version(cs.target::Platform) !== nothing
-                set_compare_strategy!(cs.target::Platform, "os_version", compare_version_cap)
+            cs = tryparse(CompilerShard, name)
+            if cs !== nothing
+                push!(ALL_SHARDS[]::Vector{CompilerShard}, cs)
             end
-            push!(ALL_SHARDS[]::Vector{CompilerShard}, cs)
         end
     end
     return ALL_SHARDS[]::Vector{CompilerShard}
@@ -606,7 +643,7 @@ function choose_shards(p::AbstractPlatform;
 
         for cs in all_compiler_shards()
             if cs.name == name && cs.version == version &&
-               (target === nothing || platforms_match(cs.target, target)) &&
+               (target === nothing || platforms_match(cs, target)) &&
                cs.archive_type == archive_type
                 return cs
             end
@@ -711,7 +748,7 @@ function choose_shards(p::AbstractPlatform;
     else
         function find_latest_version(name)
             versions = [cs.version for cs in all_compiler_shards()
-                if cs.name == name && cs.archive_type == archive_type && platforms_match(something(cs.target, p), p)
+                if cs.name == name && cs.archive_type == archive_type && platforms_match(cs, p)
             ]
             isempty(versions) && error("No latest shard found for $name")
             return maximum(versions)
@@ -770,8 +807,8 @@ function supported_platforms(;exclude::Union{Vector{<:Platform},Function}=Return
         # BSDs
         Platform("x86_64", "macos"),
         Platform("aarch64", "macos"),
-        Platform("x86_64", "freebsd"),
-        Platform("aarch64", "freebsd"),
+        Platform("x86_64", "freebsd"; os_version="13.2"),
+        Platform("aarch64", "freebsd"; os_version="13.2"),
 
         # Windows
         Platform("i686", "windows"),

test/rootfs.jl

@@ -146,6 +146,7 @@ end
         shard = CompilerShard("GCCBootstrap", v"4.8.5", Platform("x86_64", "linux"; libc="musl"), :squashfs, target = platform)
         @test preferred_libgfortran_version(platform, shard) == v"3"
         @test preferred_cxxstring_abi(platform, shard) == "cxx03"
+        @test platforms_match(shard, platform)
         shard = CompilerShard("GCCBootstrap", v"5.2.0", Platform("x86_64", "linux"; libc="musl"), :squashfs, target = platform)
         @test preferred_libgfortran_version(platform, shard) == v"3"
         @test preferred_cxxstring_abi(platform, shard) == "cxx11"
@@ -217,6 +218,25 @@ end
         @test gcc_version(p, available_gcc_builds, [:c, :rust]) == [v"5.2.0", v"6.1.0"]
     end
 
+    @testset "OS version handling" begin
+        # `platforms_match` for `CompilerShard`s and `Platform`s
+        shard = CompilerShard("GCCBootstrap", v"10.2.0", Platform("x86_64", "linux"; libc="musl"),
+                              :squashfs; target=Platform("x86_64", "freebsd"; os_version=v"13.2"))
+        # Target is for FreeBSD 13.2 so 14.1 should match
+        @test platforms_match(shard, Platform("x86_64", "freebsd"; os_version="14.1"))
+        # Reversed argument order should behave identically
+        @test platforms_match(Platform("x86_64", "freebsd"; os_version="14.1"), shard)
+        # Method largely exists for disambiguation, at least make sure it doesn't error
+        @test platforms_match(shard, shard)
+        # Target is for FreeBSD 13.2 so 11.1 should not match
+        @test !platforms_match(shard, Platform("x86_64", "freebsd"; os_version=v"11.1"))
+
+        # We didn't have AArch64 for FreeBSD < 13.2
+        @test_throws ErrorException choose_shards(Platform("aarch64", "freebsd"; os_version=v"11.1"))
+        # Shards built for earlier FreeBSD versions should be available for newer ones
+        @test !isempty(choose_shards(Platform("aarch64", "freebsd"; os_version=v"69.420")))
+    end
+
     @testset "Compiler wrappers" begin
         platform = Platform("x86_64", "linux"; libc="musl")
         mktempdir() do bin_path