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

Restore previous builds of JLLs which (currently) depend on JLLWrappers 1.7 #122129

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

giordano
Copy link
Member

Bash script used to produce this PR:

#!/bin/bash

set -euo pipefail

GENERAL_REPO="${HOME}/repo/General"

JLLS=(
    OpenSSL_jll
    Hwloc_jll
    JpegTurbo_jll
    libpng_jll
    libsixel_jll
    Bzip2_jll
    FFTW_jll
    Giflib_jll
    LERC_jll
    Expat_jll
    Libuuid_jll
    Libmount_jll
    OpenSpecFun_jll
    Lz4_jll
    LZO_jll
    Pixman_jll
    Libffi_jll
    Qhull_jll
    Zstd_jll
    SQLite_jll
    XZ_jll
    libaec_jll
    boost_jll
    brotli_jll
    pugixml_jll
    yaml_cpp_jll
    snappy_jll
    MPItrampoline_jll
    Blosc_jll
    Blosc2_jll
    libzip_jll
    libsodium_jll
    ZeroMQ_jll
    Libgpg_error_jll
    libusb_jll
    Nettle_jll
    Xorg_libXau_jll
    Xorg_xcb_proto_jll
    Xorg_libpthread_stubs_jll
    Xorg_xtrans_jll
    Xorg_libXdmcp_jll
    Xorg_libXext_jll
    Xorg_libX11_jll
    Xorg_libxcb_jll
)

for jll in "${JLLS[@]}"; do
    dir="jll/$(echo ${jll:0:1} | tr [:lower:] [:upper:])/${jll}"
    compat="${dir}/Compat.toml"
    versions="${dir}/Versions.toml"
    last_hash=$(git -C "${GENERAL_REPO}" log -1 --oneline --format=%H -- "${compat}")
    # Restore Compat.toml to previous state
    git -C "${GENERAL_REPO}" reset "${last_hash}^" -- "${compat}"
    git -C "${GENERAL_REPO}" restore -- "${compat}"
    # Yank last version
    if [[ $(tail -n 1 "${GENERAL_REPO}/${versions}") != "yanked = true" ]]; then
        echo "yanked = true" >> "${GENERAL_REPO}/${versions}"
    fi
    git -C "${GENERAL_REPO}" add "${versions}"
done

With

% find jll -name 'Compat.toml' -exec grep -H 'JLLWrappers.*1\.7\.0' '{}' \;
jll/R/Reactant_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/N/nghttp2_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/G/GMP_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/Z/Zlib_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/S/SCIP_PaPILO_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/F/Fmt_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/O/OpenLDAPClient_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/O/OpenLibm_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/H/HelloWorldC_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/H/HELICS_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/M/MbedTLS_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/D/dSFMT_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/E/Enzyme_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/L/LibCURL_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/L/LibSSH2_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/L/LibUV_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/L/libblastrampoline_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/L/LibGit2_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/P/PCRE2_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/P/PDAL_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/P/p7zip_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/W/Wayland_protocols_jll/Compat.toml:JLLWrappers = "1.7.0-1"
jll/W/Wasmtime_jll/Compat.toml:JLLWrappers = "1.7.0-1"

you can then see that all packages which still refer to JLLWrappers 1.7.0 are either stdlibs or packages which which had a brand new version (Reactant, Enzyme, SCIP_PaPILO, Fmt, OpenLDAPClient_jll, HelloWorldC, HELICS, Wayland_protocols, PDAL, Wasmtime).

@giordano
Copy link
Member Author

giordano commented Dec 29, 2024

New approach: duplicate previous build, which didn't depend on JLLWrappers. New version of the script:

#!/bin/bash

set -euo pipefail

GENERAL_REPO="${HOME}/repo/General"

