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 one artifact for each deposited GAP package #1069

Merged
merged 2 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1,623 changes: 1,618 additions & 5 deletions Artifacts.toml

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## Version 0.13.0-DEV (released YYYY-MM-DD)

- Update to GAP 4.14.0
- Instead of downloading a single huge "artifact" containing all deposited GAP
packages, we now use (and download) one artifact per GAP package.

## Version 0.12.1 (released 2024-12-09)

Expand Down
2 changes: 2 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
REPL = "3fa0cd96-eef1-5676-8a61-b3b8758bbffb"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
Scratch = "6c6a2e73-6563-6170-7368-637461726353"
TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76"

[compat]
AbstractAlgebra = "0.41.11, 0.42.1, 0.43"
Expand All @@ -40,4 +41,5 @@ Pkg = "1.6"
REPL = "1.6"
Random = "1.6"
Scratch = "1.1"
TOML = "<0.0.1, 1"
julia = "1.6"
8 changes: 8 additions & 0 deletions etc/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[deps]
ArtifactUtils = "8b73e784-e7d8-4ea5-973d-377fed4e3bce"
Artifacts = "56f22d72-fd6d-98f1-02f0-08ddc0907c33"
Downloads = "f43a241f-c20a-4ad4-852c-f6b1247861c6"
GZip = "92fee26a-97fe-5a0c-ad85-20a5f3185b63"
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
16 changes: 5 additions & 11 deletions etc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,8 @@ For this to work, follow these instructions:
5. This opens a Julia session with the override in effect. You can now e.g. load GAP.jl
via `using GAP`, or install other packages (such as Oscar) and test with them.

## Updating the package tarball

```
using ArtifactUtils
add_artifact!(
"Artifacts.toml",
"gap_packages",
"https://github.com/gap-system/PackageDistro/releases/download/v4.13.1/packages.tar.gz";
force=true
)
```
## Updating the package artifacts

Run this from the root director of GAP.jl:

julia --project=etc etc/update_artifacts.jl 4.X.Y
132 changes: 132 additions & 0 deletions etc/update_artifacts.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
#
# This script is used to update Artifacts.toml
#
# Usage variants:
# julia --project=etc etc/update_artifacts.jl 4.14.0
# julia --project=etc etc/update_artifacts.jl https://.../package-infos.json
# julia --project=etc etc/update_artifacts.jl https://.../package-infos.json.gz
# julia --project=etc etc/update_artifacts.jl local/path/package-infos.json
# julia --project=etc etc/update_artifacts.jl local/path/package-infos.json.gz
#

using Downloads: download
import Pkg
using Pkg.Artifacts
import Pkg.PlatformEngines
#using Pkg.GitTools
import GZip
import JSON
import TOML
import SHA


function sha256sum(tarball_path)
return open(tarball_path, "r") do io
return bytes2hex(SHA.sha256(io))
end
end

function add_artifacts_for_packages(; pkginfos_path::String = "package-infos.json", artifacts_toml::String="Artifacts.toml")
pkgs = GZip.open(JSON.parse, pkginfos_path, "r")
artifacts = TOML.parsefile(artifacts_toml)

for name in sort(collect(keys(pkgs)))
print("Processing '$name' ")
pkginfo = pkgs[name]
add_artifacts_for_package(pkginfo, artifacts)

# write it all out again
open(artifacts_toml, "w") do io
TOML.print(io, artifacts; sorted=true)
end

end

# delete artifacts for any packages that are no longer distributed with GAP
pkg_names = ["GAP_pkg_"*lowercase(pkginfo["PackageName"]) for (name, pkginfo) in pkgs]
to_be_removed = setdiff(keys(artifacts), pkg_names)
for name in to_be_removed
delete!(artifacts, name)
end

# write it all out again
open(artifacts_toml, "w") do io
TOML.print(io, artifacts; sorted=true)
end

return nothing
end

function add_artifacts_for_package(pkginfo, artifacts)
gap_pkgname = pkginfo["PackageName"]
pkgname = lowercase(gap_pkgname)
artifact_name = "GAP_pkg_$(pkgname)"

#
# extract info about the package tarball
#
sha256 = pkginfo["ArchiveSHA256"]
url = pkginfo["ArchiveURL"]
formats = split(pkginfo["ArchiveFormats"], " ")
url *= first(formats) # this matches what the PackageDistro does, and allow us to use ArchiveSHA256
url2 = "https://files.gap-system.org/pkg/" * basename(url)

# check if this file is already registered
if haskey(artifacts, artifact_name)
downloads = artifacts[artifact_name]["download"]
d = Dict("sha256" => sha256, "url" => url)
if d in downloads
println(" already present")
d2 = Dict("sha256" => sha256, "url" => url2)
if !(d2 in downloads)
println(" added backup URL $(url2)")
push!(downloads, d2)
end
return
end
end

