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.

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

Avoid deleting existing artifacts when ignoring hashes. #3768

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Avoid deleting existing artifacts when ignoring hashes. #3768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0dd7fa4
avoid deleting existing artifacts
nhz2 Jan 23, 2024
51f3f68
fix order
nhz2 Jan 23, 2024
410f7d2
add testset
nhz2 Jan 23, 2024
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
132 changes: 78 additions & 54 deletions src/Artifacts.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,7 @@ function create_artifact(f::Function)
# as something that was foolishly overridden. This should be virtually impossible
# unless the user has been very unwise, but let's be cautious.
new_path = artifact_path(artifact_hash; honor_overrides=false)
if !isdir(new_path)
# Move this generated directory to its final destination, set it to read-only
mv(temp_dir, new_path)
chmod(new_path, filemode(dirname(new_path)))
set_readonly(new_path)
end
_mv_temp_artifact_dir(temp_dir, new_path)

# Give the people what they want
return artifact_hash
Expand All @@ -64,6 +59,28 @@ function create_artifact(f::Function)
end
end

"""
_mv_temp_artifact_dir(temp_dir::String, new_path::String)::Nothing
Either rename the directory at `temp_dir` to `new_path` and set it to read-only
or if `new_path` artifact already exists try to do nothing.
"""
function _mv_temp_artifact_dir(temp_dir::String, new_path::String)::Nothing
if !isdir(new_path)
# This next step is like
# `mv(temp_dir, new_path)`.
# However, `mv` defaults to `cp` if `rename` returns an error.
# `cp` is not atomic, so avoid the potential of calling it.
err = ccall(:jl_fs_rename, Int32, (Cstring, Cstring), temp_dir, new_path)
# Ignore rename error, but ensure `new_path` exists.
if !isdir(new_path)
error("$(repr(new_path)) could not be made")
end
chmod(new_path, filemode(dirname(new_path)))
set_readonly(new_path)
end
nothing
end

