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

Swift Static Library support #6966

Merged
merged 42 commits into from Feb 20, 2018
Merged

Swift Static Library support #6966

merged 42 commits into from Feb 20, 2018

Conversation

endocrimes
Copy link
Member

@endocrimes endocrimes commented Aug 18, 2017

Closes #6899.

  • Examples that build static swift
  • New integration specs
  • Validator to ensure no swift pod targets depend upon built targets that don't define modules
  • CHANGELOG entry
  • Allow enabling/disabling module map generation for all pods in a target from the Podfile
  • Allow enabling/disabling module map generation for one pod in a target from the Podfile
  • Move the generated umbrella header to be in the public headers directory rather than private headers?

@keith
Copy link
Member

keith commented Aug 18, 2017

This is fantastic! I'm testing it out now, the first big issue I've noticed is that Swift pods with Swift dependencies need their dependencies' build paths in their SWIFT_INCLUDE_PATHS.

@keith
Copy link
Member

keith commented Aug 18, 2017

Here's a sample project for this TestSwiftNestedDependencies.zip (looks like Sonar doesn't actually compile in Swift 4, but it still shows the issue).

This can be fixed by adding ${PODS_CONFIGURATION_BUILD_DIR}/Alamofire to Sonar's SWIFT_INCLUDE_PATHS

@keith
Copy link
Member

keith commented Aug 18, 2017

The next issue I've noticed is that if you have a Swift pod that depends on an Objective-C pod, it has trouble building the module:

image

Here's an example project TestObjectiveCDependency.zip

In this project I've added the SWIFT_INCLUDE_PATHS mentioned above. I've solved a similar problem like this before by making the path in the module map absolute, but that doesn't seem to work in this case. Either way I assume there's a better solution for that 🤔

@endocrimes
Copy link
Member Author

@keith Both of those projects build for me now 👍.

@keith
Copy link
Member

keith commented Aug 19, 2017

Nice! Looks like that works on my end too. I noticed that specs using module_name don't seem to set the PRODUCT_NAME correctly. It looks like you explicitly commented that out though? If you want a test pod for that ModelMapper defines the module name as Mapper

@endocrimes
Copy link
Member Author

@keith I wasn't sure if it was going to be needed, but looks like it will be 👍

@keith
Copy link
Member

keith commented Aug 19, 2017

Next thing I ran into is it seems that the Facebook SDK fails to build statically because of some non modular include warnings. I don't see this when using frameworks today. This obviously could 100% be on them, but figured it was worth mentioning. Here's a sample project: TestFacebookSDK.zip

@keith
Copy link
Member

keith commented Aug 19, 2017

Hmm actually it looks like it's happening with other Objective-C pods we're using as well. I can give another project example if needed.

@endocrimes
Copy link
Member Author

@keith Good to know - a smaller example than FBSDK would be handy, but I think that's because I'm not preserving header paths properly yet.

@keith
Copy link
Member

keith commented Aug 19, 2017

Here's a slightly smaller example. It appears to be an issue with imports referencing nested Objective-C dependencies, so this example also has 2 dependencies: TestLightstep.zip

@keith
Copy link
Member

keith commented Aug 19, 2017

Ah the problem appears to be the imports in the umbrella header generated by CP. In both of these sample projects, commenting all the imports in that file out makes them compile.

@dnkoutso dnkoutso added this to the 1.4.0 milestone Sep 1, 2017
@fabb
Copy link

fabb commented Sep 6, 2017

Is there a pre version of cocoapods where static swift libs can be tried out?

@chenxiao0228
Copy link

chenxiao0228 commented Sep 6, 2017

@dantoml I tried your branch and applied it to our large scale project. In addition to your changes, I also made following changes for our project to build:

  • module_map.rb "framework module" -> "module" for static lib
  • pod_xcconfig.rb add dependent_targets' configuration_build_dir to HEADER_SEARCH_PATHS for Xcode to be able to find modulemap and umbrella headers
  • xcconfig_helper.rb add "-import-underlying-module" for swift/objc hybrid libs
  • tweaks in pod_target_installer.rb

There is however one issue left: make a swift class visible to ObjC class which depends on the lib. In theory this requires @import module -> modulemap -> umbrella -> module-Swift.h inclusion chain. I can copy it from derived-object folder but I am not sure if it's the best approach.

I attached my diff. It's just a quick hack and really messy. Let me know if you wanna collaborate.

@Pearapps
Copy link

Is this branch still being updated? Is there anything we can do to help move this along?

@dnkoutso
Copy link
Contributor

@Pearapps yes! if you feel you can move this faster than the current pace feel free to pick it up and get it past the finish line.

As a reminder, most of the work in CocoaPods is done from people's free time. There is never a deadline or strict timeline to get things done.

@Pearapps
Copy link

I opened up a PR against this that uses that diff that @chenxiao0228 put up in a previous comment #7067

@endocrimes
Copy link
Member Author

Sorry for the silence on this, I've been super busy lately, and CocoaPods feature work has taken a back seat.

I'm taking another look at this today.

@endocrimes
Copy link
Member Author

@chenxiao0228 Thanks for that diff, coercing module maps into the right format was one of the things I was wondering about.

@@ -88,6 +88,9 @@ def custom_build_settings
settings['PRIVATE_HEADERS_FOLDER_PATH'] = framework_name + '.framework' + '/PrivateHeaders'
end
else
# if target.uses_swift?
Copy link
Member

Choose a reason for hiding this comment

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

commented code

@dnkoutso
Copy link
Contributor

Punted to 1.5.0.

@dnkoutso
Copy link
Contributor

Reviving this for 1.5.0 now that 1.4.0 shipped.

xcodebuild fails if a framework target has PRODUCT_MODULE_NAME
While well-intentioned, they broke cross-project target dependency detection
… umbrella headers

This allows us to avoid incomplete umbrella header warnings
By default, targets integrated as static libraries _will not_ get a module map, unless they opt-in by including `DEFINES_MODULE = YES` in their specification's `pod_target_xcconfig`
This is to mirror our use of -isystem for header search paths, making it so that headers imported via <> have warnings supressed
@segiddins
Copy link
Member

Rebased, this is basically ready to go!

@mskafi
Copy link

mskafi commented Mar 11, 2018

Question: Will this resolve the issue with transitive dependency stuff?

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.

None yet