pinterest / PINCache

Fast, non-deadlocking parallel object cache for iOS, tvOS and OS X
Apache License 2.0
2.66k stars 361 forks source link

Fix build issues by removing nested PINOperation.xcodeproj #282

Closed elliottwilliams closed 3 years ago

elliottwilliams commented 4 years ago

Hello! Here's a quick fix for Carthage users:

This change replaces the PINOperation.xcodeproj nested inside PINCache.xcodeproj with a workspace containing both projects. It doesn't impact building of PINCache itself, and it fixes a duplicate target error users will get if they try to build both projects within Xcode.

Background

Currently, PINCache checks Carthage/Checkouts/PINOperation into git, but it also lists PINOperation in its Cartfile. As a result, Carthage users who depend on PINCache will get two distinct copies of PINOperation checked out:

Carthage/Checkouts/PINOperation                             
Carthage/Checkouts/PINCache/Carthage/Checkouts/PINOperation

PINCache.xcodeproj embeds the second PINOperation and has target dependencies on it. This causes a couple issues:

  1. If you build using carthage build, PINOperation compiles twice: once when building PINOperation (the carthage dependency), and once when building PINCache.

  2. If you add PINOperation and PINCache's xcodeprojs to a workspace and try to build from there, you get a build error because Xcode has loaded both PINOperation projects, and therefore sees duplicate targets:

    error: Multiple commands produce 'Build/Products/Debug-iphonesimulator/PINOperation.framework/PINOperation':
    1) Target 'PINOperation' has link command with output 'Build/Products/Debug-iphonesimulator/PINOperation.framework/PINOperation'
    2) Target 'PINOperation' has link command with output 'Build/Products/Debug-iphonesimulator/PINOperation.framework/PINOperation'

Alternatives

Instead of creating an xcworkspace, I think this could also be fixed by:

Testing done

elliottwilliams commented 4 years ago

So…turns out the first approach (using Cartfile.private) doesn't actually work when integrating PINCache in another project 😛

Updated to move the embedded PINOperation.xcodeproj to a workspace, and confirmed that this fixes the build errors I was seeing downstream.

garrettmoon commented 3 years ago

Thanks for putting this up! I think the Makefile analyze command needs to be updated? Possibly others?

garrettmoon commented 3 years ago

Hmm, looks like this is still failing even after updating the Makefile. I think we'll have to figure out why it's failing before this can be merged.

elliottwilliams commented 3 years ago

@garrettmoon thanks & good catch! I guess what I didn't realize was that PINCache didn't link against PINOperation (so Xcode didn't see an implicit dependency relationship between them). I've gone ahead and made PINCache link (but not embed) PINOperation. The Make commands are succeeding for me now, and I've verified in a downstream project that PINCache still builds :)

Worth noting here that linking here does change the PINCache binary by indicating that it depends on PINOperation.framework. But this change shouldn't be meaningful, since PINCache will always fail to load at runtime if PINOperation is missing.