Remove extra inclusion of pb_common.c and pb_decode.c, to avoid multiple symbol definitions. #275
Conversation
multiple symbol definitions.
| # Remove pb_common.c and pb_decode.c from PROTO_SRCS, as they are | ||
| # provided by Firebase App. Including them here causes symbols to | ||
| # have multiple definitions. | ||
| list(FILTER PROTO_SRCS EXCLUDE REGEX "/pb_.*\\.c") |
There was a problem hiding this comment.
I think this would only fix it if Firestore is included in the build, since that is the other thing adding proto.
I think you could put a
if(FIREBASE_INCLUDE_FIRESTORE)
...
endif()
There was a problem hiding this comment.
I think this would only fix it if Firestore is included in the build, since that is the other thing adding proto.
I actually don't think I need to do this. Here are the situations:
Binary SDK
Currently
Nanopb is vendored into Firebase App via packaging. It's referenced by Firestore. It's [currently] included in Remote Config as well, not via packaging but via building and linking them in PROTO_SRCS here. This only causes the multiple definitions error when Firestore is linked because the act of having Firestore reference Nanopb externally causes those symbols to have to be resolved by the linker, which then sees both versions.
(Neither Firestore nor its dependencies use this NANOPB_GENERATE_CPP rule, so the pb_decode and pb_common files are not actually linked into Firestore.)
With this change
Nanopb is vendored into Firebase App via packaging. It's referenced by Firestore and Remote Config. No multiple definitions.
Source SDK
Currently
Nanopb is included in Remote Config via PROTO_SRCS, as above. It's set as a linker library in Firestore. These could possibly conflict depending on the order in which you link Remote Config and Firestore, and which compiler you are actually using (MSVC vs GCC vs clang).
With this change
Nanopb is set as a linker library in both Firestore and Remote Config. If you include both libraries in your app, they will both depend on protobuf-nanopb-static and that file will only be linked once. We download nanopb as part of our external dependencies, and find_package(Nanopb) is included in our root CMakeLists.txt, so that library target should be available even without Firestore enabled.
Fix for #271.