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

Enables C++ support in srcs #550

Merged
merged 2 commits into from
Aug 31, 2022
Merged

Conversation

jszumski
Copy link
Collaborator

@jszumski jszumski commented Aug 24, 2022

An alternative to #304 that enables the existing C++ target. Resolves #537.

  • Sends C++ sources into a separate target instead of the existing Obj-C target to enable different copt values for each.
  • Re-uses the same mixed Obj-C and C++ test fixture, thanks @ivan-golub!

additional_cc_copts.append("-I.")
native.cc_library(
native.objc_library(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This had to be an objc_library to avoid an error in framework_packaging_impl where it expected all deps would have an Objc provider

Copy link
Collaborator

@luispadron luispadron left a comment

Choose a reason for hiding this comment

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

Believe this closes #537

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.

@jszumski - thanks for the PR and adding the test case overall, this LGTM! The one question I had for you is around the .mm files. Addressing that may be better as a followup if it's going to change behavior for people. Historically, these needed to be compiled adjacent to the C++ in Bazel. If you reference, PodToBUILD ( ObjcLibrary.swift ) - I believe you'll find it done this way or similar - that it needs has to mirror cocoapods and apply C++ specific flags ( e.g. from CXX xcconfig group ) to the files

name = cpp_libname,
srcs = cpp_sources + objc_private_hdrs,
srcs = cpp_sources + objc_private_hdrs + objc_non_exported_hdrs,
hdrs = objc_hdrs,
copts = copts_by_build_setting.cc_copts + cc_copts + additional_cc_copts,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a number of flags: perhaps you can add as a followup.


    additional_objc_vfs_deps = select({
        "@build_bazel_rules_ios//:virtualize_frameworks": [framework_vfs_overlay_name_swift] + [framework_vfs_overlay_name],
        "//conditions:default": [framework_vfs_overlay_name_swift] + [framework_vfs_overlay_name] if enable_framework_vfs else [],
    })
    additional_objc_vfs_copts = select({
        "@build_bazel_rules_ios//:virtualize_frameworks": framework_vfs_objc_copts,
        "//conditions:default": framework_vfs_objc_copts if enable_framework_vfs else [],
    })

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A follow up is a good idea 👍

Copy link
Collaborator

@mattrobmattrob mattrobmattrob left a comment

Choose a reason for hiding this comment

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

FWIW, this works in our build which has C++ sources for a couple CocoaPods. We were previously rewriting these into .mm files and now we don't have to! 🎉

    if not name.endswith(".cpp"):
        return name

    return _copy_genrule(
        name.replace(".cpp", "_transmmogrified.mm"),
        name,
        name.replace(".cpp", ".mm"),
    )

@luispadron luispadron merged commit d8e7b29 into bazel-ios:master Aug 31, 2022
@jszumski jszumski deleted the enable-cpp branch August 31, 2022 13:24
yongjincho92 pushed a commit to yongjincho92/rules_ios that referenced this pull request Nov 8, 2022
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.

framework macro does not seem to include cpp source files
4 participants