Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dependencies] Require compat argument for runtime dependencies #314

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.2.12")
]
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.73, 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