Issue with pod lib lint when podspec uses prefix_header_* #1514

Closed
wants to merge 3 commits into
from

Projects

None yet

5 participants

@olarivain

Issue with pod lib lint when podspec uses prefix_header_*

Hey,

Just ran into this problem with pod lib lint that fails to properly lint a pod when it defines a prefix_header content/file.
The source of the problem is actually quite simple: lib lint works from /tmp which is actually a symlink to /private/tmp (on at least Mountain Lion and Mavericks, probably true before too). The sandbox is created with a Pathname of "/tmp/Cocoapods/Lint", and there are a few usages of relative_path_from, which assumes that no symlinks exist between the target and the receiver.
That causes the prefix file reference to be short one "../" and Xcode barfs during the final libPod build since it can't reach said prefix file.

I've also created a sample project to help demonstrate/test the problem: https://github.com/opentable/PodLibBug

/kra

@coveralls

Coverage Status

Coverage remained the same when pulling 06e2a73 on opentable:master into 92aa8a2 on CocoaPods:master.

@fabiopelosin
CocoaPods member

Thanks for the contribution. The fix looks reasonable. I will not find the time for properly reviewing it for a couple of days so if in the meanwhile you could add tests to prevent this from happening again it would be really appreciated.

Note: need to document that Xcodeproj doesn't supports symlinks and that this behavior is a design trade off due to the fact that hitting the file system every time a file reference is added, to check for symlinks, is not reasonable from the point of view of performance.

@alloy
CocoaPods member

if in the meanwhile you could add tests to prevent this from happening again it would be really appreciated

👍 We’ll need a test for this in any case. It’s too easy to regress on.

@olarivain Can you also add an entry to the CHANGELOG?

@olarivain

Sure thing, i'll get you guys some tests.

@fabiopelosin
CocoaPods member

Bump

@olarivain

Hey,
sorry for the delay there, I can't seem to make time for this at work, been working on this on my spare time, which isn't much these days.

Anyway, I just realized that this patch isn't enough to address the problem.
If a header in the pod #imports <PodName/SomeHeader.h>, it still breaks the lib lint, for the same reason, the symlink is short one folder.

@olarivain

ok, finally wrapped my head around the tests, I've added unit tests to sandbox and validator's specs.

I see that the Sandbox class has been fixed in origin/master, but it was still missing a couple of symlinks in the validator and in the lint command, so those are there too.

@olarivain

What's the best way to go around adding a sample project to the lib lint tests?
This bug was marked as fixed in the release notes for 0.28, but wasn't really (the validator still used a broken symlink), sounds like we'd want to add a project with a sample #import to fully test it.
I've updated the sample project mentioned above with what breaks, cf opentable/PodLibBug@f9e7f11 in case the description wasn't clear enough

@rivera-ernesto

Just got this same issue. Glad to see a pull request already ;)

@fabiopelosin
CocoaPods member

Looks great could you add a note to the changelog and open a new pull request squashing the commits?

@fabiopelosin
CocoaPods member

What's the best way to go around adding a sample project to the lib lint tests?
This bug was marked as fixed in the release notes for 0.28, but wasn't really (the validator still used a broken symlink), sounds like we'd want to add a project with a sample #import to fully test it.

I don't see any note for 0.28 only one in master added by me which partially implemented a similar fix. I don't think that we should setup a project to test this (however if you are akin to do so you can do it in the https://github.com/CocoaPods/cocoapods-integration-specs repo)

@olarivain

Sure thing. I'm caught up in an uber release, but I'll get around it before the end of the week end for sure.

@olarivain

I've rebased upstream/master onto opentable/master and rewrote the commits to be a little saner.

@fabiopelosin
CocoaPods member

Merged via 0dc6fa9 (I preferred one commit given the size of the patch)! Thanks for the contribution 🍻

Btw, sorry uppercasing kra, I checked in your profile and it was uppercased so I thought that was the proper casing.

@fabiopelosin fabiopelosin referenced this pull request Dec 9, 2013
@olarivain olarivain [Validator] Fix for paths which are symlink
This fixes pod lib lint due to Pathname.relative_path_from assuming no
symlinks in path while /tmp is actually a symlink  to /private/tmp
0dc6fa9
@olarivain

Thanks guys!
And no worries, for the casing, I'm not consistent myself :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment