download/unpack: some code cleanup (#2641)

* download/unpack: some code cleanup
* move list_tarball_files into test/utils (only used for tests)
* platform engines: minor tweaks

Author: StefanKarpinski

src/Operations.jl

@@ -498,15 +498,7 @@ function install_archive(
         url_success || continue
         dir = joinpath(tempdir(), randstring(12))
         push!(tmp_objects, dir) # for cleanup
-        # Might fail to extract an archive (Pkg#190)
-        try
-            unpack(path, dir; verbose=false)
-        catch e
-            e isa InterruptException && rethrow()
-            @warn "failed to extract archive downloaded from $(url)"
-            url_success = false
-        end
-        url_success || continue
+        unpack(path, dir; verbose=false)
         if top
             unpacked = dir
         else

src/PlatformEngines.jl

@@ -9,7 +9,7 @@ import ...Pkg: Pkg, TOML, pkg_server, depots1, can_fancyprint, stderr_f
 using ..MiniProgressBars
 using Base.BinaryPlatforms, p7zip_jll
 
-export probe_platform_engines!, verify, unpack, package, download_verify_unpack
+export verify, unpack, package, download_verify_unpack
 
 const EXE7Z_LOCK = ReentrantLock()
 const EXE7Z = Ref{String}()
@@ -40,14 +40,13 @@ function find7z()
     error("7z binary not found")
 end
 
-function probe_platform_engines!(;verbose::Bool = false)
-    # don't do anything
-end
-
 is_secure_url(url::AbstractString) =
     occursin(r"^(https://|\w+://(127\.0\.0\.1|localhost)(:\d+)?($|/))"i, url)
 
-function get_server_dir(url::AbstractString, server=pkg_server())
+function get_server_dir(
+    url :: AbstractString,
+    server :: Union{AbstractString, Nothing} = pkg_server(),
+)
     server === nothing && return
     url == server || startswith(url, "$server/") || return
     m = match(r"^\w+://([^\\/]+)(?:$|/)", server)
@@ -374,14 +373,6 @@ function unpack(
     Tar.extract(`$(exe7z()) x $tarball_path -so`, dest, copy_symlinks = copy_symlinks())
 end
 
-function list_tarball_files(tarball_path::AbstractString)
-    names = String[]
-    Tar.list(`$(exe7z()) x $tarball_path -so`) do hdr
-        push!(names, hdr.path)
-    end
-    return names
-end
-
 """
     package(src_dir::AbstractString, tarball_path::AbstractString)
 

src/Types.jl

@@ -14,7 +14,6 @@ using TOML
 import ..Pkg, ..Registry
 import ..Pkg: GitTools, depots, depots1, logdir, set_readonly, safe_realpath, pkg_server, stdlib_dir, stdlib_path, isurl, stderr_f
 import Base.BinaryPlatforms: Platform
-import ..PlatformEngines: download, download_verify_unpack
 using ..Pkg.Versions
 
 import Base: SHA1

test/artifacts.jl

@@ -301,7 +301,7 @@ end
             hash = create_artifact(p -> touch(joinpath(p, "foo")))
             tarball_path = joinpath(art_dir, "foo.tar.gz")
             archive_artifact(hash, tarball_path)
-            @test "foo" in PlatformEngines.list_tarball_files(tarball_path)
+            @test "foo" in list_tarball_files(tarball_path)
 
             # Test archiving something that doesn't exist fails
             remove_artifact(hash)

test/platformengines.jl

@@ -2,7 +2,7 @@ module PlatformEngineTests
 import ..Pkg # ensure we are using the correct Pkg
 
 using Test, Pkg.PlatformEngines, Pkg.BinaryPlatforms, SHA
-
+using ..Utils: list_tarball_files
 
 @testset "Packaging" begin
     # Gotta set this guy up beforehand
@@ -36,7 +36,7 @@ using Test, Pkg.PlatformEngines, Pkg.BinaryPlatforms, SHA
             @test isfile(tarball_path)
 
             # Test that we can inspect the contents of the tarball
-            contents = PlatformEngines.list_tarball_files(tarball_path)
+            contents = list_tarball_files(tarball_path)
             @test "bin/bar.sh" in contents
             @test "lib/baz.so" in contents
             @test "etc/qux.conf" in contents

test/utils.jl

@@ -3,12 +3,14 @@
 module Utils
 
 import ..Pkg
+using Tar
 using TOML
 using UUIDs
 
 export temp_pkg_dir, cd_tempdir, isinstalled, write_build, with_current_env,
        with_temp_env, with_pkg_env, git_init_and_commit, copy_test_package,
-       git_init_package, add_this_pkg, TEST_SIG, TEST_PKG, isolate, LOADED_DEPOT
+       git_init_package, add_this_pkg, TEST_SIG, TEST_PKG, isolate, LOADED_DEPOT,
+       list_tarball_files
 
 const LOADED_DEPOT = joinpath(@__DIR__, "loaded_depot")
 
@@ -279,4 +281,12 @@ function add_this_pkg(; platform=Base.BinaryPlatforms.HostPlatform())
     Pkg.develop(spec; platform)
 end
 
+function list_tarball_files(tarball_path::AbstractString)
+    names = String[]
+    Tar.list(`$(Pkg.PlatformEngines.exe7z()) x $tarball_path -so`) do hdr
+        push!(names, hdr.path)
+    end
+    return names
+end
+
 end