Use llvm-objcopy to package Windows binaries

[triplef]

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 GitHub 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.

[triplef]

@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?

[jonsimantov]

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.

[jonsimantov]

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

[triplef]

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

[jonsimantov]

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

[jonsimantov]

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

[triplef]

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?

[jonsimantov]

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

[triplef]

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?

[triplef]

@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.

[triplef]

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? 🙏

[jonsimantov]

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

[triplef]

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

Since the -L flag we’re adding in this patch 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.