Updated Analytics Windows to allow multiple DLL hashes, and add usage of Options struct. #1744
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add Crypt32.lib to the linked libraries for the firebase_analytics target on Windows. This resolves a linker error LNK2019 for _CryptBinaryToStringA@20, which was occurring in builds that included analytics_windows.obj (e.g., firebase_analytics_test). While CryptBinaryToStringA is not directly called in analytics_windows.cc, the linker indicates that analytics_windows.obj requires this symbol, possibly due to an indirect dependency or other build system interactions. Linking Crypt32.lib provides the necessary symbol.
I've added Crypt32.lib to the dependencies of the firebase_analytics_test target in `analytics/tests/CMakeLists.txt` when building on Windows. This is to resolve a persistent LNK2019 error for _CryptBinaryToStringA@20, which occurs during the linking phase of firebase_analytics_test.obj. This change directly links the required system library at the test executable level.
…rebase/firebase-cpp-sdk into fix/analytics-dll-hash-sizeof
This change updates the desktop analytics initialization to use the
newly required Options struct for the Windows C API.
- In `analytics/src/analytics_desktop.cc`:
- `firebase::analytics::Initialize(const App& app)` now retrieves
`app_id` and `package_name` from `app.options()`.
- It calls `GoogleAnalytics_Options_Create()` to create the options struct.
- Populates `app_id`, `package_name`, and sets a default for
`analytics_collection_enabled_at_first_launch`.
- Calls `GoogleAnalytics_Initialize()` with the populated options struct.
- String lifetimes for `app_id` and `package_name` are handled by
creating local `std::string` copies before passing their `c_str()`
to the C API.
* Initialize Analytics C SDK with AppOptions on desktop
This change updates the desktop analytics initialization to use the
newly required Options struct for the Windows C API.
- In `analytics/src/analytics_desktop.cc`:
- `firebase::analytics::Initialize(const App& app)` now retrieves
`app_id` and `package_name` from `app.options()`.
- It calls `GoogleAnalytics_Options_Create()` to create the options struct.
- Populates `app_id`, `package_name`, and sets a default for
`analytics_collection_enabled_at_first_launch`.
- Calls `GoogleAnalytics_Initialize()` with the populated options struct.
- String lifetimes for `app_id` and `package_name` are handled by
creating local `std::string` copies before passing their `c_str()`
to the C API.
* Format code.
* Fix build issues.
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
jonsimantov
changed the title
a-maurice
reviewed
Jun 25, 2025
analytics/generate_windows_stubs.py
Outdated
| f.write("// clang-format off\n") | ||
| f.write(f'\n// Number of Google Analytics functions expected to be loaded from the DLL.') | ||
| f.write(f'\nconst int FirebaseAnalytics_DynamicFunctionCount = {len(function_details_for_loader)};\n\n'); | ||
| # f.write("#if defined(_WIN32)\n") |
analytics/CMakeLists.txt
Outdated
| @@ -104,6 +104,9 @@ set_property(TARGET firebase_analytics PROPERTY FOLDER "Firebase Cpp") | |||
| # Set up the dependency on Firebase App. | |||
| target_link_libraries(firebase_analytics | |||
| PUBLIC firebase_app) | |||
| if(WIN32) | |||
| target_link_libraries(firebase_analytics PUBLIC Crypt32.lib) | |||
There was a problem hiding this comment.
Does this need to be PUBLIC? Looking over other cases, we usually try to make these private, though there might be a reason this one needs to be public
There was a problem hiding this comment.
Let's try it and see what happens!
github-actions
bot
added
the
tests: in-progress
✅ Integration test succeeded!Requested by @jonsimantov on commit 57f303d |
github-actions
bot
added
the
tests: succeeded
firebase-workflow-trigger
bot
removed
the
tests: in-progress
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR updates the generate_windows_stubs.py script to keep track of known DLL hashes in known_hashes.txt and allow any of those when loading the Analytics DLL. This allows us to test multiple versions (e.g. compiled in debug mode vs. release mode) of the DLL.
This PR also adds support for the
GoogleAnalytics_Optionsstruct, which is required to initialize Analytics for Windows. This struct is populated from the Firebase App's AppOptions, except for the flag to enable data collection on first run, which defaults to true and can be disabled with a call tofirebase::analytics::SetAnalyticsCollectionEnabled(false)prior to Initialize() (on iOS/Android this is set via the app's Info.plist and AndroidManifest.xml, neither of which exist on Windows).Testing
Tested manually on Windows. Stub mode tested via integration test run.
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.