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

Support for Frameworks / Swift #2835

Merged
merged 176 commits into from Dec 25, 2014

Conversation

Projects
None yet
@mrackwitz
Member

mrackwitz commented Nov 16, 2014

Support Requests

Please make issues on this repo: https://github.com/CocoaPods/swift

Pull Request

Please make support requests around Swift CocoaPods on this repo: https://github.com/CocoaPods/swift

This Pull-Request makes first steps to bring the long-awaited Swift support by supporting Frameworks. Along the way it keeps full backwards-compatibility with CocoaPods 0.35 and tries to break as few stuff as needed.
This PR supersedes #2222. See this for some of the original discussions.

Steps

To make that all happen, the following path was taken, described here very roughly:

  • Recognize Swift source files by file accessors
  • Extend the analyzer to decide when it is required to build a framework
  • Generate a framework product type if required
  • Generate a custom umbrella header to import all public headers and avoid naming conflicts
  • Generate a custom module map to inject the umbrella header
  • Namespace frameworks in directories in build path, as their naming is constrained
    by underlying clang modules
  • Migrate from a linked aggregate library to a linked framework (which is still just used instead of a target dependency to indirectly trigger the build of all pod targets)
  • Link the built pod frameworks by build settings to the user target
  • Set the build settings appropriate to reach all that
  • Embed the built pod frameworks with a shell script build phase to the host application to allow configuration-dependent pods
  • Auto opt-in OS X targets and iOS 8+ targets to dynamic frameworks (to be discussed in a separate issue, at the moment possibly more helpful for feedback)
What has happened so far:

All the steps described above. See the commits below. That's already a very condensed history, which was the result of a lot of trial-and-error and has been squashed to a reasonable length.

Checkout:

To get as much feedback as possible, feel encouraged to check out this branch in your local clone of Rainforest. If you use bundler's local gem feature, make sure that you checkout the right branches on the linked repos. (See below Dependencies for reference.) Make sure to run bundle install. You may want to make sure, that you install it to a gemset, you can use for the projects, you want to develop with. Right after that you should be able to install the CocoaPods gem with rake install to your current gemset.

If you use it and you mention a serve problem, like a crash appears or your xcodebuild fails:

  1. Update your local copy. This crash could been already fixed. Please make sure to get a clean checkout from this branch as the upstream could have changed due to rebases, which could cause undesired local merges / conflicts.
  2. Search if someone has already created an issue for your problem, if so please take a look and provide more helpful information. If that's not possible, please don't +1, if not explicitly asked for it. We will try to solve all problems anyway.
  3. If there is no issue, please create a new issue, after reading our contribution guidelines. Prefix it with [Frameworks] and provide a reference to this Pull-Request on your newly created issue by referencing its number #2835.
Open Todos:

The following things are still open to do:

  • Add an example with Swift code
  • Add a fixture of SwiftPod for unit tests
  • Build podspecs for some OSS projects and test pod spec lint
  • Rebase after the release of 0.35.0
  • Add missing unit tests
  • Add dedicated integration tests
  • More tests & experience with real-life projects
  • Add a CHANGELOG entry
Dependencies:

This depends on the following PRs:

Related issues:

/c @CocoaPods/core

@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Nov 16, 2014

Member

I think a lot of people will want to do thanks and +1s etc, please move that to twitter and show @mrackwitz some love.

In the meantime, I've tried installing this locally using Bundler. I've extended my Gemfile in the eidolon project to include the following:

gem 'cocoapods', :git => 'https://github.com/CocoaPods/CocoaPods.git', :branch => 'swift'
gem 'cocoapods-core', :git => 'https://github.com/CocoaPods/Core.git'
gem 'xcodeproj',  :git => 'https://github.com/CocoaPods/Xcodeproj.git'
gem 'claide',  :git => 'https://github.com/CocoaPods/CLAide.git'

Running with this on eidolon and doing a pod install with only Obj-C deps for now results in this integration stage error:

NoMethodError - undefined method `display_name' for nil:NilClass
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:214:in `block (2 levels) in native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `each'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `any?'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `block in native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:212:in `select'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:212:in `native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:34:in `block in integrate!'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/user_interface.rb:49:in `section'
Member

orta commented Nov 16, 2014

I think a lot of people will want to do thanks and +1s etc, please move that to twitter and show @mrackwitz some love.

In the meantime, I've tried installing this locally using Bundler. I've extended my Gemfile in the eidolon project to include the following:

