Description

Changes the packaging script to use LLVM binutils. This fixes object files corrupted by using objcopy by using llvm-objcopy instead. While the corrupted files were working fine when using the Visual Studio toolchain they were leading to errors when using the LLVM toolchain.

See #793 (comment) for details.

---

Testing

Needs to be run as part of the actions packaging. I will test builds once they are available.

---

Type of Change

Place an x the applicable box:

• Bug fix. Add the issue # below if applicable.
• New feature. A non-breaking change which adds functionality.
• Other, such as a build process or documentation change.

---

Notes

• Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
• Read the contribution guidelines CONTRIBUTING.md.
• Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

@jonsimantov I hope I made the right changes based on your feedback - if not please let me know. Any chance you could trigger the CI to run with this branch?

I'll go ahead and test the packaging process with these changes. To do that I believe I'll have to copy these changes into a branch within this repo.

Invalid workflow file: ./workflows/cpp-packaging.yml#L130
The workflow is not valid. ./workflows/cpp-packaging.yml (Line: 130, Col: 16): Unexpected symbol: '['. Located at position 43 within expression: matrix.tools_platform == 'darwin' && join(['-', env.xcodeVersion]) || ''

Thanks for checking – can you try again with the latest changes now please?

OK, here goes: https://.com/firebase/firebase-cpp-sdk/actions/runs/6190110457

Fixed a missing quote, round 2: https://.com/firebase/firebase-cpp-sdk/actions/runs/6190946332

Thanks @jonsimantov! I tried the build and it works great for me using the LLVM toolchain.

One thing I noticed is that the firebase_cpp_sdk_windows.zip artifact itself contains another ZIP file, but I don’t think that stems from these changes?

Unfortunately, the integration tests failed to build against the newly packaged SDK on Windows.

I've attached the log file.

8_Build integration tests.txt

Hmm ok, so the issue here is all these error LNK2001: unresolved external symbol errors, right? Any idea how they could be caused by this change?

Is this also a new issue: Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed?

@aganea would you be able to take a look at these errors? Unfortunately I’m a bit lost about how this change might be causing them.

I rebased the branch onto the latest main.

@jonsimantov could you please run the tests once more to see if these linker errors are still happening? 🙏

It's still failing to link. Log attached:
8_Build integration tests.txt

Thank you @jonsimantov! It seems like the packaged library is missing a bunch of symbols.

Since the -L flag we’re adding in this is also making the packaging script use llvm-nm and llvm-ar (in addition to llvm-objcopy), I’m wondering if those are somehow behaving differently to cause this. Do they require some extra flags maybe – any idea @aganea?

Here is the same log without timestamps as well as the one from the latest run off the main branch for comparison:

• 8_Build.integration.tests-llvm.txt
• 8_Build integration tests-main.txt

The Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed is also present in the latest run and thus not a new issue.