p4-fusion fails to build on macOS with recent Helix Core C/C++ API versions

[peter-esik]

Repro

1. Set up the environment for building p4-fusion
2. Download the Helix Core C/C++ API for ARM macOS, at least version 2023.2 (somewhat older versions might be affected, too)
3. Build p4-fusion

Expected: The build succeeds Actual: The build fails at the linking stage, citing many undefined references to curl and sqlite functions.

Interestingly, the issue does not occur on Linux (tried on Ubuntu 22.04 LTS).

The issue can be fixed by adding the p4script_curl and p4script_sqlite libraries here: https://github.com/salesforce/p4-fusion/blob/3eb434255cac210e2296df7e2b857ed32e66da01/p4-fusion/CMakeLists.txt#L43

I wonder if there are valid use cases for building the tool with older P4 SDK versions.

[JoshHarmon]

For what it's worth, one of the ways I dealt with similar build troubles on Linux (don't know why you didn't have trouble but I did) was to drop the p4script and p4script_c libraries from t_l_l since I didn't see any reason for lua scripting or curl or sqlite support to be needed (AIUI, those are mostly for the benefit of Helix Core Extensions). I was able to mirror ~25k CLs from a slice of our Perforce depot in the branchless mode successfully on Friday, so if I broke something, it certainly wasn't obvious.

See: P4API: Compiling and Linking Helix Server applications: Additional script libraries for Helix Core extensions

So, I wonder if those libraries even need to be built in the first place for p4-fusion's usecase of blasting large quantities of a handful of commands at the server. It seems likely to me that they aren't needed and that it would simplify things if they in fact aren't needed.

I'm early on in building my understanding of the p4-fusion codebase, so I could very well be overlooking something and would encourage other eyes/minds to check my work.

[peter-esik]

For what it's worth, one of the ways I dealt with similar build troubles on Linux (don't know why you didn't have trouble but I did) was to drop the p4script and p4script_c libraries from t_l_l since I didn't see any reason for lua scripting or curl or sqlite support to be needed

If I reduce the set of libraries to just client, rpc, and supp, there are still linking errors (at least on macOS ARM). For example, P4Libraries::Shutdown in libclient.a references p4_sqlite3_shutdown residing in libp4script_c.a, which in turn references _sqlite3_initialize from libp4script_sqlite.a. I don't know why no such issues are present on Linux, maybe the P4 SDK is built differently on that platform.

The easiest solution would be to include these 7 static libraries on all platforms, and let the linker drop unneeded ones (if there are any). Alternatively, we could introduce branching in CMake per platform.

Which solution would you prefer @twarit-waikar ?

[twaritwaikar]

@peter-esik Thanks for looking into this! I don't think anything would have changed in the logic by different libraries being linked on Linux. But letting the linker handle it sounds good.

This part of the CMake scripts can turn out to be brittle due to us depending on how Perforce decides to build their libraries, so it's best let the complexity stay low here and let the linker handle chopping off the ones that are not required.

[peter-esik]

I think I managed to find a correct solution. There is a copy of the source code of the SDK here (not sure if it is a leak, or legal, though...). Looking at the source code it's easy to see what's going on: some source files in the client library call functions in the p4script library, even though extensions are supposed to be an optional part of the SDK. As a result, if your application relying on the SDK does not use the scripting functionality, you still either have to link these libraries as well, or link empty (or dummy, if you will) implementations of these functions.

This is what the p4script_cstub library achieves, if you look at its source code: https://github.com/Malterlib/p4source/blob/26f2c63fd3ee69498a62957850ee1661e4a26fc7/client/clientscript_stub.cc

By adding this library, my build succeeds on both Linux and macOS. As a bonus, we don't have to refer the sqlite and curl libraries bundled with the SDK, so p4-fusion will still build with older P4 SDK versions.

[peter-esik]

The p4script_cstub library was introduced in version 2023.1, with the following notes in the changelog:

The libp4script_cstub library has been added to the P4API
distribution for consumers not needing client side extension
support, substituting libp4script_c, libp4script, libp4script_curl
and libp4script_sqlite.

[JoshHarmon]

Yikes, my apologies for forgetting/omitting the fact I added the linkage to the cstub library; that was a very important detail!

[JoshHarmon]

Also to ease any concerns about the P4API source licensing mentioned by @peter-esik: https://swarm.workshop.perforce.com/projects/perforce_software-p4 mentions the code being available on an FTP site (e.g. for 24.1: https://ftp.perforce.com/perforce/r24.1/bin.tools/)