# add the artifact
println(" importing new archive $url")
tarball_path = download(url)
tarball_hash = sha256sum(tarball_path)
if sha256 != tarball_hash
error("SHA256 mismatch for $url")
end

git_tree_sha1 = create_artifact() do artifact_dir
Pkg.PlatformEngines.unpack(tarball_path, artifact_dir)
end

rm(tarball_path)
#clear && remove_artifact(git_tree_sha1)

artifacts[artifact_name] = Dict{String,Any}(
"git-tree-sha1" => bytes2hex(git_tree_sha1.bytes),
"download" => [ Dict("sha256" => sha256, "url" => url),
Dict("sha256" => sha256, "url" => url2) ]
)

return
end

# https://github.com/gap-system/gap/releases/download/v4.14.0/package-infos.json.gz
# https://github.com/gap-system/gap/releases/download/v4.14.0/package-infos.json.gz.sha256

if length(ARGS) > 0
desc = ARGS[1]
if startswith(desc, "http")
pkginfos_url = desc
println("Download package-infos from $(pkginfos_url)")
pkginfos_path = download(pkginfos_url)
elseif startswith(desc, "4.")
pkginfos_url = "https://github.com/gap-system/gap/releases/download/v$desc/package-infos.json.gz"
println("Download package-infos from $(pkginfos_url)")
pkginfos_path = download(pkginfos_url)
else
pkginfos_path = desc
end

println("processing $(pkginfos_path)")
add_artifacts_for_packages(; pkginfos_path)
end
13 changes: 9 additions & 4 deletions src/setup.jl
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
module Setup

using Pkg: GitTools
import Artifacts: @artifact_str
import Artifacts: find_artifacts_toml, @artifact_str
import GAP_jll
import GAP_lib_jll
import GAP_pkg_juliainterface_jll
import Scratch: @get_scratch!
import Pidfile
import TOML

# to separate the scratchspaces of different GAP.jl copies and Julia versions
# put the Julia version and the hash of the path to this file into the key
const scratch_key = "gap_$(string(hash(@__FILE__)))_$(VERSION.major).$(VERSION.minor)"
const scratch_key = "gap_$(hash(@__FILE__))-$(VERSION.major).$(VERSION.minor)"
Copy link
Member

Choose a reason for hiding this comment

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

IIuc this change is here to not use the same scratchspace as before as that could lead to conflicts, right? Wouldn't it make sense to either include some kind of increasing number in either the key or in the hash input, so we can bump that again for the next incompatible change in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually had an intermediate version where I computed a more complicated hash, which involved the paths of the various artifacts (!) as those can also change; but also checksums of e.g. setup.jl and other files, but then realized this doesn't solve all the problems either...

Here are some further alternatives:

  • as above, include more data into the hash (e.g. GAP.jl version, location of the various JLLs and artifacts we link to, ...)
  • stop using a scratch space, instead just create a fresh "mutable root dir" each time the package is loaded in a fresh temp dir (of course that means a lot of temp dirs would accumulate... we could add an atexit handler to delete them at the end?)
    • would be interesting how much time we spend creating the mutable root dir
  • we could adjust the setup code so that whenever it e.g. creates a dir or symlink it checks if there is already an object of the correct type there -- i.e. if we created a dir pkg and there is already a dir pkg, do nothing; but if we create a symlink pkg pointing to targetA and there is instead a directory pkg; or a symlink pointing to anotherTarget, then delete pkg.
    • this seems much more complicated and in every way I can think of inferior to the solution of just rebuilding it all from scratch, except that at least the path to this root dir is somewhat stable and predictable
    • strictly speaking we'd also have to scan for obsolete file system objects that should be removed... making this even more complicated...

Copy link
Member Author

Choose a reason for hiding this comment

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

See also #993

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, yeah. No strong preference between 1 and 2.
But it seems that this is not immediately needed as running this PR works locally


gaproot() = @get_scratch!(scratch_key)

Expand Down Expand Up @@ -195,8 +196,12 @@ function regenerate_gaproot()
force_symlink("../../gac",
joinpath(gaproot_mutable, "bin", sysinfo["GAParch"], "gac"))

# create a `pkg` symlink to the GAP packages artifact
force_symlink(artifact"gap_packages", "$gaproot_mutable/pkg")
# create a `pkg` directory with symlinks to all the GAP packages artifacts
mkpath(joinpath(gaproot_mutable, "pkg"))
pkg_artifacts = filter(startswith("GAP_pkg_"),keys(TOML.parsefile(find_artifacts_toml(@__FILE__))))
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
for name in pkg_artifacts
force_symlink(@artifact_str(name), joinpath(gaproot_mutable, "pkg", name))
end

end # mkpidlock

Expand Down
Loading