JLLS=(
    OpenSSL_jll
    Hwloc_jll
    JpegTurbo_jll
    libpng_jll
    libsixel_jll
    Bzip2_jll
    FFTW_jll
    Giflib_jll
    LERC_jll
    Expat_jll
    Libuuid_jll
    Libmount_jll
    OpenSpecFun_jll
    Lz4_jll
    LZO_jll
    Pixman_jll
    Libffi_jll
    Qhull_jll
    Zstd_jll
    SQLite_jll
    XZ_jll
    libaec_jll
    boost_jll
    brotli_jll
    pugixml_jll
    yaml_cpp_jll
    snappy_jll
    MPItrampoline_jll
    Blosc_jll
    Blosc2_jll
    libzip_jll
    libsodium_jll
    ZeroMQ_jll
    Libgpg_error_jll
    libusb_jll
    Nettle_jll
    Xorg_libXau_jll
    Xorg_xcb_proto_jll
    Xorg_libpthread_stubs_jll
    Xorg_xtrans_jll
    Xorg_libXdmcp_jll
    Xorg_libXext_jll
    Xorg_libX11_jll
    Xorg_libxcb_jll
)

for jll in "${JLLS[@]}"; do
    dir="jll/$(echo ${jll:0:1} | tr [:lower:] [:upper:])/${jll}"
    compat="${dir}/Compat.toml"
    versions="${dir}/Versions.toml"
    last_hash=$(git -C "${GENERAL_REPO}" log -1 --oneline --format=%H -- "${compat}")
    # Restore Compat.toml to previous state
    git -C "${GENERAL_REPO}" reset "${last_hash}^" -- "${compat}"
    git -C "${GENERAL_REPO}" restore -- "${compat}"
    # Clean up any local changes to Versions.toml
    git -C "${GENERAL_REPO}" restore --staged -- "${versions}"
    git -C "${GENERAL_REPO}" checkout -- "${versions}"
    # Find second to last version and bump build number by 2
    old_version_entry=$(tail -n 5 "${GENERAL_REPO}/${versions}"|head -n 2)
    new_build_version=$(($(echo "${old_version_entry}"|perl -lne 'print $1 if /[0-9.]+\+([0-9]+)/;') + 2))
    # Append new entry, duplicate of the second to last one but change the build number.
    echo >> "${GENERAL_REPO}/${versions}"
    echo "$(echo "${old_version_entry}" | sed -E "s/([0-9.]+\+)[0-9]+/\1${new_build_version}/")" >> "${GENERAL_REPO}/${versions}"
    git -C "${GENERAL_REPO}" add "${versions}"
done

@giordano giordano changed the title Yank versions of packages which changed compat bound for JLLWrappers Restore previous builds of JLLs which depend on JLLWrappers 1.7 Dec 29, 2024
giordano referenced this pull request Dec 30, 2024
* New version: FFTW_jll v3.3.10+2

UUID: f5851436-0d7a-5f13-b9de-f02708fd171a
Repo: https://github.com/JuliaBinaryWrappers/FFTW_jll.jl.git
Tree: 5cf2433259aa3985046792e2afc01fcec076b549

Registrator tree SHA: 191228b6dd8b9d0e2965ae3e705fe54c51dcfee8

* Update Compat.toml

---------

Co-authored-by: Mosè Giordano <[email protected]>
@DilumAluthge
Copy link
Member

@giordano Should the PR title say "Restore previous builds of JLLs which depend on JLLWrappers 1.2" instead?

@giordano
Copy link
Member Author

  1. it's not necessarily 1.2, could also be 1.4 for some (didn't check)
  2. the "which" refers to the JLLs which currently depend on JLLWrappers 1.7

@DilumAluthge
Copy link
Member

2. the "which" refers to the JLLs which currently depend on JLLWrappers 1.7

Ah, yeah that was my confusion. Maybe let's update the PR title to clarify that.

@DilumAluthge DilumAluthge changed the title Restore previous builds of JLLs which depend on JLLWrappers 1.7 Restore previous builds of JLLs which (currently) depend on JLLWrappers 1.7 Dec 30, 2024
@DilumAluthge
Copy link
Member

  1. it's not necessarily 1.2, could also be 1.4 for some (didn't check)

Yeah, I should've written "1.2 through 1.6".

@giordano
Copy link
Member Author