gem 'cocoapods', :git => 'https://github.com/CocoaPods/CocoaPods.git', :branch => 'swift'
gem 'cocoapods-core', :git => 'https://github.com/CocoaPods/Core.git'
gem 'xcodeproj',  :git => 'https://github.com/CocoaPods/Xcodeproj.git'
gem 'claide',  :git => 'https://github.com/CocoaPods/CLAide.git'

Running with this on eidolon and doing a pod install with only Obj-C deps for now results in this integration stage error:

NoMethodError - undefined method `display_name' for nil:NilClass
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:214:in `block (2 levels) in native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `each'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `any?'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:213:in `block in native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:212:in `select'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:212:in `native_targets_to_integrate'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/installer/user_project_integrator/target_integrator.rb:34:in `block in integrate!'
/usr/local/var/rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/bundler/gems/CocoaPods-eaef33b4f672/lib/cocoapods/user_interface.rb:49:in `section'
@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Nov 16, 2014

Member

The old libPods.a and their brethren are not removed if you migrate to frameworks.

Member

orta commented Nov 16, 2014

The old libPods.a and their brethren are not removed if you migrate to frameworks.

@mrackwitz mrackwitz referenced this pull request Nov 16, 2014

Merged

Podspec #218

@mrackwitz

This comment has been minimized.

Show comment
Hide comment
@mrackwitz

mrackwitz Nov 16, 2014

Member

I'm finding that I have to change things like #import <Artsy+UIColors/UIColor+ArtsyColors.h> to #import <Artsy_UIColors/UIColor+ArtsyColors.h>. Is there a strong reason for only supporting _ in framework names?

Yes, there is. Framework base names (determined by PRODUCT_NAME) must be equal to their underlying PRODUCT_MODULE_NAME, which may only be a C99ext identifier. At least Xcode claims that.

Member

mrackwitz commented Nov 16, 2014

I'm finding that I have to change things like #import <Artsy+UIColors/UIColor+ArtsyColors.h> to #import <Artsy_UIColors/UIColor+ArtsyColors.h>. Is there a strong reason for only supporting _ in framework names?

Yes, there is. Framework base names (determined by PRODUCT_NAME) must be equal to their underlying PRODUCT_MODULE_NAME, which may only be a C99ext identifier. At least Xcode claims that.

@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Nov 16, 2014

Member

screen shot 2014-11-16 at 1 05 09 pm

Not a specific problem with CocoaPods, but some Swift code will crash the compiler if it has any SWIFT_OPTIMIZATION_LEVEL is not set to "None". We may want to figure a way to make changing that easy. Or it has to go in specific podspecs.

( Offtopic, but this is one reason that the Eidolon app has to ship entirely unoptimized )

Member

orta commented Nov 16, 2014

screen shot 2014-11-16 at 1 05 09 pm

Not a specific problem with CocoaPods, but some Swift code will crash the compiler if it has any SWIFT_OPTIMIZATION_LEVEL is not set to "None". We may want to figure a way to make changing that easy. Or it has to go in specific podspecs.

( Offtopic, but this is one reason that the Eidolon app has to ship entirely unoptimized )

Show outdated Hide outdated lib/cocoapods/generator/embed_frameworks_script.rb
module Generator
class EmbedFrameworksScript
# @return [TargetDefinition] The target definition, whose label will be
# used to locate the target specific build products.

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

target-specific

@segiddins

segiddins Nov 17, 2014

Member

target-specific

Show outdated Hide outdated lib/cocoapods/generator/header.rb
@@ -52,6 +64,7 @@ def generate
# @return [void]
#
def save_as(path)
FileUtils.mkdir_p(path + '..')

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

whatever calls this should be necessary for making sure the containing directory exists

@segiddins

segiddins Nov 17, 2014

Member

whatever calls this should be necessary for making sure the containing directory exists

This comment has been minimized.

Show outdated Hide outdated lib/cocoapods/generator/info_plist_file.rb
def save_as(path)
contents = generate
File.open(path, 'w') do |f|
f.write(contents)

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

just f.write(generate)?

@segiddins

segiddins Nov 17, 2014

Member

just f.write(generate)?

This comment has been minimized.

@kylef

kylef Nov 17, 2014

Contributor

Eh, instead what about:

File.write(path, generate)
@kylef

kylef Nov 17, 2014

Contributor

Eh, instead what about:

File.write(path, generate)

This comment has been minimized.

@mrackwitz

mrackwitz Nov 30, 2014

Member

@kylef: Both neither what is present nor what you propose is what we now use consequently through all generators:

path.open('w') do |file|
  file.puts(generate)
end
@mrackwitz

mrackwitz Nov 30, 2014

Member

@kylef: Both neither what is present nor what you propose is what we now use consequently through all generators:

path.open('w') do |file|
  file.puts(generate)
end
<plist version="1.0">
<dict>
<key>CFBundleDevelopmentRegion</key>
<string>en</string>

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

shouldn't this be based upon the project settings or something?

@segiddins

segiddins Nov 17, 2014

Member

shouldn't this be based upon the project settings or something?

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

At the moment, the resources are not bundled into the frameworks, but still integrated with the Copy Pods Resources shell script build phase. It would perhaps make sense to set that for the pod target, as soon as we do #2730. I don't see any value to set this for the aggregate target as this isn't included anyway.

@mrackwitz

mrackwitz Nov 17, 2014

Member

At the moment, the resources are not bundled into the frameworks, but still integrated with the Copy Pods Resources shell script build phase. It would perhaps make sense to set that for the pod target, as soon as we do #2730. I don't see any value to set this for the aggregate target as this isn't included anyway.

This comment has been minimized.

@segiddins

segiddins Dec 11, 2014

Member

@neonichu has now made them bundled, right? So don't we need to address this?

@segiddins

segiddins Dec 11, 2014

Member

@neonichu has now made them bundled, right? So don't we need to address this?

This comment has been minimized.

This comment has been minimized.

@mrackwitz

mrackwitz Dec 24, 2014

Member

@segiddins: where do you want to take the value from? The podspecs don't specify any development region. Would you take the value from the user project's development region? It will then probably only be changed on each pod install. Furthermore I can't construct a case, where this would be really helpful.

@mrackwitz

mrackwitz Dec 24, 2014

Member

@segiddins: where do you want to take the value from? The podspecs don't specify any development region. Would you take the value from the user project's development region? It will then probably only be changed on each pod install. Furthermore I can't construct a case, where this would be really helpful.

Show outdated Hide outdated lib/cocoapods/generator/module_map.rb
@@ -0,0 +1,50 @@
module Pod
module Generator
# Generates the LLVM module map files. A module map file is generated for

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

the

Show outdated Hide outdated lib/cocoapods/generator/module_map.rb
module Pod
module Generator
# Generates the LLVM module map files. A module map file is generated for
# each Pod and for each Pod target definition, that requires to be built as

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

that is built as a framework

@segiddins

segiddins Nov 17, 2014

Member

that is built as a framework

framework module #{target.product_module_name} {
umbrella header "#{target.umbrella_header_path.basename}"
export *

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

shouldn't private_header_files not be exported?

@segiddins

segiddins Nov 17, 2014

Member

shouldn't private_header_files not be exported?

This comment has been minimized.

@mrackwitz

mrackwitz Nov 29, 2014

Member

Just saw that this question stayed unanswered. We don't need to exclude them explicitly, because we only import public headers in the generated umbrella header and the inferred-submodule-member export * respects that.

Clang Documentation ⟩ Module Map Language ⟩ Submodule Declaration:
The headers to which the inferred-submodule-declaration applies are exactly those headers included by the umbrella header (transitively) […]

But not caring at all about private headers is admittetly in conflict with:

Any headers not included by the umbrella header should have explicit header declarations.

We could care about that, and validate that everyone is doing everything right all the time by doing the following:

  1. Export all private headers explicitly as private private header "MyPrivateHeader.h".
  2. Providing -Wincomplete-umbrella warning option to ask Clang to complain about headers not covered by the umbrella header or the module map.
@mrackwitz

mrackwitz Nov 29, 2014

Member

Just saw that this question stayed unanswered. We don't need to exclude them explicitly, because we only import public headers in the generated umbrella header and the inferred-submodule-member export * respects that.

Clang Documentation ⟩ Module Map Language ⟩ Submodule Declaration:
The headers to which the inferred-submodule-declaration applies are exactly those headers included by the umbrella header (transitively) […]

But not caring at all about private headers is admittetly in conflict with:

Any headers not included by the umbrella header should have explicit header declarations.

We could care about that, and validate that everyone is doing everything right all the time by doing the following:

  1. Export all private headers explicitly as private private header "MyPrivateHeader.h".
  2. Providing -Wincomplete-umbrella warning option to ask Clang to complain about headers not covered by the umbrella header or the module map.

This comment has been minimized.

@segiddins

segiddins Dec 23, 2014

Member

@mrackwitz I think we should do that when linting.

@segiddins

segiddins Dec 23, 2014

Member

@mrackwitz I think we should do that when linting.

This comment has been minimized.

@mrackwitz

mrackwitz Dec 24, 2014

Member

@segiddins: That would be perhaps an interesting enhancement. But that's out-of-scope of this PR. Do you want to file an issue for that?

@mrackwitz

mrackwitz Dec 24, 2014

Member

@segiddins: That would be perhaps an interesting enhancement. But that's out-of-scope of this PR. Do you want to file an issue for that?

Show outdated Hide outdated lib/cocoapods/generator/xcconfig/aggregate_xcconfig.rb
pod_targets.each do |pod_target|
XCConfigHelper.add_settings_for_file_accessors_of_target(pod_target, @xcconfig)
# Add pod framework to list of frameworks / libraries that are

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

s/pod framework/pod target/

@segiddins

segiddins Nov 17, 2014

Member

s/pod framework/pod target/

Show outdated Hide outdated lib/cocoapods/generator/xcconfig/private_pod_xcconfig.rb
dependencies = target.module_dependencies.reject { |dep| dep == target.product_module_name }
build_settings = {
'PODS_FRAMEWORK_BUILD_PATH' => target.configuration_build_dir,
'CONFIGURATION_BUILD_DIR' => '$PODS_FRAMEWORK_BUILD_PATH',

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

shouldn't all of these be quoted?

@segiddins

segiddins Nov 17, 2014

Member

shouldn't all of these be quoted?

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

Nope, CONFIGURATION_BUILD_DIR may not be quoted. Otherwise it will be interpreted as relative. (e.g. /Users/marius/Development/eidolon/Pods/"/Users/marius/Development/eidolon/DerivedData/…) FRAMEWORK_SEARCH_PATHS is pluralized and must be quoted to distinguish a single path with spaces from multiple enumerated paths.

@mrackwitz

mrackwitz Nov 17, 2014

Member

Nope, CONFIGURATION_BUILD_DIR may not be quoted. Otherwise it will be interpreted as relative. (e.g. /Users/marius/Development/eidolon/Pods/"/Users/marius/Development/eidolon/DerivedData/…) FRAMEWORK_SEARCH_PATHS is pluralized and must be quoted to distinguish a single path with spaces from multiple enumerated paths.

#
def self.add_code_signing_settings(target, xcconfig)
build_settings = {}
if target.platform.to_sym == :osx

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

why only for OS X?

@segiddins

segiddins Nov 17, 2014

Member

why only for OS X?

This comment has been minimized.

@flovilmart

flovilmart Nov 20, 2014

Should be for iOS As well, the app can't be distributed otherwise (had the problem today)

@flovilmart

flovilmart Nov 20, 2014

Should be for iOS As well, the app can't be distributed otherwise (had the problem today)

This comment has been minimized.

@mrackwitz

mrackwitz Nov 20, 2014

Member

Generally, I would say code-sign every single framework is undesired, if not needed. But Xcode claims that it is required for iOS frameworks, not on build of the whole application target, but already when building the single framework. At least with 6.1, didn't tested that specific detail with the beta so far.

@mrackwitz

mrackwitz Nov 20, 2014

Member

Generally, I would say code-sign every single framework is undesired, if not needed. But Xcode claims that it is required for iOS frameworks, not on build of the whole application target, but already when building the single framework. At least with 6.1, didn't tested that specific detail with the beta so far.

This comment has been minimized.

@flovilmart

flovilmart Nov 21, 2014

When you 'embed' projects (like Alamofire) through the xcodeproj, there's this build phase 'Copy Frameworks that have 'CodeSign on Copy' flagged on. Why not do like Xcode and sign at copy time afte the rsync? That seems to cause issues at distribution time, not being able to install the app on the target device through IPA

@flovilmart

flovilmart Nov 21, 2014

When you 'embed' projects (like Alamofire) through the xcodeproj, there's this build phase 'Copy Frameworks that have 'CodeSign on Copy' flagged on. Why not do like Xcode and sign at copy time afte the rsync? That seems to cause issues at distribution time, not being able to install the app on the target device through IPA

This comment has been minimized.

@AliSoftware

AliSoftware Nov 23, 2014

Contributor

Be careful, CodeSigning is not always wanted, for example on our CI server we build & test using the -sdk iphonesimulator and we dont CodeSign, to be sure we won't have CodeSigning issue just because our CI don't have all the Provisioning Profiles installed.

For example one of our latest app uses HealthKit, which needs specific entitlements, leading to a Provisioning Profile specific for this app. We don't want to have to copy/install the Provisioning Profile on our CI each time an app has a specific PP needed, that's why we prefer our CI not to CodeSign.

Making CP force the CodeSigning of the Swift frameworks it integrates could this lead to problems. We should only CodeSign if the target SDK is iphoneos (or OS X), and not iphonesimulator. Better yet, we should use the same settings for the Swift frameworks that is used for the app.

@AliSoftware

AliSoftware Nov 23, 2014

Contributor

Be careful, CodeSigning is not always wanted, for example on our CI server we build & test using the -sdk iphonesimulator and we dont CodeSign, to be sure we won't have CodeSigning issue just because our CI don't have all the Provisioning Profiles installed.

For example one of our latest app uses HealthKit, which needs specific entitlements, leading to a Provisioning Profile specific for this app. We don't want to have to copy/install the Provisioning Profile on our CI each time an app has a specific PP needed, that's why we prefer our CI not to CodeSign.

Making CP force the CodeSigning of the Swift frameworks it integrates could this lead to problems. We should only CodeSign if the target SDK is iphoneos (or OS X), and not iphonesimulator. Better yet, we should use the same settings for the Swift frameworks that is used for the app.

This comment has been minimized.

@flovilmart

flovilmart Nov 24, 2014

Ok +1 for you.

Maybe just using a copy files phase instead of a script would solve the issue?

On Mon, Nov 24, 2014 at 5:10 PM, Samuel E. Giddins
notifications@github.com wrote:

@@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
xcconfig.merge!(build_settings)
end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    The issue with using target dependencies for that is that it doesn't allow scoping dependencies to specific build configurations
    -Samuel E. Giddins
    On Nov 24, 2014, at 2:02 PM, Florent Vilmart notifications@github.com wrote:
    In lib/cocoapods/generator/xcconfig/xcconfig_helper.rb:
    @@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
    xcconfig.merge!(build_settings)
    end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    OK, I start thinking the workspace architecture is getting in the way of simplicity and reusability.
    Maybe the following changes would make sense:

  • Add each pod as a Target dependency item
    1.a Add the pods project a a subproject (instead of a project in the same workspace)

  • Instead of a Copy Pods script phase, replace with a 'Copy File' Build phase with codesign on and each pod target
    What do you think?
    That would actually make a lot of sense, instead of circumventing/R. engineer what Xcode's doing behind the scenes, just embrace the Xcode build strategy

    Reply to this email directly or view it on GitHub.

    Reply to this email directly or view it on GitHub:
    https://github.com/CocoaPods/CocoaPods/pull/2835/files#r20826608

@flovilmart

flovilmart Nov 24, 2014

Ok +1 for you.

Maybe just using a copy files phase instead of a script would solve the issue?

On Mon, Nov 24, 2014 at 5:10 PM, Samuel E. Giddins
notifications@github.com wrote:

@@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
xcconfig.merge!(build_settings)
end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    The issue with using target dependencies for that is that it doesn't allow scoping dependencies to specific build configurations
    -Samuel E. Giddins
    On Nov 24, 2014, at 2:02 PM, Florent Vilmart notifications@github.com wrote:
    In lib/cocoapods/generator/xcconfig/xcconfig_helper.rb:
    @@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
    xcconfig.merge!(build_settings)
    end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    OK, I start thinking the workspace architecture is getting in the way of simplicity and reusability.
    Maybe the following changes would make sense:

  • Add each pod as a Target dependency item
    1.a Add the pods project a a subproject (instead of a project in the same workspace)

  • Instead of a Copy Pods script phase, replace with a 'Copy File' Build phase with codesign on and each pod target
    What do you think?
    That would actually make a lot of sense, instead of circumventing/R. engineer what Xcode's doing behind the scenes, just embrace the Xcode build strategy

    Reply to this email directly or view it on GitHub.

    Reply to this email directly or view it on GitHub:
    https://github.com/CocoaPods/CocoaPods/pull/2835/files#r20826608

This comment has been minimized.

@mrackwitz

mrackwitz Nov 24, 2014

Member

@flovilmart: The way you drafted, is what I tried first. The same issue with scoping exists for Copy Files Phases. Implicit target dependencies can be resolved by Xcode, that shouldn't be a problem anyway.
There is even one more issue with that: We regenerate the targets in the Pods project on every pod install run. This causes changes to the user projects, which are likely undeseriable and can cause duplicate proxy entries, if they aren't garbage collected.

@mrackwitz

mrackwitz Nov 24, 2014

Member

@flovilmart: The way you drafted, is what I tried first. The same issue with scoping exists for Copy Files Phases. Implicit target dependencies can be resolved by Xcode, that shouldn't be a problem anyway.
There is even one more issue with that: We regenerate the targets in the Pods project on every pod install run. This causes changes to the user projects, which are likely undeseriable and can cause duplicate proxy entries, if they aren't garbage collected.

This comment has been minimized.

@flovilmart

flovilmart Nov 24, 2014

In that case, we could use a particularly name build phase like [Pods] Copy Frameworks so it’s easy to remove/readd on each install…

Any Idea why it would symlink the .framework instead of copy upon archive?

On Mon, Nov 24, 2014 at 5:23 PM, Marius Rackwitz notifications@github.com
wrote:

@@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
xcconfig.merge!(build_settings)
end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    @flovilmart: The way you drafted, is what I tried first. The same issue with scoping exists for Copy Files Phases. Implicit target dependencies can be resolved by Xcode, that shouldn't be a problem anyway.
    There is even one more issue with that: We regenerate the targets in the Pods project on every pod install run. This causes changes to the user projects, which are likely undeseriable and can cause duplicate proxy entries, if they aren't garbage collected.

    Reply to this email directly or view it on GitHub:
    https://github.com/CocoaPods/CocoaPods/pull/2835/files#r20827496
@flovilmart

flovilmart Nov 24, 2014

In that case, we could use a particularly name build phase like [Pods] Copy Frameworks so it’s easy to remove/readd on each install…

Any Idea why it would symlink the .framework instead of copy upon archive?

On Mon, Nov 24, 2014 at 5:23 PM, Marius Rackwitz notifications@github.com
wrote:

@@ -88,6 +114,58 @@ def self.add_library_build_settings(library_path, xcconfig, sandbox_root)
xcconfig.merge!(build_settings)
end

  •    # Add the code signing settings for generated targets to ensure that
    
  •    # frameworks are correctly signed to be integrated and re-signed when
    
  •    # building the application and embedding the framework
    
  •    #
    
  •    # @param  [Target] target
    
  •    #         The target.
    
  •    #
    
  •    # @param  [Xcodeproj::Config] xcconfig
    
  •    #         The xcconfig to edit.
    
  •    #
    
  •    def self.add_code_signing_settings(target, xcconfig)
    
  •      build_settings = {}
    
  •      if target.platform.to_sym == :osx
    

    @flovilmart: The way you drafted, is what I tried first. The same issue with scoping exists for Copy Files Phases. Implicit target dependencies can be resolved by Xcode, that shouldn't be a problem anyway.
    There is even one more issue with that: We regenerate the targets in the Pods project on every pod install run. This causes changes to the user projects, which are likely undeseriable and can cause duplicate proxy entries, if they aren't garbage collected.

    Reply to this email directly or view it on GitHub:
    https://github.com/CocoaPods/CocoaPods/pull/2835/files#r20827496

This comment has been minimized.

This comment has been minimized.

@mrackwitz

mrackwitz Dec 11, 2014

Member

@segiddins: regarding

Any Idea why it would symlink the .framework instead of copy upon archive?

This was a bug, which @flovilmart fixed in the embed resources script.
So this is done.

@mrackwitz

mrackwitz Dec 11, 2014

Member

@segiddins: regarding

Any Idea why it would symlink the .framework instead of copy upon archive?

This was a bug, which @flovilmart fixed in the embed resources script.
So this is done.

Show outdated Hide outdated lib/cocoapods/installer/target_installer/pod_target_installer.rb
create_info_plist_file
create_module_map
create_umbrella_header do |generator|
generator.imports += target.file_accessors.map(&:public_headers).flatten.map(&:basename)

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

target.file_accessors.flat_map(&:public_headers).map(&:basename)

@segiddins

segiddins Nov 17, 2014

Member

target.file_accessors.flat_map(&:public_headers).map(&:basename)

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

👍

Show outdated Hide outdated lib/cocoapods/installer/user_project_integrator.rb
@@ -132,6 +132,7 @@ def warn_about_empty_podfile
end
end
IGNORED_KEYS = %w(CODE_SIGN_IDENTITY)

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

.freeze?

@segiddins

segiddins Nov 17, 2014

Member

.freeze?

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

👍

Show outdated Hide outdated lib/cocoapods/installer/user_project_integrator/target_integrator.rb
end
if target.requires_framework?
build_file.settings ||= {}
build_file.settings['ATTRIBUTES'] = ['Weak']

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

why does this need to be a weak framework?

@segiddins

segiddins Nov 17, 2014

Member

why does this need to be a weak framework?

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

Otherwise it need to be embedded and will be eagerly linked by dyld, but as it is empty and not needed at all as an built product, we can circumvent that by weak linking it. See the commit message.

@mrackwitz

mrackwitz Nov 17, 2014

Member

Otherwise it need to be embedded and will be eagerly linked by dyld, but as it is empty and not needed at all as an built product, we can circumvent that by weak linking it. See the commit message.

This comment has been minimized.

@segiddins

segiddins Nov 21, 2014

Member

Show outdated Hide outdated lib/cocoapods/installer/user_project_integrator/target_integrator.rb
def add_embed_frameworks_script_phase
phase_name = 'Embed Pods Frameworks'
native_targets_to_integrate.each do |native_target|
embed_build_phase = native_target.shell_script_build_phases.select { |bp| bp.name == phase_name }.first

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

use .find there

@segiddins

segiddins Nov 17, 2014

Member

use .find there

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

👍

Show outdated Hide outdated lib/cocoapods/sandbox/headers_store.rb
@@ -26,6 +26,10 @@ def initialize(sandbox, relative_path)
@search_paths = []
end
# @param [Platform] platform
# indicate for which platform the header search paths should be

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

the platform for which the header search paths should be returned

@segiddins

segiddins Nov 17, 2014

Member

the platform for which the header search paths should be returned

Show outdated Hide outdated lib/cocoapods/target.rb
# @return [Boolean] Whether the target needs to be implemented as a framework.
# Computed by analyzer.
#
attr_accessor :host_requires_framework

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

why not make this accessor plural, i.e. host_requires_frameworks?

@segiddins

segiddins Nov 17, 2014

Member

why not make this accessor plural, i.e. host_requires_frameworks?

Show outdated Hide outdated lib/cocoapods/target.rb
# dependents, which can only be checked after the specs were been
# fetched.
#
def requires_framework?

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

requires_frameworks? ?

@segiddins

segiddins Nov 17, 2014

Member

requires_frameworks? ?

Show outdated Hide outdated lib/cocoapods/target.rb
@@ -41,6 +88,20 @@ def inspect
#-------------------------------------------------------------------------#
# @return [Boolean] whether the generated target need to be implemented
# as a framework

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

needs

Show outdated Hide outdated lib/cocoapods/target.rb
#-------------------------------------------------------------------------#
protected

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

should be private

@segiddins

segiddins Nov 17, 2014

Member

should be private

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

👍

Show outdated Hide outdated lib/cocoapods/target/aggregate_target.rb
pod_target.include_in_build_config?(build_configuration)
end.map(&:specs).flatten
result[build_configuration] = pod_targets_for_build_configuration(build_configuration).
map(&:specs).flatten

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

use flat_map

@segiddins

segiddins Nov 17, 2014

Member

use flat_map

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

👍

Show outdated Hide outdated spec/unit/installer/user_project_integrator_spec.rb
@integrator = UserProjectIntegrator.new(@podfile, config.sandbox, temporary_directory, [@target])
describe '#warn_about_xcconfig_overrides' do
before do
UI.warnings = ''

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

this shouldn't be necessary

@segiddins

segiddins Nov 17, 2014

Member

this shouldn't be necessary

@segiddins segiddins added this to the 0.36.0 milestone Nov 17, 2014

Show outdated Hide outdated lib/cocoapods/installer/target_installer/pod_target_installer.rb
# Set added headers as public if needed
if native_target.symbol_type == :framework
native_target.headers_build_phase.files.each do |build_file|

This comment has been minimized.

@neonichu

neonichu Nov 17, 2014

Member

This should only be done for headers defined in public_header_files of the corresponding spec, otherwise we'll end up with unbuildable frameworks (error will be "Umbrella header for module 'X' does not include header ‘Private.h’").

@neonichu

neonichu Nov 17, 2014

Member

This should only be done for headers defined in public_header_files of the corresponding spec, otherwise we'll end up with unbuildable frameworks (error will be "Umbrella header for module 'X' does not include header ‘Private.h’").

Show outdated Hide outdated lib/cocoapods/installer/user_project_integrator/target_integrator.rb
build_phase = native_target.frameworks_build_phase
# Find and delete possible reference for the other product type
old_product_name = target.requires_framework? ? target.static_library_name : target.framework_name

This comment has been minimized.

@mrackwitz

mrackwitz Nov 17, 2014

Member

@segiddins: This must not be product_name, because it evaluates the name, which the other product type would have.

@mrackwitz

mrackwitz Nov 17, 2014

Member

@segiddins: This must not be product_name, because it evaluates the name, which the other product type would have.

This comment has been minimized.

@segiddins

segiddins Nov 17, 2014

Member

ah gotcha

@segiddins

segiddins Nov 17, 2014

Member

ah gotcha

@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Nov 18, 2014

Member

As the transition to frameworks is a user code breaking change, we should have a way to state "don't use frameworks for this iOS8+ app,"

Member

orta commented Nov 18, 2014

As the transition to frameworks is a user code breaking change, we should have a way to state "don't use frameworks for this iOS8+ app,"

@bassrock

This comment has been minimized.

Show comment
Hide comment
@bassrock

bassrock Nov 19, 2014

Just checked out this branch to start early work on some extensions using pods. First, it is great progress and works well for most pods.

I am using NZCircularImageView and it fails for this because it imports a header like #import "UIImageView+UIActivityIndicatorForSDWebImage.h"

Will we have to circumvent breaks like this in the future or is there a way we can handle frameworks importing each other? Or is this one pod special and I should just use another?

Just checked out this branch to start early work on some extensions using pods. First, it is great progress and works well for most pods.

I am using NZCircularImageView and it fails for this because it imports a header like #import "UIImageView+UIActivityIndicatorForSDWebImage.h"

Will we have to circumvent breaks like this in the future or is there a way we can handle frameworks importing each other? Or is this one pod special and I should just use another?

@bassrock

This comment has been minimized.

Show comment
Hide comment
@bassrock

bassrock Nov 19, 2014

Also to add on all my pods in the list:

pod 'BlocksKit'
pod 'SDWebImage', '>= 3.5.2'
pod 'MRProgress', '>= 0.2.1'
pod 'Reachability', :git => 'https://github.com/tonymillion/Reachability.git', :branch => 'master'
pod 'AFNetworking',  '>=1.3.2'
pod 'NZAlertView'
pod 'JSQMessagesViewController'
pod 'MagicalRecord', '~> 2.2'
pod 'CrashlyticsFramework'
pod 'ActionSheetPicker-3.0', '>= 1.0.5'
pod 'SWTableViewCell', '0.2.4'
pod 'Facebook-iOS-SDK', '>= 3.13'
pod 'SVPullToRefresh', '>= 0.4'
pod 'GoogleAnalytics-iOS-SDK', '>= 3.0'
pod 'Parse', '~> 1.4'
pod 'INTULocationManager'

Worked expect for ActionSheetPicker-3.0 and Parse.

When importing ActionSheetPicker-3.0 as #import <ActionSheetPicker_3_0/Pods-ActionSheetPicker-3.0-umbrella.h> I get the error: Could not build module 'ActionSheetPicker_3_0' Upon further investigation it appears the umbrella file for this pod has the included .m files.

As for Parse it looks like the header files are not being included in the dynamic pod framework. I know its not a framework in a framework type issue, because the Crashlytics and Facebook pod do the same thing. The difference between them is they have the header files with the framework.

Also to add on all my pods in the list:

pod 'BlocksKit'
pod 'SDWebImage', '>= 3.5.2'
pod 'MRProgress', '>= 0.2.1'
pod 'Reachability', :git => 'https://github.com/tonymillion/Reachability.git', :branch => 'master'
pod 'AFNetworking',  '>=1.3.2'
pod 'NZAlertView'
pod 'JSQMessagesViewController'
pod 'MagicalRecord', '~> 2.2'
pod 'CrashlyticsFramework'
pod 'ActionSheetPicker-3.0', '>= 1.0.5'
pod 'SWTableViewCell', '0.2.4'
pod 'Facebook-iOS-SDK', '>= 3.13'
pod 'SVPullToRefresh', '>= 0.4'
pod 'GoogleAnalytics-iOS-SDK', '>= 3.0'
pod 'Parse', '~> 1.4'
pod 'INTULocationManager'

Worked expect for ActionSheetPicker-3.0 and Parse.

When importing ActionSheetPicker-3.0 as #import <ActionSheetPicker_3_0/Pods-ActionSheetPicker-3.0-umbrella.h> I get the error: Could not build module 'ActionSheetPicker_3_0' Upon further investigation it appears the umbrella file for this pod has the included .m files.

As for Parse it looks like the header files are not being included in the dynamic pod framework. I know its not a framework in a framework type issue, because the Crashlytics and Facebook pod do the same thing. The difference between them is they have the header files with the framework.

mrackwitz and others added some commits Dec 16, 2014

[Linter] Add new option `--use-frameworks`
This let lint use frameworks to install the given spec.

kylef pushed a commit that referenced this pull request Dec 25, 2014

Kyle Fuller
Merge pull request #2835 from CocoaPods/swift
Support for Frameworks / Swift

@kylef kylef merged commit c9ac09d into master Dec 25, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@kylef kylef deleted the swift branch Dec 25, 2014

@AliSoftware

This comment has been minimized.

Show comment
Hide comment
@AliSoftware

AliSoftware Dec 25, 2014

Contributor

💥

Contributor

AliSoftware commented Dec 25, 2014

💥

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