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

Add support for mixed objective-c and pure c++ code in same apple_library #304

Closed
wants to merge 4 commits into from
Closed

Add support for mixed objective-c and pure c++ code in same apple_library #304

wants to merge 4 commits into from

Conversation

ivan-golub
Copy link

@ivan-golub ivan-golub commented Sep 21, 2021

Background:

  • It is possible to create library that will contain mix of objective-c, objective-c++, pure c and pure c++ code in vanilla Xcode.
  • objc_library supports all languages above as well
  • Current apple_library adds .m, .mm, .c sources as well as .hm .hh, .hpp headers headers but excludes .cc and .cpp which causes compilation error if library has pure c++ code

Changes:

  • Add c++ source code (.cc and .cpp) files in srcs of apple_library
  • Add test project that mixes plain objective-c and pure c++

Test Plan:

  • Run bazel test //tests/ios/frameworks/objcpp:ObjCppMixedLibTests

Risks:

  • Logic of https://github.com/bazel-ios/rules_ios/blob/master/rules/library.bzl#L866 is not entirely clear from comment. If enable it, then lib containing both c++ and objective-c will create two libraries, one with objective-c sources and objective-c + c++ headers (likely not compilable due to missing c++ implementations) and cc library that contains all objective-c private headers and c++ implementations. cc_library idea needs clarification and either should be removed and this PR merged or revived in form that will allow tests from this PR to pass

@nkoroste
Copy link
Contributor

@jerrymarino fyi

Copy link
Contributor

@jerrymarino jerrymarino left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and adding the example!

I'd be in favor to re-enable https://github.com/bazel-ios/rules_ios/pull/304/files#diff-23068963449a2bf4d162d813a4dda51236b9d044d1442e8a4bb4ad7680473478L867 to set the language level copts at the lib level

@@ -898,7 +898,7 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names

objc_library(
name = objc_libname,
srcs = objc_sources + objc_private_hdrs + objc_non_exported_hdrs,
srcs = objc_sources + objc_private_hdrs + objc_non_exported_hdrs + cpp_sources,
Copy link
Contributor

@jerrymarino jerrymarino Sep 30, 2021

Choose a reason for hiding this comment

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

I wonder what the the tradeoffs of this line and https://github.com/bazel-ios/rules_ios/blob/master/rules/library.bzl#L870 are ?

I don't know why https://github.com/bazel-ios/rules_ios/blob/master/rules/library.bzl#L870 was disabled, one of the problems with putting the sources in the same library is there is no way to specify the C++ specific copts at the library level.

@jerrymarino
Copy link
Contributor

Also Cocoapods, it compiles C++ sources as a separate objc_library maybe we can just apply similar logic in that line to make the logic more close to that.

@mattrobmattrob
Copy link
Collaborator

Closing since this should be handled in #550. Reopen #537 if it doesn't!

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.

4 participants