Any comment on the content of the PR besides the title?

@DilumAluthge
Copy link
Member

Yeah. I'm a little confused by the PR changes. Give me a minute, let me find an example.

@DilumAluthge
Copy link
Member

Okay, so using OpenSpecFun_jll as an example.

So, originally, we had OpenSpecFun_jll v0.5.5+0, which depended on JLLWrappers ^1.2.0.

And then OpenSpecFun_jll v0.5.5+1 was released, which depended on JLLWrappers ^1.7.0. This unfortunately changed the compat entries for all OpenSpecFun_jll v0.5.5+* to be JLLWrappers ^1.7.0.

So now this PR makes two changes to OpenSpecFun_jll:

  1. It creates a new version OpenSpecFun_jll v0.5.5+2, which has the exact same git-tree-sha1 as OpenSpecFun_jll v0.5.5+0.
  2. It edits the compat entry for all three OpenSpecFun_jll v0.5.5+* versions (OpenSpecFun_jll v0.5.5+0, OpenSpecFun_jll v0.5.5+1, and OpenSpecFun_jll v0.5.5+2) to be JLLWrappers ^1.2.0.
    • This is fine for OpenSpecFun_jll v0.5.5+0 and OpenSpecFun_jll v0.5.5+2.
    • **But isn't this technically incorrect for OpenSpecFun_jll v0.5.5+2? The registry now says that OpenSpecFun_jll v0.5.5+2 is fine with any JLLWrappers ^1.2.0. But in reality, OpenSpecFun_jll v0.5.5+2 needs JLLWrappers ^1.7.0.

So this PR technically breaks OpenSpecFun_jll v0.5.5+1, but is the idea of this PR that this breakage is intentional, because we prioritize fixing OpenSpecFun_jll v0.5.5+{0,2} over breaking OpenSpecFun_jll v0.5.5+1?

@giordano
Copy link
Member Author

So this PR technically breaks OpenSpecFun_jll v0.5.5+1, but is the idea of this PR that this breakage is intentional, because we prioritize fixing OpenSpecFun_jll v0.5.5+{0,2} over breaking OpenSpecFun_jll v0.5.5+1?

That's what @IanButterworth suggested, yes.

@IanButterworth
Copy link
Member

The premise being far fewer manifests will have +1 than +0

@giordano
Copy link
Member Author

Also, this is unlikely to effectively break the build versions we're overriding: a re-resolution of the package manager would also need to downgrade JLLWrappers to v1.6-, besides not updating the jll packages, which I don't think would happen?

@DilumAluthge
Copy link
Member

Ah, okay, yeah this PR makes sense to me. Do we want to get any other eyes on it before merging?

@fredrikekre @KristofferC @StefanKarpinski @ericphanson?

@DilumAluthge
Copy link
Member

Also, I haven't personally had the chance to test this PR to make sure it fixes the problem in existing manifests (although by glancing at the PR diff, and based on the discussion above, it does sound like this PR should fix the problem).

Has anyone tested this PR branch, to make sure it fixes the problem in existing manifests?

Copy link
Member

@DilumAluthge DilumAluthge left a comment

Choose a reason for hiding this comment

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

SGTM based on the above discussion.

@giordano giordano merged commit 70074d2 into master Dec 30, 2024
8 checks passed
@giordano giordano deleted the mg/yank-jllwrappers branch December 30, 2024 15:13
@DilumAluthge
Copy link
Member

Passing along feedback from a colleague: We can verify that this PR fixed the issue for one of our use cases.

@bauglir
Copy link

bauglir commented Jan 6, 2025

Unfortunately, it looks like the fix causes issues when these JLLs end up in a system image. Specifically, if one of the fixed versions, e.g. [email protected]+3 as a transitive dependency of Plots or Makie, gets baked into a system image and you then try to do any Pkg operation that tries to resolve you end up with an error because Pkg resolves to the original versions, e.g. [email protected]+1, instead of the one baked into the system image. The error is also quite cryptic as it only indicates that a specific version cannot be found in some map.

