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

respect versions in sysimage #3002

Merged
merged 3 commits into from
Feb 26, 2022
Merged
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
Pkg v1.9 Release Notes
=======================

- Pkg will now respect the version of packages put into the sysimage using e.g. PackageCompiler. For example,
if version 1.3.2 of package A is in the sysimage, Pkg will always install that version when adding the package,
or when the packge is installed as a dependency to some other package. This can be disabled by calling `Pkg.respect_sysimage_versions(false)`.

Pkg v1.8 Release Notes
======================

Expand Down
1 change: 1 addition & 0 deletions docs/src/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Pkg.status
Pkg.compat
Pkg.precompile
Pkg.offline
Pkg.respect_sysimage_versions
Pkg.setprotocol!
Pkg.dependencies
Pkg.project
Expand Down
26 changes: 25 additions & 1 deletion src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import ..Artifacts: ensure_artifact_installed, artifact_names, extract_all_hashe
artifact_exists, select_downloadable_artifacts
using Base.BinaryPlatforms
import ...Pkg
import ...Pkg: pkg_server, Registry, pathrepr, can_fancyprint, printpkgstyle, stderr_f, OFFLINE_MODE, UPDATED_REGISTRY_THIS_SESSION
import ...Pkg: pkg_server, Registry, pathrepr, can_fancyprint, printpkgstyle, stderr_f, OFFLINE_MODE, UPDATED_REGISTRY_THIS_SESSION, RESPECT_SYSIMAGE_VERSIONS

#########
# Utils #
Expand Down Expand Up @@ -389,6 +389,7 @@ end
get_or_make!(d::Dict{K,V}, k::K) where {K,V} = get!(d, k) do; V() end

