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

Adds check for whether directory is actually a bundle before moving children out #85

Closed

Conversation

3sands
Copy link

@3sands 3sands commented Jan 18, 2019

Checks whether post-uncompressed directory is a bundle. If so, do not move the contents out. #84

I don't know if this is an exhaustive list of extensions for bundles that masquerade as directories, but I wanted to put up at least a first pass at a solution that worked for at least some of them.

def isBundle?(entry)
isBundle = File.extname(entry) == '.framework'
isBundle ||= File.extname(entry) == '.app'
isBundle ||= File.extname(entry) == '.framework'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this duplicated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about resource.bundles?

Copy link
Author

@3sands 3sands Jan 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I accidentally duplicated the line. Will correct and commit change. As for the resource.bundles, should cocoapods check just for the .bundles that way it doesn't miss out on non-resource ones?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep any .bundle should be caught by this

Copy link
Member

@amorde amorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a CHANGELOG entry and a unit test for the new function

# @return [Bool] Whether the entry is a bundle
#

def isBundle?(entry)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename to is_bundle? and remove the extra empty line above the function declaration

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can rename the method and remove the line, but I don't know how to write unit tests in ruby.

As for the CHANGELOG entry, I also wasn't sure of the proper git flow for Cocoapods, so I made the PR into master. Should I then put the entry under Master > Bug Fixes?

@segiddins
Copy link
Member

I’m not sure I love this change — seems better to just specify flatten: false?

@amorde
Copy link
Member

amorde commented Jan 18, 2019

is that configurable in the Podspec? I'm not too familiar with this, but if something like this is possible then yes I agree

spec.source = {
    :http => 'some_url',
    :flatten => false
}

@amorde
Copy link
Member

amorde commented Jul 11, 2019

Going to close as the :flatten option is supported, so that can be used instead. Thanks!

@amorde amorde closed this Jul 11, 2019
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.

None yet

4 participants