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

Clean paths fix #602

Merged
merged 4 commits into from
Oct 19, 2012
Merged

Clean paths fix #602

merged 4 commits into from
Oct 19, 2012

Conversation

croach
Copy link
Contributor

@croach croach commented Oct 17, 2012

This pull request contains the changes discussed in #572. Specifically, the clean_paths and expand_paths methods both use the File::FNM_CASEFOLD flag in their calls toDir.glob`.

The earlier fix for clean paths is forcing some directories to lower
case which could cause some side effects. This fix removes the
`File::FNM_CASEFOLD` flag from the call to `Dir.glob` and allows the
OS to treat the directories as they wish. This makes the paths
returned in the `clean_paths` method consistent with those returned in
the `expanded_paths` method.
…SEFOLD`

This change is an update to an earlier pull request to fix Issue
CocoaPods#572.
used = used_files.map(&:downcase)
files = Dir.glob(root + "**/*", File::FNM_DOTMATCH).map(&:downcase)
used = used_files
files = Dir.glob(root + "**/*", [File::FNM_DOTMATCH, File::FNM_CASEFOLD])
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be a union of the flags (File::FNM_DOTMATCH | File::FNM_CASEFOLD) and not an array? (See CI details.)

When I apply this change the tests fail for me. Can you confirm if that change still works for you?

Copy link
Member

Choose a reason for hiding this comment

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

A simple illustration of the aforementioned bug:

~/C/C/CocoaPods [croach-clean-paths-fix] » irb
>> Dir.glob('foo', [File::FNM_DOTMATCH, File::FNM_CASEFOLD])
TypeError: can't convert Array into Integer
    from (irb):1:in `glob'
    from (irb):1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry about that. I don't really use Ruby and an example I had found showed an array being passed in for the flags argument. Seemed kind of weird to me at the time, most other languages tend to do a union of all the flags, but when I tested it locally, it worked. I guess I didn't get it installed properly cause I tried it again with a fresh install and I do get an error now. I'll update the pull request. Thanks for pointing that out, sorry for taking up so much of your guys' time with this.

@alloy
Copy link
Member

alloy commented Oct 18, 2012

Hmm, from your public activity feed I can see you replied to my comment on your PR, however I can’t seem to find it in full anywhere. Can you repost it here?

@alloy
Copy link
Member

alloy commented Oct 18, 2012

Ok, so the only real difference seems OS X 10.7 vs 10.8, right? I will have to see if I can setup a VM with that version to try to replicate the issue. Can you give me details on what Ruby version you are using?

@fabiopelosin
Copy link
Member

@croach Thanks for helping with this issue.

There are still some changes that need to be done (using Pathname.glob and revert the changes that I made to some specs with my previous commit), however as we need to release release CocoaPods 0.15.2 I'm merging this and I'll commit on top your contribution.

It would be great if you could check that the issue is actually resolved once the new version is out, because I still can't reproduce the issue.

fabiopelosin added a commit that referenced this pull request Oct 19, 2012
@fabiopelosin fabiopelosin merged commit 22fbc3d into CocoaPods:master Oct 19, 2012
fabiopelosin added a commit that referenced this pull request Oct 19, 2012
This patch also removes the custom Pathname.glob based on Dir::glob.

Related #572 and #602.
@croach
Copy link
Contributor Author

croach commented Oct 20, 2012

@alloy I'm using rbenv to manage my ruby installs and I'm using ruby v1.9.3p194.

@irrationalfab Thanks working through this issue with me. I just tried out the latest version (v0.15.2) of cocoapods and it worked perfectly. I think we got the problem fixed!

@fabiopelosin
Copy link
Member

@croach 😂

jzapater pushed a commit to jzapater/CocoaPods that referenced this pull request Sep 17, 2013
fabiopelosin added a commit that referenced this pull request Oct 25, 2014
fabiopelosin added a commit that referenced this pull request Oct 25, 2014
This patch also removes the custom Pathname.glob based on Dir::glob.

Related #572 and #602.
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.

3 participants