Skip to content

Commit

Permalink
[Dependencies] Require compat argument for runtime dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
giordano committed Jun 24, 2023
1 parent 944f07c commit d9774e4
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/BinaryBuilderBase.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export HostPlatform, platform_dlext, valid_dl_path, arch, libc,
Platform, AnyPlatform

export AbstractSource, AbstractDependency, SetupSource, PatchSource,
resolve_jlls, coerce_dependency, coerce_source, Runner,
resolve_jlls, coerce_source, Runner,
generate_compiler_wrappers!, preferred_runner, CompilerShard, UserNSRunner,
DockerRunner, choose_shards, exeext, preferred_libgfortran_version,
preferred_cxxstring_abi, gcc_version, available_gcc_builds, getversion,
Expand Down
39 changes: 17 additions & 22 deletions src/Dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,8 @@ the latest version of the package compatible with the environment will be
automatically chosen by the package resolver, unless `compat` is specified, see
below.
The optional keyword argument `compat` can be used to specify a string for use
in the `Project.toml` of the generated Julia package. If `compat` is non-empty
and `build_version` is not passed, the latter defaults to the minimum version
compatible with the `compat` specifier.
The keyword argument `compat` must be used to specify a string for use in the `Project.toml` of the generated Julia package.
If `build_version` is not passed, the minimum version compatible with the `compat` specifier is used as build version.
The optional keyword argument `platforms` is a vector of `AbstractPlatform`s
which indicates for which platforms the dependency should be used. By default
Expand Down Expand Up @@ -123,9 +121,12 @@ struct Dependency <: AbstractDependency
if build_version spec
throw(ArgumentError("build_version and compat for $(pkg) are incompatible"))
end
if pkg.version != PKG_VERSIONS.VersionSpec("*") && !(pkg.version in spec)
throw(ArgumentError("PackageSpec version and compat for $(pkg) are incompatible"))
if pkg.version != PKG_VERSIONS.VersionSpec("*")
compatible_p = pkg.version isa PKG_VERSIONS.VersionSpec ? !isempty(intersect(pkg.version, spec)) : (pkg.version in spec)
compatible_p || throw(ArgumentError("""PackageSpec version and compat ("$(compat)") for $(pkg) are incompatible"""))
end
else
throw(ArgumentError("""Dependency("$(getname(pkg))") must have a non-empty compat bound."""))
end
if top_level
@warn("Dependency(\"$(getname(pkg))\") was defined as top-level but this is deprecated, use `RuntimeDependency` instead")
Expand Down Expand Up @@ -154,8 +155,7 @@ Define a binary dependency that is only listed as dependency of the generated JL
but its artifact is not installed in the prefix during the build. The `dep` argument can be
either a string with the name of the JLL package or a `Pkg.PackageSpec`.
The optional keyword argument `compat` can be used to specify a string for use
in the `Project.toml` of the generated Julia package.
The keyword argument `compat` must be used to specify a string for use in the `Project.toml` of the generated Julia package.
The optional keyword argument `platforms` is a vector of `AbstractPlatform`s which indicates
for which platforms the dependency should be used. By default `platforms=[AnyPlatform()]`,
Expand All @@ -176,9 +176,12 @@ struct RuntimeDependency <: AbstractDependency
top_level::Bool=false)
if !isempty(compat)
spec = PKG_VERSIONS.semver_spec(compat) # verify compat is valid
if pkg.version != PKG_VERSIONS.VersionSpec("*") && !(pkg.version in spec)
throw(ArgumentError("PackageSpec version and compat for $(pkg) are incompatible"))
if pkg.version != PKG_VERSIONS.VersionSpec("*")
compatible_p = pkg.version isa PKG_VERSIONS.VersionSpec ? !isempty(intersect(pkg.version, spec)) : (pkg.version in spec)
compatible_p || throw(ArgumentError("""PackageSpec version and compat ("$(compat)") for $(pkg) are incompatible"""))
end
else
throw(ArgumentError("""RuntimeDependency("$(getname(pkg))") must have a non-empty compat bound."""))
end
if top_level
if !(isempty(platforms) || all(==(AnyPlatform()), platforms))
Expand Down Expand Up @@ -276,7 +279,9 @@ filter_platforms(deps::AbstractVector{<:AbstractDependency}, p::AbstractPlatform
function registry_resolve!(ctx, dependencies::Vector{<:AbstractDependency})
resolved_dependencies = Pkg.Types.registry_resolve!(ctx.registries, getpkg.(dependencies))
for idx in eachindex(dependencies)
dependencies[idx] = typeof(dependencies[idx])(resolved_dependencies[idx]; platforms=dependencies[idx].platforms)
dependencies[idx] = typeof(dependencies[idx])(resolved_dependencies[idx];
compat=dependencies[idx].compat,
platforms=dependencies[idx].platforms)
end
return dependencies
end
Expand All @@ -300,8 +305,7 @@ function resolve_jlls(dependencies::Vector; ctx = Pkg.Types.Context(), outs=stdo
end

# Don't clobber caller
# XXX: Coercion is needed as long as we support old-style dependencies.
dependencies = deepcopy(coerce_dependency.(dependencies))
dependencies = deepcopy(dependencies)

# If all dependencies already have a UUID, return early
if all(x->getpkg(x).uuid !== nothing, dependencies)
Expand Down Expand Up @@ -378,12 +382,3 @@ function dependencify(d::Dict)
end
error("Cannot convert to dependency")
end


# XXX: compatibility functions. These are needed until we support old-style
# dependencies.
coerce_dependency(dep::AbstractDependency) = dep
function coerce_dependency(dep)
@warn "Using PackageSpec or string as dependency is deprecated, use Dependency instead"
Dependency(dep)
end
82 changes: 36 additions & 46 deletions test/dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,44 +22,48 @@ end

@testset "Dependencies" begin
name = "Foo_jll"
dep = Dependency(PackageSpec(; name = name); platforms=supported_platforms(; experimental=true, exclude=!Sys.isapple))
@test Dependency(name) dep
dep = Dependency(PackageSpec(; name = name); compat="0", platforms=supported_platforms(; experimental=true, exclude=!Sys.isapple))
@test Dependency(name; compat="0") dep
@test !is_host_dependency(dep)
@test is_target_dependency(dep)
@test is_build_dependency(dep)
@test is_runtime_dependency(dep)
@test !is_top_level_dependency(dep)
@test getname(dep) == name
@test getname(PackageSpec(; name = name)) == name
@test getpkg(dep) == PackageSpec(; name = name)
@test getcompat(dep) == ""
@test getpkg(dep) == PackageSpec(; name = name, version = PKG_VERSIONS.VersionSpec(v"0"))
@test getcompat(dep) == "0"

build_version = v"1.2.3"
dep_buildver = Dependency(PackageSpec(; name = name), build_version)
@test Dependency(name, build_version) == dep_buildver
dep_buildver = Dependency(PackageSpec(; name = name), build_version; compat = "~1.2")
@test Dependency(name, build_version; compat="~1.2") == dep_buildver
@test getname(dep_buildver) == name
@test getpkg(dep_buildver) == PackageSpec(; name = name, version = PKG_VERSIONS.VersionSpec(build_version))
@test getcompat(dep_buildver) == ""
@test getcompat(dep_buildver) == "~1.2"

# the same but with compat info
# the same but with platforms argument
dep_buildver = Dependency(PackageSpec(; name = name), build_version; compat = "~1.2", platforms=[Platform("x86_64", "linux"; cxxstring_abi="cxx11")])
@test Dependency(name, build_version) dep_buildver
@test Dependency(name, build_version; compat="~1.2") dep_buildver
@test getname(dep_buildver) == name
@test getpkg(dep_buildver) == PackageSpec(; name = name, version = PKG_VERSIONS.VersionSpec(build_version))
@test getcompat(dep_buildver) == "~1.2"

# the same but only with compat specifier
dep_compat = Dependency(PackageSpec(; name); compat = "2, ~$(build_version)")
@test Dependency(name, build_version) dep_compat
@test Dependency(name, build_version; compat="2, ~$(build_version)") dep_compat
@test getname(dep_compat) == name
@test getpkg(dep_compat) == PackageSpec(; name, version = PKG_VERSIONS.VersionSpec(build_version))
@test getcompat(dep_compat) == "2, ~$(build_version)"

# if build_version and compat don't match, an error should be thrown
@test_throws ArgumentError Dependency(PackageSpec(; name = name), build_version; compat = "2.0")

# Runtime dependencies without compat bounds should throw an error
@test_throws ArgumentError Dependency(name)
@test_throws ArgumentError RuntimeDependency(name)

run_dep = RuntimeDependency(PackageSpec(; name); compat="3.14")
@test RuntimeDependency(name) run_dep
@test RuntimeDependency(name; compat="3.14") run_dep
@test !is_host_dependency(run_dep)
@test is_target_dependency(run_dep)
@test !is_build_dependency(run_dep)
Expand All @@ -72,8 +76,8 @@ end
# We should be able to convert a `Vector{RuntimeDependency}` to `Vector{Dependency}`
@test Dependency[RuntimeDependency(name; compat="~1.8", platforms=[Platform("aarch64", "macos"; cxxstring_abi="cxx03")])] ==
[Dependency(name; compat="~1.8", platforms=[Platform("aarch64", "macos"; cxxstring_abi="cxx03")])]
@test @test_logs((:warn, r"was defined as top-level"), Dependency[RuntimeDependency(name; top_level=true)]) ==
[@test_logs((:warn, r"was defined as top-level"), Dependency(name; top_level=true))]
@test @test_logs((:warn, r"was defined as top-level"), Dependency[RuntimeDependency(name; compat="1", top_level=true)]) ==
[@test_logs((:warn, r"was defined as top-level"), Dependency(name; compat="1", top_level=true))]
# If the version in the PackageSpec and the compat don't match, an error should be thrown
@test_throws ArgumentError RuntimeDependency(PackageSpec(; name, version=v"1.2.3"); compat = "2.0")

Expand Down Expand Up @@ -102,12 +106,12 @@ end
@test getpkg(host_dep) == PackageSpec(; name = host_name)

top_level_name = "MPIPreferences"
@test_logs (:warn, r"deprecated") @test_throws ArgumentError Dependency(PackageSpec(; name=top_level_name); platforms=supported_platforms(; exclude=!Sys.isapple), top_level=true)
@test_throws ArgumentError RuntimeDependency(PackageSpec(; name=top_level_name); platforms=supported_platforms(; exclude=!Sys.isapple), top_level=true)
@test_logs (:warn, r"deprecated") @test_throws ArgumentError Dependency(PackageSpec(; name=top_level_name); compat="0", platforms=supported_platforms(; exclude=!Sys.isapple), top_level=true)
@test_throws ArgumentError RuntimeDependency(PackageSpec(; name=top_level_name); compat="1", platforms=supported_platforms(; exclude=!Sys.isapple), top_level=true)

top_level_dep = @test_logs (:warn, r"deprecated") Dependency(PackageSpec(; name = top_level_name); top_level=true)
top_level_dep = @test_logs (:warn, r"deprecated") Dependency(PackageSpec(; name = top_level_name); compat="1", top_level=true)
@test is_top_level_dependency(top_level_dep)
top_level_dep = RuntimeDependency(PackageSpec(; name = top_level_name); top_level=true)
top_level_dep = RuntimeDependency(PackageSpec(; name = top_level_name); compat="1", top_level=true)
@test is_top_level_dependency(top_level_dep)

@testset "Filter dependencies by platform" begin
Expand All @@ -117,7 +121,7 @@ end

@testset "JSON (de)serialization" begin
jdep = JSON.lower(dep)
@test jdep == Dict("type" => "dependency", "name" => name, "uuid" => nothing, "compat" => "", "version-major" => 0x0, "version-minor" => 0x0, "version-patch" => 0x0, "platforms" => ["x86_64-apple-darwin", "aarch64-apple-darwin"], "top_level" => false)
@test jdep == Dict("type" => "dependency", "name" => name, "uuid" => nothing, "compat" => "0", "version-major" => 0x0, "version-minor" => 0x0, "version-patch" => 0x0, "platforms" => ["x86_64-apple-darwin", "aarch64-apple-darwin"], "top_level" => false)
@test dependencify(jdep) == dep

jrun_dep = JSON.lower(run_dep)
Expand All @@ -137,14 +141,14 @@ end
@test jhost_dep == Dict("type" => "hostdependency", "name" => host_name, "uuid" => nothing, "compat" => "", "version-major" => 0x0, "version-minor" => 0x0, "version-patch" => 0x0, "platforms" => ["any"], "top_level" => false)
@test dependencify(jhost_dep) == host_dep

full_dep = Dependency(PackageSpec(; name = "Baz_jll", uuid = "00000000-1111-2222-3333-444444444444", version = PKG_VERSIONS.VersionSpec("3.1.4")))
full_dep = Dependency(PackageSpec(; name = "Baz_jll", uuid = "00000000-1111-2222-3333-444444444444", version = PKG_VERSIONS.VersionSpec("3.1.4")); compat="3")
jfull_dep = JSON.lower(full_dep)
@test jfull_dep == Dict("type" => "dependency", "name" => "Baz_jll", "uuid" => "00000000-1111-2222-3333-444444444444", "compat" => "", "version-major" => 0x3, "version-minor" => 0x1, "version-patch" => 0x4, "platforms" => ["any"], "top_level" => false)
@test jfull_dep == Dict("type" => "dependency", "name" => "Baz_jll", "uuid" => "00000000-1111-2222-3333-444444444444", "compat" => "3", "version-major" => 0x3, "version-minor" => 0x1, "version-patch" => 0x4, "platforms" => ["any"], "top_level" => false)
@test dependencify(jfull_dep) == full_dep
@test_throws ErrorException dependencify(Dict("type" => "git"))

jtop_level_dep = JSON.lower(top_level_dep)
@test jtop_level_dep == Dict("type" => "runtimedependency", "name" => "MPIPreferences", "uuid" => nothing, "compat" => "", "version-major" => 0x0, "version-minor" => 0x0, "version-patch" => 0x0, "platforms" => ["any"], "top_level" => true)
@test jtop_level_dep == Dict("type" => "runtimedependency", "name" => "MPIPreferences", "uuid" => nothing, "compat" => "1", "version-major" => 0x0, "version-minor" => 0x0, "version-patch" => 0x0, "platforms" => ["any"], "top_level" => true)
@test dependencify(jtop_level_dep) == top_level_dep
end

Expand All @@ -164,7 +168,7 @@ end
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
Dependency("Zlib_jll")
Dependency("Zlib_jll"; compat="1")
]
platform = HostPlatform()
ap = @test_logs setup_dependencies(prefix, getpkg.(dependencies), platform)
Expand Down Expand Up @@ -199,7 +203,7 @@ end
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
Dependency("LibCURL_jll")
Dependency("LibCURL_jll"; compat="7, 8")
]
platform = HostPlatform()
ap = @test_logs setup_dependencies(prefix, getpkg.(dependencies), platform)
Expand All @@ -222,7 +226,7 @@ end
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
Dependency("LibOSXUnwind_jll")
Dependency("LibOSXUnwind_jll"; compat="0.0.7")
]
platform = Platform("i686", "linux"; libc="musl")
@test_logs (:warn, r"Dependency LibOSXUnwind_jll does not have a mapping for artifact LibOSXUnwind for platform i686-linux-musl") begin
Expand All @@ -234,7 +238,7 @@ end
# Test setup of dependencies that depend on the Julia version
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [Dependency("GMP_jll")]
dependencies = [Dependency("GMP_jll"; compat="6.1.2")]
platform = Platform("x86_64", "linux"; julia_version=v"1.5")

