From a483e67a8cae14497a1e99c6e2af1907065de622 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Tue, 18 Feb 2025 18:50:32 -0800 Subject: [PATCH 1/3] Consider OS version when matching artifacts to platforms We currently largely ignore OS version, and for most platforms that doesn't really matter. But binaries built for a particular FreeBSD version are forward- but not backward-compatible with other FreeBSD versions, and that isn't accounted for when choosing artifacts. That means that whenever the FreeBSD version used in the RootFS is bumped, entries in Artifacts.toml corresponding to older versions need to be manually deleted so they don't get chosen instead. More care is needed here than I had anticipated, as `platforms_match` has documented but unfortunate behavior when both arguments have `compare_version_cap` set for their `os_version` comparison strategies. The function assumes that this case occurs only when both arguments are host platforms, and it compares their versions using `==`. That strategy is set explicitly for `CompilerShard` targets with a comment about treating the OS version as a bound, except it doesn't act as a bound as such if the comparator is e.g. `HostPlatform()`. --- src/Platforms.jl | 29 +++++++++++++++++------------ src/Rootfs.jl | 30 ++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/Platforms.jl b/src/Platforms.jl index f822b67c..4b572fe6 100644 --- a/src/Platforms.jl +++ b/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) diff --git a/src/Rootfs.jl b/src/Rootfs.jl index 571ddf0e..45e9a270 100644 --- a/src/Rootfs.jl +++ b/src/Rootfs.jl @@ -59,6 +59,28 @@ 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) + +Base.BinaryPlatforms.platforms_match(a::CompilerShard, b::CompilerShard) = + platforms_match(something(a.target, a.host), something(b.target, b.host)) + """ artifact_name(cs::CompilerShard) @@ -606,7 +628,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 +733,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 +792,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"), From dd1b4fe2269195cdc7def447a25b65a625acf989 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Wed, 19 Feb 2025 11:02:18 -0800 Subject: [PATCH 2/3] Tests --- test/rootfs.jl | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/rootfs.jl b/test/rootfs.jl index 6340efda..342de8b0 100644 --- a/test/rootfs.jl +++ b/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 isempty(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 From ad06cef68f717e608915b4bedf6ba0a8077454aa Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Wed, 19 Feb 2025 18:07:53 -0800 Subject: [PATCH 3/3] Stuff --- src/Rootfs.jl | 45 ++++++++++++++++++++++++++++++--------------- test/rootfs.jl | 2 +- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/Rootfs.jl b/src/Rootfs.jl index 45e9a270..c5a8ebd5 100644 --- a/src/Rootfs.jl +++ 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 @@ -78,8 +83,17 @@ end Base.BinaryPlatforms.platforms_match(p::AbstractPlatform, cs::CompilerShard) = platforms_match(cs, p) -Base.BinaryPlatforms.platforms_match(a::CompilerShard, b::CompilerShard) = - platforms_match(something(a.target, a.host), something(b.target, b.host)) +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) @@ -100,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], @@ -115,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 @@ -131,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} diff --git a/test/rootfs.jl b/test/rootfs.jl index 342de8b0..1657b5e3 100644 --- a/test/rootfs.jl +++ b/test/rootfs.jl @@ -232,7 +232,7 @@ end @test !platforms_match(shard, Platform("x86_64", "freebsd"; os_version=v"11.1")) # We didn't have AArch64 for FreeBSD < 13.2 - @test isempty(choose_shards(Platform("aarch64", "freebsd"; os_version=v"11.1"))) + @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