ERROR: KeyError: key v"1.0.11+3" not found
Stacktrace:
  [1] getindex
    @ ./dict.jl:498 [inlined]
  [2] resolve_versions!(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, julia_version::VersionNumber, installed_only::Bool)
    @ Pkg.Operations ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:452
  [3] up(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}, level::Pkg.Types.UpgradeLevel; skip_writing_project::Bool, preserve::Nothing)
    @ Pkg.Operations ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1546
  [4] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1528 [inlined]
  [5] up(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; level::Pkg.Types.UpgradeLevel, mode::Pkg.Types.PackageMode, preserve::Nothing, update_registry::Bool, skip_writing_project::Bool, kwargs::@Kwargs{})
    @ Pkg.API ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:351
  [6] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:326 [inlined]
  [7] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:164 [inlined]
  [8] #resolve#143
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:357 [inlined]
  [9] resolve
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:356 [inlined]
 [10] resolve(; io::Base.TTY, kwargs::@Kwargs{})
    @ Pkg.API ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:355
 [11] top-level scope
    @ none:1
Script to reproduce the error
#!/usr/bin/env bash

export JLL_NAME="Xorg_libXau_jll"
export JLL_VERSION="v1.0.11"

JULIA_DEPOT_PATH="$(mktemp -d)"
export JULIA_DEPOT_PATH

export SYSIMAGE_PATH="${JULIA_DEPOT_PATH}/${JLL_NAME}.so"

julia --eval "using Pkg" \
  --eval "Pkg.add(\"PackageCompiler\")" \
  --eval "using PackageCompiler" \
  --eval "Pkg.add(PackageSpec(; name=ENV[\"JLL_NAME\"], version=ENV[\"JLL_VERSION\"]))" \
  --eval "create_sysimage([ENV[\"JLL_NAME\"]]; sysimage_path=ENV[\"SYSIMAGE_PATH\"])"

julia --sysimage "${SYSIMAGE_PATH}" \
  --eval "using Pkg" \
  --eval "Pkg.resolve()"
Example output using [email protected]+3
  Installing known registries into `/tmp/tmp.1GOhypvXYn`
       Added `General` registry to /tmp/tmp.1GOhypvXYn/registries
    Updating registry at `/tmp/tmp.1GOhypvXYn/registries/General.toml`
   Resolving package versions...
   Installed Scratch ──────────── v1.2.1
   Installed Glob ─────────────── v1.3.1
   Installed RelocatableFolders ─ v1.0.1
   Installed PackageCompiler ──── v2.2.0
    Updating `/tmp/tmp.1GOhypvXYn/environments/v1.10/Project.toml`
  [9b87118b] + PackageCompiler v2.2.0
    Updating `/tmp/tmp.1GOhypvXYn/environments/v1.10/Manifest.toml`
  [c27321d9] + Glob v1.3.1
  [9b87118b] + PackageCompiler v2.2.0
  [05181044] + RelocatableFolders v1.0.1
  [6c6a2e73] + Scratch v1.2.1
  [0dad84c5] + ArgTools v1.1.1
  [56f22d72] + Artifacts
  [2a0f44e3] + Base64
  [ade2ca70] + Dates
  [f43a241f] + Downloads v1.6.0
  [7b1f6079] + FileWatching
  [b77e0a4c] + InteractiveUtils
  [4af54fe1] + LazyArtifacts
  [b27032c2] + LibCURL v0.6.4
  [76f85450] + LibGit2
  [8f399da3] + Libdl
  [56ddb016] + Logging
  [d6f4376e] + Markdown
  [ca575930] + NetworkOptions v1.2.0
  [44cfe95a] + Pkg v1.10.0
  [de0858da] + Printf
  [3fa0cd96] + REPL
  [9a3f8284] + Random
  [ea8e919c] + SHA v0.7.0
  [9e88b42a] + Serialization
  [6462fe0b] + Sockets
  [fa267f1f] + TOML v1.0.3
  [a4e569a6] + Tar v1.10.0
  [cf7118a7] + UUIDs
  [4ec0a83e] + Unicode
  [deac9b47] + LibCURL_jll v8.4.0+0
  [e37daf67] + LibGit2_jll v1.6.4+0
  [29816b5a] + LibSSH2_jll v1.11.0+1
  [c8ffd9c3] + MbedTLS_jll v2.28.2+1
  [14a3606d] + MozillaCACerts_jll v2023.1.10
  [83775a58] + Zlib_jll v1.2.13+1
  [8e850ede] + nghttp2_jll v1.52.0+1
  [3f19e933] + p7zip_jll v17.4.0+2