const JULIA_UUID = UUID("1222c4b2-2114-5bfd-aeef-88e4692bbb3e")
const PKGORIGIN_HAVE_VERSION = :version in fieldnames(Base.PkgOrigin)
function deps_graph(env::EnvCache, registries::Vector{Registry.RegistryInstance}, uuid_to_name::Dict{UUID,String},
reqs::Resolve.Requires, fixed::Dict{UUID,Resolve.Fixed}, julia_version)
uuids = Set{UUID}()
Expand Down Expand Up @@ -456,6 +457,19 @@ function deps_graph(env::EnvCache, registries::Vector{Registry.RegistryInstance}
is_package_downloaded(env.project_file, pkg_spec) || continue
end

# Skip package version that are not the same as external packages in sysimage
if PKGORIGIN_HAVE_VERSION && RESPECT_SYSIMAGE_VERSIONS[] && julia_version == VERSION
pkgid = Base.PkgId(uuid, pkg.name)
if Base.in_sysimage(pkgid)
pkgorigin = get(Base.pkgorigins, pkgid, nothing)
if pkgorigin !== nothing && pkgorigin.version !== nothing
if v != pkgorigin.version
continue
end
end
end
end

all_compat_u[v] = compat_info
union!(uuids, keys(compat_info))
end
Expand Down Expand Up @@ -1857,6 +1871,14 @@ function status_compat_info(pkg::PackageSpec, env::EnvCache, regs::Vector{Regist
max_version == v"0" && return nothing
pkg.version >= max_version && return nothing

pkgid = Base.PkgId(pkg.uuid, pkg.name)
if PKGORIGIN_HAVE_VERSION && RESPECT_SYSIMAGE_VERSIONS[] && Base.in_sysimage(pkgid)
pkgorigin = get(Base.pkgorigins, pkgid, nothing)
if pkgorigin !== nothing && pkg.version !== nothing && pkg.version == pkgorigin.version
return ["sysimage"], max_version, max_version_in_compat
end
end

# Check compat of project
if pkg.version == max_version_in_compat && max_version_in_compat != max_version
return ["compat"], max_version, max_version_in_compat
Expand Down Expand Up @@ -2055,6 +2077,8 @@ function print_status(env::EnvCache, old_env::Union{Nothing,EnvCache}, registrie
printstyled(io, " (<v", max_version, ")"; color=Base.warn_color())
if packages_holding_back == ["compat"]
printstyled(io, " [compat]"; color=:light_magenta)
elseif packages_holding_back == ["sysimage"]
printstyled(io, " [sysimage]"; color=:light_magenta)
else
pkg_str = isempty(packages_holding_back) ? "" : string(": ", join(packages_holding_back, ", "))
printstyled(io, pkg_str; color=Base.warn_color())
Expand Down
14 changes: 14 additions & 0 deletions src/Pkg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ devdir(depot = depots1()) = get(ENV, "JULIA_PKG_DEVDIR", joinpath(depot, "dev"))
envdir(depot = depots1()) = joinpath(depot, "environments")
const UPDATED_REGISTRY_THIS_SESSION = Ref(false)
const OFFLINE_MODE = Ref(false)
const RESPECT_SYSIMAGE_VERSIONS = Ref(true)
# For globally overriding in e.g. tests
const DEFAULT_IO = Ref{Union{IO,Nothing}}(nothing)
stderr_f() = something(DEFAULT_IO[], stderr)
Expand Down Expand Up @@ -513,6 +514,19 @@ set the environment variable `JULIA_PKG_OFFLINE` to `"true"`.
"""
offline(b::Bool=true) = (OFFLINE_MODE[] = b; nothing)

"""
Pkg.respect_sysimage_versions(b::Bool=true)

Enable (`b=true`) or disable (`b=false`) respecting versions that are in the
sysimage (enabled by default).

If this option is enabled, Pkg will only install packages that have been put into the sysimage
(e.g. via PackageCompiler) at the version of the package in the sysimage.
Also, trying to add a package at a URL or `develop` a package that is in the sysimage
will error.
"""
respect_sysimage_versions(b::Bool=true) = (RESPECT_SYSIMAGE_VERSIONS[] = b; nothing)

"""
PackageSpec(name::String, [uuid::UUID, version::VersionNumber])
PackageSpec(; name, url, path, subdir, rev, version, mode, level)
Expand Down
6 changes: 6 additions & 0 deletions src/Resolve/graphtype.jl
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,13 @@ function init_log!(data::GraphData)
else
vspec = range_compressed_versionspec(versions)
vers = logstr(id, vspec)
uuid = data.pkgs[p0]
name = data.uuid_to_name[uuid]
pkgid = Base.PkgId(uuid, name)
msg = "possible versions are: $vers or uninstalled"
if Base.in_sysimage(pkgid)
msg *= " (package in sysimage!)"
end
end
first_entry = get!(rlog.pool, p) do
ResolveLogEntry(rlog.journal, p, "$(logstr(id)) log:")
Expand Down
20 changes: 18 additions & 2 deletions src/Types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ using REPL.TerminalMenus

using TOML
import ..Pkg, ..Registry
import ..Pkg: GitTools, depots, depots1, logdir, set_readonly, safe_realpath, pkg_server, stdlib_dir, stdlib_path, isurl, stderr_f
import ..Pkg: GitTools, depots, depots1, logdir, set_readonly, safe_realpath, pkg_server, stdlib_dir, stdlib_path, isurl, stderr_f, RESPECT_SYSIMAGE_VERSIONS
import Base.BinaryPlatforms: Platform
using ..Pkg.Versions

Expand Down Expand Up @@ -526,6 +526,18 @@ function devpath(env::EnvCache, name::AbstractString, shared::Bool)
return joinpath(dev_dir, name)
end

function error_if_in_sysimage(pkg::PackageSpec)
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks BinaryBuilder

Copy link
Member

Choose a reason for hiding this comment

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

You can set Pkg.respect_sysimage_versions(false) as a workaround.

Although, it would be good to figure out why it breaks BinaryBuilder.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/JuliaPackaging/BinaryBuilderBase.jl/runs/5369016457?check_suite_focus=true

  Expression: setup_dependencies(prefix, getpkg.(dependencies), platform)
  tried to develop or add a package by URL that is already in the sysimage
  Stacktrace:
    [1] pkgerror(msg::String)
      @ Pkg.Types /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/Types.jl:67
    [2] error_if_in_sysimage(pkg::Pkg.Types.PackageSpec)
      @ Pkg.Types /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/Types.jl:537
    [3] (::Pkg.Types.var"#47#48"{Pkg.Types.Context, Pkg.Types.PackageSpec, String})(repo::GitRepo)
      @ Pkg.Types /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/Types.jl:752
    [4] with(f::Pkg.Types.var"#47#48"{Pkg.Types.Context, Pkg.Types.PackageSpec, String}, obj::GitRepo)
      @ LibGit2 /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/LibGit2/src/types.jl:1153
    [5] handle_repo_add!(ctx::Pkg.Types.Context, pkg::Pkg.Types.PackageSpec)
      @ Pkg.Types /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/Types.jl:709
    [6] handle_repos_add!(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec})
      @ Pkg.Types /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/Types.jl:779
    [7] add(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; preserve::Pkg.Types.PreserveLevel, platform::Platform, kwargs::Base.Pairs{Symbol, Union{Base.DevNull, Nothing}, Tuple{Symbol, Symbol}, NamedTuple{(:io, :julia_version), Tuple{Base.DevNull, Nothing}}})
      @ Pkg.API /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/API.jl:255
    [8] (::BinaryBuilderBase.var"#89#95"{Bool, Prefix, Vector{Pkg.Types.PackageSpec}, Platform, Vector{String}, Vector{String}})()
      @ BinaryBuilderBase ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/src/Prefix.jl:631
    [9] activate(f::BinaryBuilderBase.var"#89#95"{Bool, Prefix, Vector{Pkg.Types.PackageSpec}, Platform, Vector{String}, Vector{String}}, new_project::String)
      @ Pkg.API /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/API.jl:1700
   [10] setup_dependencies(prefix::Prefix, dependencies::Vector{Pkg.Types.PackageSpec}, platform::Platform; verbose::Bool)
      @ BinaryBuilderBase ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/src/Prefix.jl:582
   [11] setup_dependencies
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/src/Prefix.jl:556 [inlined]
   [12] (::var"#38#60"{Platform, Vector{Dependency}, Prefix})()
      @ Main ./none:0
   [13] with_logstate(f::Function, logstate::Any)
      @ Base.CoreLogging ./logging.jl:511
   [14] with_logger
      @ ./logging.jl:623 [inlined]
   [15] #collect_test_logs#65
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/logging.jl:114 [inlined]
   [16] collect_test_logs
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/logging.jl:113 [inlined]
   [17] #match_logs#66
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/logging.jl:260 [inlined]
   [18] match_logs
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/logging.jl:260 [inlined]
   [19] (::var"#37#59")(dir::String)
      @ Main ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:151
   [20] #30
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:14 [inlined]
   [21] activate(f::var"#30#32"{String, var"#37#59"}, new_project::String)
      @ Pkg.API /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Pkg/src/API.jl:1700
   [22] #29
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:13 [inlined]
   [23] mktempdir(fn::var"#29#31"{var"#37#59"}, parent::String; prefix::String)
      @ Base.Filesystem ./file.jl:760
   [24] mktempdir (repeats 2 times)
      @ ./file.jl:758 [inlined]
   [25] with_temp_project(f::var"#37#59")
      @ Main ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:12
   [26] macro expansion
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:145 [inlined]
   [27] macro expansion
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/Test.jl:1357 [inlined]
   [28] macro expansion
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:109 [inlined]
   [29] macro expansion
      @ /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/Test/src/Test.jl:1357 [inlined]
   [30] top-level scope
      @ ~/work/BinaryBuilderBase.jl/BinaryBuilderBase.jl/test/dependencies.jl:20

We install JLL stdlibs with different version numbers

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, yeah that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tests still fail even with JuliaPackaging/BinaryBuilderBase.jl#227, but in a different way, so there may be something else broken now. But I don't have the time to debug now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, I believe that the remaining failure is unrelated to any changes in Pkg. Pkg.respect_sysimage_versions(false) is sufficient to be able to install packages without problems.

RESPECT_SYSIMAGE_VERSIONS[] || return false
if pkg.uuid === nothing
@error "Expected package to have a set UUID, please file a bug report"
return false
end
pkgid = Base.PkgId(pkg.uuid, pkg.name)
if Base.in_sysimage(pkgid)
pkgerror("tried to develop or add a package by URL that is already in the sysimage")
end
end

function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool)
# First, check if we can compute the path easily (which requires a given local path or name)
is_local_path = pkg.repo.source !== nothing && !isurl(pkg.repo.source)
Expand All @@ -544,6 +556,7 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool)
end
if isdir(dev_path)
resolve_projectfile!(ctx.env, pkg, dev_path)
error_if_in_sysimage(pkg)
if is_local_path
pkg.path = isabspath(dev_path) ? dev_path : relative_project_path(ctx.env.project_file, dev_path)
else
Expand Down Expand Up @@ -605,6 +618,7 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool)
if !has_uuid(pkg)
resolve_projectfile!(ctx.env, pkg, dev_path)
end
error_if_in_sysimage(pkg)
pkg.path = shared ? dev_path : relative_project_path(ctx.env.project_file, dev_path)
if pkg.repo.subdir !== nothing
pkg.path = joinpath(pkg.path, pkg.repo.subdir)
Expand Down Expand Up @@ -735,13 +749,15 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec)

