Skip to content

Fix bundle_artifacts to honor Overrides.toml #1089

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.

Sign up for GitHub

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

Open
asinghvi17 wants to merge 1 commit into
base: kc/distro
Choose a base branch
from
Open

Fix bundle_artifacts to honor Overrides.toml #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 16 additions & 15 deletions src/PackageCompiler.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1910,7 +1910,7 @@ function pretty_byte_str(size)
end

# Copy pasted from Pkg since `collect_artifacts` doesn't allow lazy artifacts to get installed
function _collect_artifacts(pkg_root::String; platform::Base.BinaryPlatforms.AbstractPlatform=Base.BinaryPlatforms.HostPlatform(), include_lazy::Bool)
function _collect_artifacts(pkg_root::String; platform::Base.BinaryPlatforms.AbstractPlatform=Base.BinaryPlatforms.HostPlatform(), include_lazy::Bool, pkg_uuid::Union{Nothing, Base.UUID}=nothing)
# Check to see if this package has an (Julia)Artifacts.toml
artifacts_tomls = Tuple{String,Base.TOML.TOMLDict}[]

Expand All @@ -1934,7 +1934,7 @@ function _collect_artifacts(pkg_root::String; platform::Base.BinaryPlatforms.Abs
push!(artifacts_tomls, (artifacts_toml, TOML.parse(meta_toml)))
else
# Otherwise, use the standard selector from `Artifacts`
artifacts = Pkg.Artifacts.select_downloadable_artifacts(artifacts_toml; platform, include_lazy)
artifacts = Pkg.Artifacts.select_downloadable_artifacts(artifacts_toml; platform, include_lazy, pkg_uuid)
push!(artifacts_tomls, (artifacts_toml, artifacts))
end
break
Expand All @@ -1951,35 +1951,36 @@ function bundle_artifacts(ctx, dest_dir; include_lazy_artifacts::Bool)
depot_path = joinpath(dest_dir, "share", "julia")
artifact_app_path = joinpath(depot_path, "artifacts")

source_paths_names = Tuple{String, String}[]
source_paths_names = Tuple{String, String, Union{Nothing, Base.UUID}}[]
for pkg in pkgs
pkg_source_path = source_path(ctx, pkg)
pkg_source_path === nothing && continue
push!(source_paths_names, (pkg_source_path, pkg.name))
push!(source_paths_names, (pkg_source_path, pkg.name, pkg.uuid))
end
# Also want artifacts for the project itself
push!(source_paths_names, (dirname(ctx.env.project_file), ctx.env.project_file))
push!(source_paths_names, (dirname(ctx.env.project_file), ctx.env.project_file, nothing))

bundled_artifacts = Pair{String, Vector{Pair{String, String}}}[]
bundled_artifacts = Pair{String, Vector{Tuple{String, String, String}}}[]

for (pkg_source_path, pkg_name) in source_paths_names
bundled_artifacts_pkg = Pair{String, String}[]
for (pkg_source_path, pkg_name, pkg_uuid) in source_paths_names
bundled_artifacts_pkg = Tuple{String, String, String}[]
if isdefined(Pkg.Operations, :collect_artifacts)
for (artifacts_toml, artifacts) in _collect_artifacts(pkg_source_path; platform, include_lazy=include_lazy_artifacts)
for (artifacts_toml, artifacts) in _collect_artifacts(pkg_source_path; platform, include_lazy=include_lazy_artifacts, pkg_uuid)
for (name, data) in artifacts
Pkg.ensure_artifact_installed(name, artifacts[name], artifacts_toml; platform)
hash = Base.SHA1(data["git-tree-sha1"])
push!(bundled_artifacts_pkg, name => artifact_path(hash))
push!(bundled_artifacts_pkg, (name, bytes2hex(hash.bytes), artifact_path(hash)))
end
end
else
for f in Pkg.Artifacts.artifact_names
artifacts_toml_path = joinpath(pkg_source_path, f)
if isfile(artifacts_toml_path)
artifacts = Artifacts.select_downloadable_artifacts(artifacts_toml_path; platform, include_lazy=include_lazy_artifacts)
artifacts = Artifacts.select_downloadable_artifacts(artifacts_toml_path; platform, include_lazy=include_lazy_artifacts, pkg_uuid)
for name in keys(artifacts)
artifact_path = Pkg.ensure_artifact_installed(name, artifacts[name], artifacts_toml_path; platform)
push!(bundled_artifacts_pkg, name => artifact_path)
hash_hex = artifacts[name]["git-tree-sha1"]
artifact_resolved = Pkg.ensure_artifact_installed(name, artifacts[name], artifacts_toml_path; platform)
push!(bundled_artifacts_pkg, (name, hash_hex, artifact_resolved))
end
break
end
Expand Down Expand Up @@ -2008,8 +2009,8 @@ function bundle_artifacts(ctx, dest_dir; include_lazy_artifacts::Bool)
if !std_jll
println()
end
for (j, (artifact, artifact_path)) in enumerate(artifacts)
git_tree_sha_artifact = basename(artifact_path)
for (j, (artifact, hash_hex, artifact_path)) in enumerate(artifacts)
git_tree_sha_artifact = hash_hex
already_bundled = git_tree_sha_artifact in bundled_shas
size = already_bundled ? 0 : recursive_dir_size(artifact_path)
total_size += size
Expand Down
60 changes: 60 additions & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,66 @@ end
end
end

@testset "bundle_artifacts with overrides" begin
# Test that bundle_artifacts honors Overrides.toml (UUID-based overrides).
# This verifies two fixes:
# 1. pkg_uuid is forwarded so process_overrides converts UUID entries to hash lookups
# 2. The copy loop uses the known hash, not basename(artifact_path)

app_source_dir = joinpath(@__DIR__, "..", "examples/MyApp/")
tmp_override_app = joinpath(tmp, "MyAppOverride")
cp(app_source_dir, tmp_override_app)
ctx = PackageCompiler.create_pkg_context(tmp_override_app)
Pkg.instantiate(ctx)

# Find HelloWorldC_jll artifact hash via the Pkg context
pkgs = PackageCompiler.load_all_deps(ctx)
jll_pkg = only(filter(p -> p.name == "HelloWorldC_jll", pkgs))
jll_pkg_path = PackageCompiler.source_path(ctx, jll_pkg)
jll_artifacts_toml = joinpath(jll_pkg_path, "Artifacts.toml")
jll_hash = Pkg.Artifacts.artifact_hash("HelloWorldC", jll_artifacts_toml)
original_artifact = Pkg.Artifacts.artifact_path(jll_hash)
@assert isdir(original_artifact) "HelloWorldC_jll artifact not found at $original_artifact"

# Create override: copy artifact to temp dir and add a marker file
override_dir = mktempdir()
override_artifact = joinpath(override_dir, "HelloWorldC")
cp(original_artifact, override_artifact)
touch(joinpath(override_artifact, "OVERRIDE_MARKER"))

# Write UUID-based Overrides.toml in the test depot
# HelloWorldC_jll UUID = dca1746e-5efc-54fc-8249-22745bc95a49
overrides_toml = joinpath(new_depot, "artifacts", "Overrides.toml")
mkpath(dirname(overrides_toml))
write(overrides_toml, """
[dca1746e-5efc-54fc-8249-22745bc95a49]
HelloWorldC = "$(escape_string(override_artifact))"
""")

# Force the Artifacts stdlib to reload overrides (cache may be stale from prior tests)
import Artifacts as ArtifactsStdlib
ArtifactsStdlib.ARTIFACT_OVERRIDES[] = nothing

dest_dir = mktempdir()
try
PackageCompiler.bundle_artifacts(ctx, dest_dir; include_lazy_artifacts=true)

# The overridden artifact should be bundled under the original hash name
bundled_artifact = joinpath(dest_dir, "share", "julia", "artifacts", bytes2hex(jll_hash.bytes))
@test isdir(bundled_artifact)
@test isfile(joinpath(bundled_artifact, "OVERRIDE_MARKER"))
finally
rm(overrides_toml; force=true)
# Reset override cache so subsequent tests aren't affected
ArtifactsStdlib.ARTIFACT_OVERRIDES[] = nothing
rm(override_dir; recursive=true, force=true)
rm(tmp_override_app; recursive=true, force=true)
rm(joinpath(new_depot, "packages"); recursive=true, force=true)
rm(joinpath(new_depot, "compiled"); recursive=true, force=true)
rm(joinpath(new_depot, "artifacts"); recursive=true, force=true)
end
end

@testset "create_library" begin
# Test library creation
lib_source_dir = joinpath(@__DIR__, "..", "examples/MyLib")
Expand Down
Loading