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

respect versions in sysimage #3002

merged 3 commits into from
Feb 26, 2022

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Feb 23, 2022

I created a sysimage with an outdated Plots versions, loaded Julia with that sysimage and created a temp repo and added Plots:

(Pkg) pkg> activate --temp
  Activating new project at `/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5`

(jl_hr2NH5) pkg> add Plots
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `/private/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5/Project.toml`
⌃ [91a5bcdd] + Plots v1.24.3
    Updating `/private/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5/Manifest.toml`
  [79e6a3ab] + Adapt v3.3.3
  [d360d2e6] + ChainRulesCore v1.12.2

(jl_hr2NH5) pkg> st --outdated -m
Status `/private/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5/Manifest.toml`
⌅ [28b8d3ca] GR v0.62.1 (<v0.64.0): Plots
⌅ [77ba4419] NaNMath v0.3.7 (<v1.0.0): Plots, RecipesPipeline
⌃ [91a5bcdd] Plots v1.24.3 (<v1.25.11)
⌅ [01d81517] RecipesPipeline v0.4.1 (<v0.5.1): Plots

(jl_hr2NH5) pkg> up
  No Changes to `/private/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5/Project.toml`
  No Changes to `/private/var/folders/26/gw1dqz6n5zsbkp72mktkhz3c0000gn/T/jl_hr2NH5/Manifest.toml`

Requires JuliaLang/julia#44318

@KristofferC KristofferC changed the title wip: respect versions in sysimage respect versions in sysimage Feb 24, 2022
@KristofferC
Copy link
Member Author

outdated support:

image

@DilumAluthge
Copy link
Member

What will the error message look like if the user explicitly does ] add [email protected]?

@KristofferC
Copy link
Member Author

(jl_3MCoqV) pkg> add [email protected]
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
ERROR: Unsatisfiable requirements detected for package Plots [91a5bcdd]:
 Plots [91a5bcdd] log:
 ├─possible versions are: 1.24.3 or uninstalled (package in sysimage!)
 └─restricted to versions 1.25.11 by an explicit requirement — no versions left

@KristofferC
Copy link
Member Author

Need to wait for a Julia nightly that includes JuliaLang/julia#44318 now.

@staticfloat
Copy link
Member

How would this compose with #2916? Is it the case that any package that is in the system image is an stdlib? If so, could we get this same information by storing the current list of stdlibs (as in that PR) and using that information during resolution?

In any case, I like the UI improvements here.

@KristofferC
Copy link
Member Author

KristofferC commented Feb 26, 2022

How would this compose with #2916? Is it the case that any package that is in the system image is an stdlib?

Not really, the concept of an stdlib and a package in the sysimage is not the same. An stdlib is a package in the stdlib folder, a package in the sysimage is a package that is in the sysimage. They do not need to have any overlap, for example an stdlib does not need to be in the sysimage (like the _jll stdlibs) and if you use PackageCompiler, a package in the sysimage does not need to be an stdlib.

Also a package in the sysimage (that is not an stdlib) still gets its dependency and compat information from the registry.

@KristofferC
Copy link
Member Author

Looking at the test failure, I'll disable the sysimage check when julia_version != VERSION because at that point you don't really care about the current Julia session.

@DilumAluthge DilumAluthge reopened this Feb 26, 2022
@KristofferC KristofferC merged commit 31af274 into master Feb 26, 2022
@KristofferC KristofferC deleted the kc/sysimage branch February 26, 2022 20:41
@@ -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.

@ericphanson
Copy link
Contributor

any chance this is eligible for backport to 1.8?

@ViralBShah
Copy link
Member

Maybe this can go into 1.8.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants