This is a sample from a tar file class I created for a chef cookbook. In this example I'm iterating over entries in the tar file. Because of the way tar handles filenames over 100 chars (././@LongLink) I have to do something to register that i'm dealing a longlink and treat the next entry differently.
I would love any advice on how to clean this up. Its a super long method, I especially don't like that I have to set full_name to nil and twice.
| def extract_tar | |
| if valid_archive_and_destination?(@archive, @destination) | |
| tar = tar_reader(gzip_reader(@archive)) | |
| full_name = nil | |
| tar.each do |entry| | |
| Chef::Log.debug("Iteration: #{ entry.full_name }") | |
| if longlink?(entry) | |
| full_name = longlink(entry) | |
| next | |
| end | |
| full_name ||= link(entry) | |
| dest = get_destination(@destination, full_name) | |
| Chef::Log.debug("Destination is #{ dest }") | |
| write_file(dest, entry) | |
| full_name = nil | |
| end | |
| else | |
| Chef::Log.info("Archive #{ @archive } is invalid.") | |
| end | |
| end |
Probably naive, but maybe use map?
tar = tar_reader(gzip_reader(@archive))
tar.map! { |fn| longlink?(fn) ? longlink(fn) : fn }
tar.each do |entry|
# ...
end
@miah to make sure I understand..
we can think of tar as a list of name/content tuples. if the name is of type longlink, then the associated content is empty, and the name applies to the content of the next tuple in this list?
[(a:link, foo), (foobar:longlink, nil), (nil, longcontent), (b:link, bar)]
Is there something about variable scope that prevents this?
def extract_tar
if valid_archive_and_destination?(@archive, @destination)
tar = tar_reader(gzip_reader(@archive))
full_name = nil
tar.each do |entry|
Chef::Log.debug("Iteration: #{ entry.full_name }")
if longlink?(entry)
full_name = longlink(entry)
else
full_name ||= link(entry)
dest = get_destination(@destination, full_name)
Chef::Log.debug("Destination is #{ dest }")
write_file(dest, entry)
full_name = nil
end
end
else
Chef::Log.info("Archive #{ @archive } is invalid.")
end
endAlso, it having call the gzip_reader() call seems inconsistent with the method name.
Got any test cases so I can play with this more?
oh, and if @mattupstate's code works, and longlink(fn) returned false iff longlink?(fn), you could be a bit more brief by using
tar.map! { |fn| longlink(fn) || fn }instead of
tar.map! { |fn| longlink?(fn) ? longlink(fn) : fn }
@josephholsten
I agree, gzip_reader shouldn't be in here, just consider that we are working with a IO stream.
I don't really have any tests for this specifically. I have a test-kitchen wrapper that you could run. I have no good excuses =) I'm still learning to flex that muscle.
bundle exec kitchen test
You may want to replace my simple .tar.gz in the files directory with a real java tar.gz. You can see the tests all under the appropriate directory. I can give more details if needed. =)
Yes, that is accurate.
That may work. I don't know how I feel about map! though, can we just assign that output to a new variable?
deleted my comment because I wasn't sure it was correct. The
nexttripped me up -- I would try to make that bit more explicit