Precompiling project...
  6 dependencies successfully precompiled in 2 seconds
   Resolving package versions...
   Installed Xorg_libXau_jll ─ v1.0.11+3
   Installed JLLWrappers ───── v1.7.0
   Installed Preferences ───── v1.4.3
  Downloaded artifact: Xorg_libXau
    Updating `/tmp/tmp.1GOhypvXYn/environments/v1.10/Project.toml`
  [0c0b7dd1] + Xorg_libXau_jll v1.0.11+3
    Updating `/tmp/tmp.1GOhypvXYn/environments/v1.10/Manifest.toml`
  [692b3bcd] + JLLWrappers v1.7.0
  [21216c6a] + Preferences v1.4.3
  [0c0b7dd1] + Xorg_libXau_jll v1.0.11+3
Precompiling project...
  3 dependencies successfully precompiled in 1 seconds. 6 already precompiled.
Precompiling project...
  9 dependencies successfully precompiled in 3 seconds
✔ [01m:40s] PackageCompiler: compiling incremental system image
ERROR: KeyError: key v"1.0.11+3" not found
Stacktrace:
  [1] getindex
    @ ./dict.jl:498 [inlined]
  [2] resolve_versions!(env::Pkg.Types.EnvCache, registries::Vector{Pkg.Registry.RegistryInstance}, pkgs::Vector{Pkg.Types.PackageSpec}, julia_version::VersionNumber, installed_only::Bool)
    @ Pkg.Operations ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:452
  [3] up(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}, level::Pkg.Types.UpgradeLevel; skip_writing_project::Bool, preserve::Nothing)
    @ Pkg.Operations ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1546
  [4] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/Operations.jl:1528 [inlined]
  [5] up(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; level::Pkg.Types.UpgradeLevel, mode::Pkg.Types.PackageMode, preserve::Nothing, update_registry::Bool, skip_writing_project::Bool, kwargs::@Kwargs{})
    @ Pkg.API ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:351
  [6] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:326 [inlined]
  [7] up
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:164 [inlined]
  [8] #resolve#143
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:357 [inlined]
  [9] resolve
    @ ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:356 [inlined]
 [10] resolve(; io::Base.TTY, kwargs::@Kwargs{})
    @ Pkg.API ~/.asdf/installs/julia/1.10.7/share/julia/stdlib/v1.10/Pkg/src/API.jl:355
 [11] top-level scope
    @ none:1

I believe this is due to this code in Pkg and how there might be a difference in how available versions get resolved (given that build identifiers are not taken into account when comparing versions), but there isn't really too much to do there short-term given the nature of Pkg.

Note that this does not just affect Xorg_libXau_jll, I'm just using it as an example. All the JLLs mentioned in the earlier scripts expose the issue unless they have had a new version released since the fix was put in place. Therefore I think the proper solution might have to be to release actual new versions of all affected JLLs after all.

I have tried this with both Julia v1.10.7 and v1.11.2, the issue occurs on both versions.

For the time being, I have worked around this by pinning all the "old" versions. But it'd be nice if this'd "just work" instead as some of these packages might not see releases for a long time which would mean having to keep these work-arounds in place indefinitely.

@IanButterworth
Copy link
Member

I just hit this. Investigating a fix here JuliaLang/Pkg.jl#4130

@IanButterworth
Copy link
Member

That error should be fixed on julia master later today, and is queued up for 1.10.8 and 1.11.3

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.

4 participants