search

signalapp / libsignal-protocol-c

GNU General Public License v3.0
1.41k stars 295 forks source link

Remove bundled protobuf-c #128

Open fbrausse opened 5 years ago

fbrausse commented 5 years ago

Hi,

Besides Fedora (#103) removing the bundled (old) protobuf-c is also wished for packaging (and using it) in the Gentoo ecosystem, where the package is already provided. All I could find related to the need for the copy is a statement in https://github.com/signalapp/libsignal-protocol-c/pull/105#issuecomment-369988065. Unfortunately, there is no explanation as to why this obscure

frozen-in-time

copy is required and linking with the system's libprotobuf-c.{so or a} is not an option. For this PR I removed the protobuf-c copy (and updated the generated files). All tests still pass (shared and static).

It does not address #103's "making the internal copy optional", though, as I don't understand to reason for bundling in the first place.

dkonigsberg commented 5 years ago

The reason for the bundling is that this library was not originally intended to be a shared library package bundled with a desktop Linux distribution. It was intended to be a static library linked directly into the binary of a smartphone app, that would work with little to no dependencies on a wide variety of platforms. As such, the protobuf-c code was considered to be more of an "implementation detail" than a dependency. (That's also why, as stated in https://github.com/signalapp/libsignal-protocol-c/pull/105#issuecomment-369988065 , it was modified for old-compiler-friendliness.)

Another issue is that the Java version of this library is pinned to a specific old version of the upstream Protocol Buffers library (2.5.0), which could necessitate this library also pinning to an old version of protobuf-c. That would make things more complicated and annoying if an external version of that library were used.

Finally, you're talking about a single source/header file, that does not actually export any symbols via this library.

fbrausse commented 5 years ago

Thank you for your explanations! From what I understand about the Protocol Buffers spec, it is meant to be stable in the sense that serializations produced by library A can be deserialized by library B if both implement the spec. There is freedom in which order field instances get serialized and parsers have to be able to handle any order within one such structure, atomic fields however are serialized in the same format. From that point of view, different versions A-v1 and A-v2 just take the roles of A and B (or B and A) above.

Out of curiosity: Is the reason for pinning Protobuf-v2.5.0 that libsignal-protocol-java (or other software from the signalapp-stack) depends on specific serialization characteristics of that particular lib which matches the bundled protobuf-c here?