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

use julia_version Pkg.add kwarg rather than specifying it through ctx #416

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions src/Dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ patch(v::VersionNumber) = v.patch
major(v::Pkg.Types.VersionBound) = v.t[1]
minor(v::Pkg.Types.VersionBound) = v.t[2]
patch(v::Pkg.Types.VersionBound) = v.t[3]

# If a version in a PackageSpec is given as a string it signifies being a VersionSpec that Pkg will handle as such.
__version(v::AbstractString) = __version(PKG_VERSIONS.VersionSpec(v))
__version(v::VersionNumber) = v
__version(v::PKG_VERSIONS.VersionSpec) = v.ranges[1].lower
version(d::AbstractDependency) = __version(getpkg(d).version)
Expand Down
32 changes: 18 additions & 14 deletions src/Prefix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -597,20 +597,25 @@ function get_addable_spec(name::AbstractString, version::VersionNumber;
uuid=uuid,
#version=version,
tree_hash=tree_hash_sha1,
repo=Pkg.Types.GitRepo(rev=git_commit_sha, source=valid_url),
rev=git_commit_sha,
url=valid_url,
Comment on lines -600 to +601
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying repo directly isn't compatible with the public Pkg.add API as Pkg.handle_package_input! doesn't expect it to exist, instead constructs it from rev and url etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved handling JuliaLang/Pkg.jl#4170

)
end

# Helper function to install packages also in Julia v1.8
function Pkg_add(args...; kwargs...)
@static if VERSION < v"1.8.0"
Pkg.add(args...; kwargs...)
else
try
Pkg.respect_sysimage_versions(false)
# we don't want to precompile packages during installation
# auto-precompilation also calls `Pkg.instantiate` which will warn about non-VERSION `julia_version` values
withenv("JULIA_PKG_PRECOMPILE_AUTO" => "false") do
@static if VERSION < v"1.8.0"
Pkg.add(args...; kwargs...)
finally
Pkg.respect_sysimage_versions(true)
else
try
Pkg.respect_sysimage_versions(false)
Pkg.add(args...; kwargs...)
finally
Pkg.respect_sysimage_versions(true)
end
end
end
end
Expand Down Expand Up @@ -669,18 +674,16 @@ function setup_dependencies(prefix::Prefix,
deps_project = joinpath(prefix, triplet(platform), ".project")
Pkg.activate(deps_project) do
# Update registry first, in case the jll packages we're looking for have just been registered/updated
ctx = Pkg.Types.Context(;julia_version)
outs = verbose ? stdout : devnull
update_registry(outs)

# Add all dependencies. Note: Pkg.add(ctx, deps) modifies in-place `deps` without
# notice. We need to `deepcopy` the argument to prevent it from modying our
# dependencies from under our feet: <https://github.com/JuliaLang/Pkg.jl/issues/3112>.
Pkg_add(ctx, deepcopy(dependencies); platform=platform, io=outs)
# Add all dependencies.
Pkg_add(dependencies; platform=platform, io=outs, julia_version=julia_version)

# Ony Julia v1.6, `Pkg.add()` doesn't mutate `dependencies`, so we can't use the `UUID`
# that was found during resolution there. Instead, we'll make use of `ctx.env` to figure
# out the UUIDs of all our packages.
ctx = Pkg.Types.Context()
dependency_uuids = Set([uuid for (uuid, pkg) in ctx.env.manifest if pkg.name ∈ dependencies_names])

# Some JLLs are also standard libraries that may be present in the manifest because
Expand Down Expand Up @@ -718,7 +721,8 @@ function setup_dependencies(prefix::Prefix,

# Re-install stdlib dependencies, but this time with `julia_version = nothing`
if !isempty(stdlib_pkgspecs)
Pkg_add(ctx, stdlib_pkgspecs; io=outs, julia_version=nothing)
# TODO: shouldn't this take platform?
Pkg_add(stdlib_pkgspecs; io=outs, julia_version=nothing)
end

# Load their Artifacts.toml files
Expand Down
68 changes: 37 additions & 31 deletions test/dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,8 @@ end
uuid="86de99a1-58d6-5da7-8064-bd56ce2e322c",
tree_hash=Base.SHA1("83481d62501cf2ef22bed745dbcedc4e75fa6e95"),
version=PKG_VERSIONS.VersionSpec("*"),
repo=Pkg.Types.GitRepo(
source="https://github.com/JuliaBinaryWrappers/LLVM_jll.jl.git",
rev="2772761b330d51146ace3125b26acdad0df4f30f",
),
url="https://github.com/JuliaBinaryWrappers/LLVM_jll.jl.git",
rev="2772761b330d51146ace3125b26acdad0df4f30f",
)

with_temp_project() do dir
Expand Down Expand Up @@ -370,39 +368,47 @@ end

@testset "PackageSpec with version" begin
# Install a dependency with a specific version number.
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
PackageSpec(; name="CMake_jll", version = v"3.24.3")
]
platform = Platform("x86_64", "linux"; libc="musl", cxxstring_abi="cxx11")
if v"1.9" <= VERSION < v"1.11"
# For reasons I can't understand, in CI on GitHub Actions (and only
# there, can't reproduce the same behaviour locally) the error thrown
# inside the `setup_dependencies` "escapes" the `try` block. For lack of
# time to debug this stupid thing we just mark this step as broken and
# move on, it's really broken anyway.
@test false broken=true
else
try
test_setup_dependencies(prefix, dependencies, platform)
catch
if VERSION>=v"1.9"
# This test is expected to be broken on Julia v1.9+
@test false broken=true
@testset for version in (v"3.24.3+0", "3.24.3")
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
PackageSpec(; name="CMake_jll", version = version)
]
platform = Platform("x86_64", "linux"; libc="musl", cxxstring_abi="cxx11")
test_setup_dependencies(prefix, dependencies, platform)
# The directory contains also executables from CMake dependencies.
@test readdir(joinpath(destdir(dir, platform), "bin")) == ["c_rehash", "cmake", "cpack", "ctest", "openssl"]
end
end
if VERSION >= v"1.9.0-0"
@testset "should error if build is missing from a specific VersionNumber, with `julia_version=nothing`" begin
with_temp_project() do dir
prefix = Prefix(dir)
dependencies = [
PackageSpec(; name="CMake_jll", version = v"3.24.3")
]
platform = Platform("x86_64", "linux"; libc="musl", cxxstring_abi="cxx11", julia_version=nothing)

# Pkg needs improve its error message here, but assume that it will still throw a pkgerror
# https://github.com/JuliaLang/Pkg.jl/issues/4159
# Before https://github.com/JuliaLang/Pkg.jl/pull/4151 this would throw a MethodError for `abspath(::Nothing)`
# So this test will need fixing if/when that gets backported
error_type = if VERSION >= v"1.13.0-0"
Pkg.Types.PkgError
elseif VERSION >= v"1.10.0-0"
MethodError
else
# For previous versions we don't expect errors and we
# want to see them.
rethrow()
KeyError
end
@test_throws error_type setup_dependencies(prefix, dependencies, platform)
end
end
# The directory contains also executables from CMake dependencies.
# Test will fail if `setup_dependencies` above failed.
@test readdir(joinpath(destdir(dir, platform), "bin")) == ["c_rehash", "cmake", "cpack", "ctest", "openssl"] broken=VERSION>=v"1.9"
else
# The above test doesn't throw before v1.9. Unclear why. Pkg misinterpreting a specific (incorrect)
# VersionNumber spec as a VersionSpec?
@test_broken false
end
end

end
end

Expand Down