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

bugfix. correctly handles filenames with spaces #21

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

algal
Copy link

@algal algal commented Mar 19, 2014

Previously, if a file had a space in its filename and used cocoapods,
then objc-run would fail to build it correctly, as a result of trying to
copy to the unescaped filename, so that it copied only to the first
token of the filename.

This commit fixes that by wrapping the filename in quotes.

Previously, if a file had a space in its filename and used cocoapods,
then objc-run would fail to build it correctly, as a result of trying to
copy to the unescaped filename, so that it copied only to the first
token of the filename.

This commit fixes that by wrapping the filename in quotes.
@iljaiwas
Copy link
Owner

Does the test script still work with your changes? Travis says it's broken.

algal added 2 commits March 20, 2014 11:56
This adds more quoting around file names, to handle
scripts with file names containing spaces that use
cocoapods
@algal
Copy link
Author

algal commented Mar 20, 2014

I've added further bug fixes (which are also just adding missing quotes around paths), and added a new test case to the ./test.bash script that exposes the issue. You can see the test case fails if you try to run it without the other changes to objc-run. The test script works for me, after purging the objc-run temporary directory.

@algal
Copy link
Author

algal commented Mar 20, 2014

I am not sure of it, but I would guess Travis is failing because old files are persisting in the temporary directory between test runs, and corrupting the test.

@iljaiwas
Copy link
Owner

Test script is failing for me locally, too. What version of Xcode are you running? I'm still holding out with 5.0.x.

@algal
Copy link
Author

algal commented Mar 21, 2014

very odd. I'm on 5.1. Did you clear the temporary outputs in /var/folders/stuff/objc-run before running?

@algal
Copy link
Author

algal commented Mar 22, 2014

The Travis error is that it cannot find the temporary directory. So it seems the deeper problem here is the temporary directory needing to be manually cleared between upgrades of this script. Seems like that's an issue of its own.

I suspect you've forgotten more about bash scripting than I've ever learned, so I may not be the right one to fix this but I'll have a look

algal added 3 commits March 23, 2014 10:35
Previously, for scripts using Cocoapods, objc-run built an on-the-fly
project into a temporary project directory keyed only to the filename of
the script.

Now the temporary project directory is keyed to the script filename and
to exact version of objc-run itself.

Thus, objc-run will not use temporary assets that were created by
earlier versions of itself.
Because Apple is shipping ruby 2.0.0p247, which is what
Cocoapods is presumably targetting.
@algal
Copy link
Author

algal commented Mar 23, 2014

Okay, I updated the script so it won't re-use temporary project assets that were created with earlier versions of the script.

However, that doesn't resolve the travis build errors, which I believe are due to the travis build environment using an older versions of Xcode, ruby, and cocoa pods. When travis updates its stuff, or if/when I figure out how to configure it to use more modern build environment, then I'll be a in a position to debug this further, but for now I'm throwing in the towel.

FWIW, my commit 41cbe31 works for me with: Apple's Xcode 5.1, Apple's system ruby v2.0.0p247, and cocoa pods v0.29.0.

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.

2 participants