"""
remove_artifact(hash::SHA1; honor_overrides::Bool=false)

Expand Down Expand Up @@ -289,68 +306,75 @@ function download_artifact(
return true
end

# We download by using `create_artifact()`. We do this because the download may
# Ensure the `artifacts` directory exists in our default depot
artifacts_dir = first(artifacts_dirs())
mkpath(artifacts_dir)
# expected artifact path
dst = joinpath(artifacts_dir, bytes2hex(tree_hash.bytes))

# We download by using a temporary directory. We do this because the download may
# be corrupted or even malicious; we don't want to clobber someone else's artifact
# by trusting the tree hash that has been given to us; we will instead download it
# to a temporary directory, calculate the true tree hash, then move it to the proper
# location only after knowing what it is, and if something goes wrong in the process,
# everything should be cleaned up. Luckily, that is precisely what our
# `create_artifact()` wrapper does, so we use that here.
calc_hash = try
create_artifact() do dir
download_verify_unpack(tarball_url, tarball_hash, dir, ignore_existence=true, verbose=verbose,
# everything should be cleaned up.

# Temporary directory where we'll do our creation business
temp_dir = mktempdir(artifacts_dir)

try
download_verify_unpack(tarball_url, tarball_hash, temp_dir, ignore_existence=true, verbose=verbose,
quiet_download=quiet_download, io=io)
calc_hash = SHA1(GitTools.tree_hash(temp_dir))

# Did we get what we expected? If not, freak out.
if calc_hash.bytes != tree_hash.bytes
msg = """
Tree Hash Mismatch!
Expected git-tree-sha1: $(bytes2hex(tree_hash.bytes))
Calculated git-tree-sha1: $(bytes2hex(calc_hash.bytes))
"""
# Since tree hash calculation is rather fragile and file system dependent,
# we allow setting JULIA_PKG_IGNORE_HASHES=1 to ignore the error and move
# the artifact to the expected location and return true
ignore_hash_env_set = get(ENV, "JULIA_PKG_IGNORE_HASHES", "") != ""
if ignore_hash_env_set
ignore_hash = Base.get_bool_env("JULIA_PKG_IGNORE_HASHES", false)
ignore_hash === nothing && @error(
"Invalid ENV[\"JULIA_PKG_IGNORE_HASHES\"] value",
ENV["JULIA_PKG_IGNORE_HASHES"],
)
ignore_hash = something(ignore_hash, false)
else
# default: false except Windows users who can't symlink
ignore_hash = Sys.iswindows() &&
!mktempdir(can_symlink, artifacts_dir)
end
if ignore_hash
desc = ignore_hash_env_set ?
"Environment variable \$JULIA_PKG_IGNORE_HASHES is true" :
"System is Windows and user cannot create symlinks"
msg *= "\n$desc: \
ignoring hash mismatch and moving \
artifact to the expected location"
@error(msg)
else
error(msg)
end
end
# Move it to the location we expected
_mv_temp_artifact_dir(temp_dir, dst)
catch err
@debug "download_artifact error" tree_hash tarball_url tarball_hash err
if isa(err, InterruptException)
rethrow(err)
end
# If something went wrong during download, return the error
return err
finally
# Always attempt to cleanup
rm(temp_dir; recursive=true, force=true)
end

# Did we get what we expected? If not, freak out.
if calc_hash.bytes != tree_hash.bytes
msg = """
Tree Hash Mismatch!
Expected git-tree-sha1: $(bytes2hex(tree_hash.bytes))
Calculated git-tree-sha1: $(bytes2hex(calc_hash.bytes))
"""
# actual and expected artifiact paths
src = artifact_path(calc_hash; honor_overrides=false)
dst = artifact_path(tree_hash; honor_overrides=false)
# Since tree hash calculation is rather fragile and file system dependent,
# we allow setting JULIA_PKG_IGNORE_HASHES=1 to ignore the error and move
# the artifact to the expected location and return true
ignore_hash_env_set = get(ENV, "JULIA_PKG_IGNORE_HASHES", "") != ""
if ignore_hash_env_set
ignore_hash = Base.get_bool_env("JULIA_PKG_IGNORE_HASHES", false)
ignore_hash === nothing && @error(
"Invalid ENV[\"JULIA_PKG_IGNORE_HASHES\"] value",
ENV["JULIA_PKG_IGNORE_HASHES"],
)
ignore_hash = something(ignore_hash, false)
else
# default: false except Windows users who can't symlink
ignore_hash = Sys.iswindows() &&
!mktempdir(can_symlink, dirname(src))
end
if ignore_hash
desc = ignore_hash_env_set ?
"Environment variable \$JULIA_PKG_IGNORE_HASHES is true" :
"System is Windows and user cannot create symlinks"
msg *= "\n$desc: \
ignoring hash mismatch and moving \
artifact to the expected location"
@error(msg)
# Move it to the location we expected
mv(src, dst; force=true)
return true
end
return ErrorException(msg)
end

return true
end

Expand Down
26 changes: 26 additions & 0 deletions test/artifacts.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,4 +799,30 @@ end
end
end

@testset "installing artifacts when symlinks are copied" begin
# copy symlinks to simulate the typical Microsoft Windows user experience where
# developer mode is not enabled (no admin rights)
withenv("BINARYPROVIDER_COPYDEREF"=>"true", "JULIA_PKG_IGNORE_HASHES"=>"true") do
temp_pkg_dir() do tmpdir
artifacts_toml = joinpath(tmpdir, "Artifacts.toml")
cp(joinpath(@__DIR__, "test_packages", "ArtifactInstallation", "Artifacts.toml"), artifacts_toml)
Pkg.activate(tmpdir)
cts_real_hash = create_artifact() do dir
local meta = Artifacts.artifact_meta("collapse_the_symlink", artifacts_toml)
local collapse_url = meta["download"][1]["url"]
local collapse_hash = meta["download"][1]["sha256"]
# Because "BINARYPROVIDER_COPYDEREF"=>"true", this will copy symlinks.
download_verify_unpack(collapse_url, collapse_hash, dir; verbose=true, ignore_existence=true)
end
cts_hash = artifact_hash("collapse_the_symlink", artifacts_toml)
@test !artifact_exists(cts_hash)
@test artifact_exists(cts_real_hash)
@test_logs (:error, r"Tree Hash Mismatch!") match_mode=:any Pkg.instantiate()
@test artifact_exists(cts_hash)
# Make sure existing artifacts don't get deleted.
@test artifact_exists(cts_real_hash)
end
end
end

end # module