JLLPrefixes.jl icon indicating copy to clipboard operation
JLLPrefixes.jl copied to clipboard

Future Pkg compatibility

Open staticfloat opened this issue 2 years ago • 17 comments

Let's discuss here the future of Pkg's hooks that we can use in this package to grab JLLs belonging to a particular Julia version.

staticfloat avatar Feb 07 '23 04:02 staticfloat

First of all, I don't care in the slightlest about what the syntax will look like. Despite whatever people may think, I'm not married to the whole context business at all, I'm not even the one who suggested nor implemented it in the first place, it's just the only way we have to be able to deal with the resolver using different julia versions, and I believe this is 99% of the complications here.

The only thing I'd like to have is a stable syntax which allows us to install artifacts which doesn't break every other month, ideally with regression tests in Pkg.jl itself.

Requirements:

  • the ultimate goal in all cases is to be able to access the Artifacts.toml file on the given JLL package
    • currently, this is achieved with
      joinpath(Pkg.Operations.find_installed(dep.name, dep.uuid, dep.tree_hash), "Artifacts.toml")
      
      https://github.com/JuliaPackaging/JLLPrefixes.jl/blob/1758c51152ddc56108202fa3e386d48a535966a1/src/JLLPrefixes.jl#L120-L129 If there are better ways to do that, I don't care, but we need a reliable way to always access the Artifacts.toml file
  • for non-stdlib registered JLLs, I really don't think there's any ambiguity whatsoever: for registered packages, asking to install a package with given name, uuid, and optionally version number, should just install that one using the standard resolver (whatever Pkg.add(; name, version, uuid) would do), as simple as that. If a version number is specified, we want that version number, nothing else
  • we want also to be able to install unregistered packages/packages from URL/local path. Also in this case, there shouldn't really be any ambiguity (whatever Pkg.add(; url/path, [rev]) would do)
  • for stdlibs JLLs, we're often happy to install whatever is the version that Pkg.add("Name_jll") would get us, but if a version number is specified, we want that version number. Note: given the very first point, we must be able to access the Artifacts.toml file also for stdlib JLLs
  • in general, we want to be able to install an exact PackageSpec. If it specifies a version number of a registered package down to the build number, we want that (currently we can get a version number different from what we asked: https://github.com/JuliaLang/Pkg.jl/issues/3113)
  • build numbers: in most cases we'd like to get the latest version within the given patch number. For example, if we ask for Package_jll v0.5.3, we want to get v0.5.3+2, if that's the latest build of the v0.5.3 series. But if we ask for v0.5.3+1, we want that (basically same point as above). I'd be ok with having two modes to resolve, one stricter (exact matching) and one laxer (latest matching), or whatever (different ways to construct the PackageSpec perhaps?), as long as we can deal with it internally in a reliable way
  • julia version number: sometimes we need to ask Pkg to resolve packages as if within a different Julia version than the current one. At the moment, to do that we're forced to use a context, but that's considered internal, and the source of basically all problems in BinaryBuilder using Pkg

CC @IanButterworth @KristofferC

giordano avatar Mar 20 '23 14:03 giordano

It was mentioned somewhere but I can't find the comment, but I wonder whether the key issue is that the julia_version kwarg doesn't work as desired. That way all the regular API entry points could be used i.e.Pkg.add(.... julia_version=v"1.6") and forget about Context()

IanButterworth avatar Mar 20 '23 15:03 IanButterworth

No

giordano avatar Mar 20 '23 15:03 giordano

That comment is unhelpfully terse. Are you saying that

the key issue is that the julia_version kwarg doesn't work as desired

isn't the issue, or just pointing to that it doesn't currently work?

IanButterworth avatar Mar 20 '23 15:03 IanButterworth

More specifically was referring to

That way all the regular API entry points could be used i.e.Pkg.add(.... julia_version=v"1.6") and forget about Context()

That wouldn't work, as per the issue linked (which is also why I put as very first requirement being able to access the Artifacts.toml with joinpath(Pkg.Operations.find_installed(dep.name, dep.uuid, dep.tree_hash), "Artifacts.toml")).

giordano avatar Mar 20 '23 15:03 giordano

Perhaps you misread my comment? I believe we agree

IanButterworth avatar Mar 20 '23 15:03 IanButterworth

Ok, yes, Pkg.add(...; julia_version) doesn't work as desired, otherwise we'd be using it no problem. Every time I tried to remove the context argument in BBB, some tests started failing badly (can't remember the details now because I stopped doing that exercise). Using the context argument just barely holds things together up, but again, I'd be more than happy getting rid of it if we only could.

giordano avatar Mar 20 '23 16:03 giordano

Great. So let's:

  • expand the testing of the Pkg API with the julia_version kwarg (no Context), marking broken tests
  • Identify where the API is lacking in serving the information BB needs. i.e. Pkg.Operations.find_installed is an internal

IanButterworth avatar Mar 20 '23 16:03 IanButterworth

Okay, here's Elliot and Mosè's mini Pkg test suite:

Ways of specifying input

We prefer to pass PackageSpec objects so that we can specify multiple packages at once, instead of running Pkg.add() again and again, and so that they are jointly resolved. In all of these examples, consider them as shorthand for creating PackageSpec's if not otherwise specified.

At the end of the add(), we always want to load the Artifacts.toml installed by the JLL. Here's some example code for that: https://github.com/JuliaPackaging/JLLPrefixes.jl/blob/1758c51152ddc56108202fa3e386d48a535966a1/src/JLLPrefixes.jl#L120-L129

Test suite

For each of these commands, we expect to be able to load the Artifacts.toml for that exact version. We've marked a few of these functions known to fail (and congrats, many that we expected to fail are actually passing on Julia v1.9!)

# Standard add (non-stdlib, flexible version)
Pkg.add(; name="HelloWorldC_jll")

# Standard add (non-stdlib, url and rev)
Pkg.add(; name="HelloWorldC_jll", url="https://github.com/JuliaBinaryWrappers/HelloWorldC_jll.jl", rev="0b4959a49385d4bb00efd281447dc19348ebac08")

# Standard add (non-stdlib, specified version)
Pkg.add(; name="HelloWorldC_jll", version=v"1.0.10+1")

# Standard add (non-stdlib, versionspec)
Pkg.add(; name="HelloWorldC_jll", version=Pkg.Types.VersionSpec("1.0.10"))

# Julia-version-dependent add (non-stdlib, flexible version)
Pkg.add(; name="libcxxwrap_julia_jll", julia_version=v"1.7")

# Julia-version-dependent add (non-stdlib, specified version)
Pkg.add(; name="libcxxwrap_julia_jll", version=v"0.9.4+0", julia_version=v"1.7")
@test_fails Pkg.add(; name="libcxxwrap_julia_jll", version=v"0.8.8+1", julia_version=v"1.9")

# Stdlib add (current julia version)
Pkg.add(; name="GMP_jll")

# Stdlib add (other julia version)
Pkg.add(; name="GMP_jll", julia_version=v"1.7")

# Stdlib add (other julia version, with specific version bound)
# Note, this doesn't work properly, it adds but doesn't install any artifacts.
# Technically speaking, this is probably okay from Pkg's perspective, since
# we're asking Pkg to resolve according to what Julia v1.7 would do.... and
# Julia v1.7 would not install anything because it's a stdlib!  However, we
# would sometimes like to resolve the latest version of GMP_jll for Julia v1.7
# then install that.  If we have to manually work around that and look up what
# GMP_jll for Julia v1.7 is, then ask for that version explicitly, that's ok.
@test_fails Pkg.add(; name="GMP_jll", julia_version=v"1.7")

# This is expected to fail, that version can't live with `julia_version = v"1.7"`
@test_fails Pkg.add(; name="GMP_jll", version=v"6.2.0+5", julia_version=v"1.7")

# Stdlib add (julia_version == nothing)
# Note: this is currently known to be broken, we get the wrong GMP_jll!
Pkg.add(; name="GMP_jll", version=v"6.2.1+1", julia_version=nothing)

# Stdlib add (impossible constraints due to julia version compat, so
# must pass `julia_version=nothing`). In this case, we always fully
# specify versions, but if we don't, it's okay to just give us whatever
# the resolver prefers
Pkg.add([
    PkgSpec(;name="OpenBLAS_jll",  version=v"0.3.13"),
    PkgSpec(;name="libblastrampoline_jll", version=v"5.1.1"),
]; julia_version=nothing)

staticfloat avatar Mar 21 '23 20:03 staticfloat

Just hit BB won't build in Julia 1.8+, seems pretty bad to me a huge part of Julia infra is stuck with <= 1.7?

Moelf avatar Nov 29 '23 22:11 Moelf

Tagging myself here.

mkitti avatar Feb 29 '24 00:02 mkitti

Should an initial goal be to try to get to work on Julia 1.10 before Julia 1.11?

mkitti avatar Mar 02 '24 20:03 mkitti

We might need to solve https://github.com/JuliaPackaging/HistoricalStdlibVersions.jl/issues/9 as a prerequisite to fixing this.

   Resolving package versions...
ERROR: LoadError: If you want to set `julia_version`, you must first populate the `STDLIBS_BY_VERSION` global constant
Stacktrace:
  [1] pkgerror(msg::String)
    @ Pkg.Types ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Types.jl:70
  [2] get_last_stdlibs(julia_version::VersionNumber; use_historical_for_current_version::Bool)
    @ Pkg.Types ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Types.jl:477
  [3] get_last_stdlibs
    @ ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Types.jl:471 [inlined]
  [4] deps_graph(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, uuid_to_name::Dict{Base.UUID, String}, reqs::Dict{Base.UUID, Pkg.Versions.VersionSpec}, fixed::Dict{Base.UUID, Pkg.Resolve.Fixed}, julia_version::VersionNumber, installed_only::Bool)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:472
  [5] resolve_versions!(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, julia_version::VersionNumber, installed_only::Bool)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:406
  [6] targeted_resolve(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, preserve::Pkg.Types.PreserveLevel, julia_version::VersionNumber)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1360
  [7] tiered_resolve(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, julia_version::VersionNumber, try_all_installed::Bool)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1332
  [8] _resolve(io::Base.TTY, env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, preserve::Pkg.Types.PreserveLevel, julia_version::VersionNumber)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1370
  [9] add(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}, new_git::Set{Base.UUID}; preserve::Pkg.Types.PreserveLevel, platform::Base.BinaryPlatforms.Platform)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1387
 [10] add
    @ ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1376 [inlined]
 [11] add(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; preserve::Pkg.Types.PreserveLevel, platform::Base.BinaryPlatforms.Platform, kwargs::@Kwargs{julia_version::VersionNumber, io::Base.TTY})
    @ Pkg.API ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:278
 [12] add(pkgs::Vector{Pkg.Types.PackageSpec}; io::Base.TTY, kwargs::@Kwargs{julia_version::VersionNumber})
    @ Pkg.API ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:159
 [13] add
    @ ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:146 [inlined]
 [14] add(; name::String, uuid::Nothing, version::Nothing, url::Nothing, rev::Nothing, path::Nothing, mode::Pkg.Types.PackageMode, subdir::Nothing, kwargs::@Kwargs{julia_version::VersionNumber})
    @ Pkg.API ~/.julia/juliaup/julia-1.10.2+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:176
 [15] top-level scope
    @ ~/.julia/dev/bb/test_suite.jl:43

mkitti avatar Mar 02 '24 20:03 mkitti

Here's a version of the test suite that actually uses tests. I still need to check the artifacts.

using Pkg
using Test
Pkg.activate(; temp = true)
Pkg.add(; name="HistoricalStdlibVersions")

using HistoricalStdlibVersions
append!(Pkg.Types.STDLIBS_BY_VERSION, HistoricalStdlibVersions.STDLIBS_BY_VERSION)

const HelloWorldC_jll_UUID = Base.UUID("dca1746e-5efc-54fc-8249-22745bc95a49")
const GMP_jll_UUID = Base.UUID("781609d7-10c4-51f6-84f2-b8444358ff6d")
const OpenBLAS_jll_UUID = Base.UUID("4536629a-c528-5b80-bd46-f80d51c5b363")
const libcxxwrap_julia_jll_UUID = Base.UUID("3eaa8342-bff7-56a5-9981-c04077f7cee7")
const libblastrampoline_jll_UUID = Base.UUID("8e850b90-86db-534c-a0d3-1478176c7d93")

@testset "Elliot and Mosè's mini Pkg test suite" begin

    @testset "HelloWorldC_jll" begin
        # Standard add (non-stdlib, flexible version)
        Pkg.add(; name="HelloWorldC_jll")
        @test haskey(Pkg.dependencies(), HelloWorldC_jll_UUID)

        # Standard add (non-stdlib, url and rev)
        Pkg.add(; name="HelloWorldC_jll", url="https://github.com/JuliaBinaryWrappers/HelloWorldC_jll.jl", rev="0b4959a49385d4bb00efd281447dc19348ebac08")
        @test Pkg.dependencies()[Base.UUID("dca1746e-5efc-54fc-8249-22745bc95a49")].git_revision === "0b4959a49385d4bb00efd281447dc19348ebac08"

        # Standard add (non-stdlib, specified version)
        Pkg.add(; name="HelloWorldC_jll", version=v"1.0.10+1")
        @test Pkg.dependencies()[Base.UUID("dca1746e-5efc-54fc-8249-22745bc95a49")].version === v"1.0.10+1"

        # Standard add (non-stdlib, versionspec)
        Pkg.add(; name="HelloWorldC_jll", version=Pkg.Types.VersionSpec("1.0.10"))
        @test Pkg.dependencies()[Base.UUID("dca1746e-5efc-54fc-8249-22745bc95a49")].version === v"1.0.10+1"
    end

    @testset "libcxxwrap_julia_jll" begin

        # Julia-version-dependent add (non-stdlib, flexible version)
        Pkg.add(; name="libcxxwrap_julia_jll", julia_version=v"1.7")
        @test Pkg.dependencies()[libcxxwrap_julia_jll_UUID].version === v"0.12.1+0"

        # Julia-version-dependent add (non-stdlib, specified version)
        Pkg.add(; name="libcxxwrap_julia_jll", version=v"0.9.4+0", julia_version=v"1.7")
        @test Pkg.dependencies()[libcxxwrap_julia_jll_UUID].version === v"0.9.4+0"

        Pkg.add(; name="libcxxwrap_julia_jll", version=v"0.8.8+1", julia_version=v"1.9")
        @test Pkg.dependencies()[libcxxwrap_julia_jll_UUID].version === v"0.8.8+1"
    end

    @testset "GMP_jll" begin
        # Stdlib add (current julia version)
        Pkg.add(; name="GMP_jll")
        @test Pkg.dependencies()[GMP_jll_UUID].version === v"6.2.1+6"

        # Stdlib add (other julia version)
        Pkg.add(; name="GMP_jll", julia_version=v"1.7")
        @test Pkg.dependencies()[GMP_jll_UUID].version === v"6.2.1+1"

        # Stdlib add (other julia version, with specific version bound)
        # Note, this doesn't work properly, it adds but doesn't install any artifacts.
        # Technically speaking, this is probably okay from Pkg's perspective, since
        # we're asking Pkg to resolve according to what Julia v1.7 would do.... and
        # Julia v1.7 would not install anything because it's a stdlib!  However, we
        # would sometimes like to resolve the latest version of GMP_jll for Julia v1.7
        # then install that.  If we have to manually work around that and look up what
        # GMP_jll for Julia v1.7 is, then ask for that version explicitly, that's ok.

        Pkg.add(; name="GMP_jll", julia_version=v"1.7")

        # This is expected to fail, that version can't live with `julia_version = v"1.7"`
        @test_throws Pkg.Resolve.ResolverError Pkg.add(; name="GMP_jll", version=v"6.2.0+5", julia_version=v"1.7")

        # Stdlib add (julia_version == nothing)
        # Note: this is currently known to be broken, we get the wrong GMP_jll!
        Pkg.add(; name="GMP_jll", version=v"6.2.1+1", julia_version=nothing)
        @test_broken Pkg.dependencies()[GMP_jll_UUID].version === v"6.2.1+1"
    end

    @testset "Julia Version = Nothing" begin
        # Stdlib add (impossible constraints due to julia version compat, so
        # must pass `julia_version=nothing`). In this case, we always fully
        # specify versions, but if we don't, it's okay to just give us whatever
        # the resolver prefers
        Pkg.add([
            PackageSpec(;name="OpenBLAS_jll",  version=v"0.3.13"),
            PackageSpec(;name="libblastrampoline_jll", version=v"5.1.1"),
        ]; julia_version=nothing)
        @test v"0.3.14" > Pkg.dependencies()[OpenBLAS_jll_UUID].version >= v"0.3.13"
        @test v"5.1.2" > Pkg.dependencies()[libblastrampoline_jll_UUID].version >= v"5.1.1"
    end
end

mkitti avatar Mar 05 '24 08:03 mkitti

I gave an update on https://github.com/JuliaPackaging/HistoricalStdlibVersions.jl/issues/9#issuecomment-1978758724

KristofferC avatar Mar 05 '24 13:03 KristofferC