# If we already resolved a uuid, we can bail early if this package is already installed at the current tree_hash
if has_uuid(pkg)
error_if_in_sysimage(pkg)
version_path = Pkg.Operations.source_path(ctx.env.project_file, pkg, ctx.julia_version)
isdir(version_path) && return false
end

temp_path = mktempdir()
GitTools.checkout_tree_to_path(repo, tree_hash_object, temp_path)
package = resolve_projectfile!(ctx.env, pkg, temp_path)
resolve_projectfile!(ctx.env, pkg, temp_path)
error_if_in_sysimage(pkg)

# Now that we are fully resolved (name, UUID, tree_hash, repo.source, repo.rev), we can finally
# check to see if the package exists at its canonical path.
Expand Down
39 changes: 38 additions & 1 deletion test/new.jl
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ end
Pkg.add("Markdown")
Pkg.add("Markdown")
Pkg.dependencies(markdown_uuid) do pkg
pkg.name == "Markdown"
@test pkg.name == "Markdown"
end
@test haskey(Pkg.project().dependencies, "Markdown")
end
Expand Down Expand Up @@ -2893,4 +2893,41 @@ end
end
end

if :version in fieldnames(Base.PkgOrigin)
@testset "sysimage functionality" begin
old_sysimage_modules = copy(Base._sysimage_modules)
old_pkgorigins = copy(Base.pkgorigins)
try
# Fake having a packages in the sysimage.
json_pkgid = Base.PkgId(json_uuid, "JSON")
push!(Base._sysimage_modules, json_pkgid)
Base.pkgorigins[json_pkgid] = Base.PkgOrigin(nothing, nothing, v"0.20.1")
isolate(loaded_depot=true) do
Pkg.add("JSON"; io=devnull)
Pkg.dependencies(json_uuid) do pkg
pkg.version == v"0.20.1"
end
io = IOBuffer()
Pkg.status(; outdated=true, io=io)
str = String(take!(io))
@test occursin("⌅ [682c06a0] JSON v0.20.1", str)
@test occursin("[sysimage]", str)

@test_throws PkgError Pkg.add(name="JSON", rev="master"; io=devnull)
@test_throws PkgError Pkg.develop("JSON"; io=devnull)

Pkg.respect_sysimage_versions(false)
Pkg.add("JSON"; io=devnull)
Pkg.dependencies(json_uuid) do pkg
pkg.version != v"0.20.1"
end
end
finally
copy!(Base._sysimage_modules, old_sysimage_modules)
copy!(Base.pkgorigins, old_pkgorigins)
Pkg.respect_sysimage_versions(true)
end
end
end

end #module