halp <3

halp.md

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.

Code here

Tar Longlink

tar.rb

      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

@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
end

Also, 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. =)

@mwhooker

Yes, that is accurate.

@mattupstate

That may work. I don't know how I feel about map! though, can we just assign that output to a new variable?