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

Add has_symlink public API function #166

Closed
wants to merge 3 commits into from

Conversation

mkitti
Copy link
Member

@mkitti mkitti commented Jan 8, 2024

Implement Tar.has_symlink as requested by @staticfloat

julia> using Tar, CodecZlib, Downloads

julia> Downloads.download("https://github.com/JuliaBinaryWrappers/P4est_jll.jl/releases/download/P4est-v2.8.1+2/P4est.v2.8.1.x86_64-w64-mingw32-mpi+microsoftmpi.tar.gz", "P4est.v2.8.1.x86_64-w64-mingw32-mpi+microsoftmpi.tar.gz")

julia> open(GzipDecompressorStream, "P4est.v2.8.1.x86_64-w64-mingw32-mpi+microsoftmpi.tar.gz") do io
           Tar.has_symlink(io)
       end
true

julia> Downloads.download("https://github.com/JuliaBinaryWrappers/iso_codes_jll.jl/releases/download/iso_codes-v4.15.0%2B0/iso_codes.v4.15.0.any.tar.gz", "iso_codes.v4.15.0.any.tar.gz")

julia> open(GzipDecompressorStream, "iso_codes.v4.15.0.any.tar.gz") do io
           Tar.has_symlink(io)
       end
true

julia> Tar.create("empty", "foo.tar")
"foo.tar"

julia> Tar.has_symlink("foo.tar")
false

xref: JuliaLang/Pkg.jl#3643 (comment)

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6269b5b) 97.28% compared to head (17a81a3) 97.55%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   97.28%   97.55%   +0.26%     
==========================================
  Files           4        4              
  Lines         810      817       +7     
==========================================
+ Hits          788      797       +9     
+ Misses         22       20       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mkitti
Copy link
Member Author

mkitti commented Jan 8, 2024

@StefanKarpinski
Copy link
Member

This feels like it could be implemented externally since it's quite a simple application of Tar.list. In the calling code it would simply be this:

has_symlink = false
Tar.list(tarball) do hdr
    has_symlink |= hdr.type == :symlink
end

If anything, I'd be inclined to add a generic predicate checker like this:

Tar.check_any(predicate::Function, tarball::ArgRead)

That checks if any entry in the tarball satsifies a predicate; it could be specialized with a type::Symbol that would check if any of the entries in the tarball are of that type.

@mkitti
Copy link
Member Author

mkitti commented Jan 8, 2024

How would one write a function without getting into the non-public details of the header structure?

If islink worked, Tar.list(islink, mytar.tar) I would consider it

Also I don't get the need to |, or, the results together. What we actually need is short circuting. Once we found the first symlink, we're done.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 8, 2024

The Header struct is public and documented and part of several APIs, see https://github.com/JuliaIO/Tar.jl#tarheader

@StefanKarpinski
Copy link
Member

We could add short-circuiting as an optimization if we have a Tar.contains function, but I'd consider that an implementation detail.

@mkitti
Copy link
Member Author

mkitti commented Jan 9, 2024

How about we define Base.islink(h::Header) = h.type == :symlink?

@mkitti
Copy link
Member Author

mkitti commented Jan 9, 2024

Here is the API I am imagining.

julia> open(GzipDecompressorStream, "P4est.v2.8.1.x86_64-w64-mingw32-mpi+microsoftmpi.tar.gz") do io
           findfirst(islink, Tar.list(io)) != nothing
       end
true

julia> Tar.create(mkdir("empty"), "foo.tar")
"foo.tar"

julia> findfirst(islink, Tar.list("foo.tar")) != nothing
false

For the short-circuiting what I really want is a lazy iterator over the headers rather than the eager Vector{Header} from Tar.list.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 9, 2024

I've made a PR for the predicates on Header types: https://github.com/JuliaIO/Tar.jl/pull/169/files. Making an iterable object that does what iterate_headers does seems quite annoying. The alternative would be to use the callback interface and raise an exception to terminate early; ugly but would avoid needing an iterator. In the mean time, I think that doing any(islink, Tar.list(tarball)) is fine.

@StefanKarpinski
Copy link
Member

Please also note that none of this is going to help the Pkg situation since I don't think we want to add faetures to Tar in a point release and a point release is what is where this issue is going to be fixed.

@mkitti mkitti closed this Jan 11, 2024
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.

2 participants