Skip to content

Unpack tgz files into pod's root #727 #728

Merged
merged 1 commit into from Jan 13, 2013

3 participants

@nevyn
nevyn commented Jan 9, 2013

Zip files are usually packaged with their contents in the archive file's root.
Tgz files are usually packaged with only a folder in the root.
Make them both behave the same by moving that single folder's contents up.
This is basically the opposite of what Finder does (which creates a folder for the zip file's
contents, so you always unarchive into a folder).

@alloy alloy and 1 other commented on an outdated diff Jan 9, 2013
lib/cocoapods/downloader/http.rb
@@ -73,6 +73,15 @@ def extract_with_type(full_filename, type=:zip)
else
raise UnsupportedFileTypeError.new "Unsupported file type: #{type}"
end
+
+ # If the archive only contained a folder, move its contents to the target
+ contents = Dir[target_path.to_s+"/*"]
+ puts contents.delete(full_filename.to_s)
@alloy
CocoaPods member
alloy added a note Jan 9, 2013

I guess this was for debugging only?

@nevyn
nevyn added a note Jan 9, 2013

sigh :( I'll fix that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@alloy
CocoaPods member
alloy commented Jan 9, 2013

Awesome, thanks!

However, this needs a test case. Is that something you’re comfortable with?

@nevyn
nevyn commented Jan 9, 2013

Oh right! Sure, I'm on it.

@nevyn nevyn Unpack tgz files into pod's root #727
Zip files are usually packaged with their contents in the archive file's root.
Tgz files are usually packaged with only a folder in the root.
Make them both behave the same by moving that single folder's contents up.
This is basically the opposite of what Finder does (which creates a folder for the zip file's
contents, so you always unarchive into a folder).
fef5dad
@nevyn
nevyn commented Jan 9, 2013

There. Never written a unit test in ruby, shout if I did it wrong. Also removed the debug print. Did a force push to my issue727 branch, is that how you are supposed to update a pull request?

@nevyn nevyn referenced this pull request in CocoaPods/Specs Jan 9, 2013
Closed

New pods: OpenSSL and CHMath #999

@alloy
CocoaPods member
alloy commented Jan 10, 2013

It looks good. I’ll try to get a bug fix release out in the next couple of days.

@alloy alloy merged commit fef5dad into CocoaPods:master Jan 13, 2013

1 check failed

Details default The Travis build failed
@alloy
CocoaPods member
alloy commented Jan 13, 2013

Also applied to the extraction of the downloaders for the next version: CocoaPods/cocoapods-downloader@f5fc744

@kbok
kbok commented on fef5dad Feb 4, 2013

This is breaking the BugSense pod. De we really need this? I'd rather have more directories and keep the behaviour consistent. Can we make this an option or something?

CocoaPods member
alloy replied Feb 4, 2013

@kbok This commit is in fact making tar balls (any but zipped) work consistent with the other downloaders. Please update the BugSense pod spec for this change.

kbok replied Feb 4, 2013

This is actually removing the first path component also for zip archives, provided they contain only one folder at their root. What about enabling this only for tarballs? I know I could update the BugSense spec, but it's unnatural because the folder is part of the archive, and we don't know which other specs are broken by this change.

CocoaPods member
alloy replied Feb 4, 2013

@kbok Oh, now I get the problem. You have a zip archive of the BugSense lib that contains a directory only, and this is completely valid because the directory is a framework bundle. Did I sum it up correctly?

In this case, seeing that the unzip tool did not have the ‘issue’ anyways, we should probably indeed not do this for zip archives.

/cc @nevyn

kbok replied Feb 5, 2013

Yes indeed :) I'll submit a PR for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.