# Test that a particular version of GMP is installed
Expand All @@ -245,7 +249,7 @@ end
# Next, test on Julia v1.6
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [Dependency("GMP_jll")]
dependencies = [Dependency("GMP_jll"; compat="6.2.1")]
platform = Platform("x86_64", "linux"; julia_version=v"1.6")

# Test that a particular version of GMP is installed
Expand All @@ -257,8 +261,8 @@ end
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
Dependency("GMP_jll", v"6.1.2"),
Dependency("MPFR_jll",v"4.1.0"),
Dependency("GMP_jll"; compat="6.1.2"),
Dependency("MPFR_jll"; compat="4.1.0"),
]

# Test that this is not instantiatable with either Julia v1.5 or v1.6
Expand Down Expand Up @@ -348,30 +352,16 @@ end
end

@testset "resolve_jlls" begin
# Deps given by name::String
dependencies = ["OpenSSL_jll",]
@test_logs (:warn, r"use Dependency instead") begin
truefalse, resolved_deps = resolve_jlls(dependencies)
@test truefalse
@test all(x->getpkg(x).uuid !== nothing, resolved_deps)
end
# Deps given by name::PackageSpec
@test_logs (:warn, r"use Dependency instead") begin
dependencies = [PackageSpec(name="OpenSSL_jll"),]
truefalse, resolved_deps = resolve_jlls(dependencies)
@test truefalse
@test all(x->getpkg(x).uuid !== nothing, resolved_deps)
end
# Deps given by (name,uuid)::PackageSpec
dependencies = [Dependency(PackageSpec(name="OpenSSL_jll", uuid="458c3c95-2e84-50aa-8efc-19380b2a3a95")),]
dependencies = [Dependency(PackageSpec(name="OpenSSL_jll", uuid="458c3c95-2e84-50aa-8efc-19380b2a3a95"); compat="3.0.3"),]
truefalse, resolved_deps = resolve_jlls(dependencies)
@test truefalse
@test all(x->getpkg(x).uuid !== nothing, resolved_deps)
# Deps given by combination of name::String, name::PackageSpec and (name,uuid)::PackageSpec
dependencies = [
Dependency("Zlib_jll"),
Dependency(PackageSpec(name="Bzip2_jll")),
Dependency(PackageSpec(name="OpenSSL_jll", uuid="458c3c95-2e84-50aa-8efc-19380b2a3a95")),
Dependency("Zlib_jll"; compat="1.2.13"),
Dependency(PackageSpec(name="Bzip2_jll"); compat="1.0.8"),
Dependency(PackageSpec(name="OpenSSL_jll", uuid="458c3c95-2e84-50aa-8efc-19380b2a3a95"); compat="3.0.3"),
]
truefalse, resolved_deps = resolve_jlls(dependencies)
@test truefalse
Expand Down

0 comments on commit d9774e4

Please sign in to comment.