Adding ability to build as a shared library

[henry-nicolas]

Dear,

This PR adds support for several things, detailed hereunder.

I am working with Debian's team DebianOnMobile to get this library into Debian so we can later add purple-lurch to add OMEMO support to the chatty client.
In our distribution context, library must be built as shared objects and not statically linked.

This merge does the following change:

• create build target for a shared object (called shared)
• make the git submodule for libsignal-protocol-c non necessary as we do not statically link against it, we load it from the system's libraries.
• Adds some CFLAGS for security hardening purposes
• Adds a pkgconfig file descriptor
• Remove the direct call to session_builder_process_pre_key_signal_message which is marked on upstream source tree as internal and therefore not to be called by third party software. The required headers (session_builder_internal.h) do not come installed with libsignal-protocol-c-dev
• Adds a make install target

If you would merge this PR, it would make the distribution of the axc software easier for sure :)

Best regards,

[henry-nicolas]

Regarding session_builder_process_pre_key_signal_message which is internal, it is actually also called via session_cipher_decrypt_pre_key_signal_message which you also use later on inside the same function under src/axc.c.

The packaging for axc is currently taking place here if you wish to have a look:
https://salsa.debian.org/DebianOnMobile-team/libaxc

[gkdr]

Hi, thank you for your work. By now, libsignal-protocol-c should really be available as a package everywhere itself, so this was overdue. I'll have to think about whether it's good to have this in the coming release of lurch, but the answer is probably "yes".

I'll look into what the function you had to comment out does, but I assume I just misunderstood the API at some point. This is tracked in #16.

Also, if you don't mind, I'll change the base branch of this PR to "dev".

[gkdr]

I noticed you removed the target for compiling the static libsignal-protocol-c completely, but I probably need that for the Windows build.

[henry-nicolas]

Indeed - I could update this PR and re-add the AX bits:

• AX_PATH target, variable definition and cleanup
• LDFLAGS : those should become platform dependant then?
  On Windows you would statically link against libsignal-protocol-c and on the rest, use shared library

Let me know if that makes sense to you before I go ahead and make the changes

[henry-nicolas]

... alternatively, you could also build with dynamic linking for windows as well, since you were already doing it for sqlite3 and gcrypt, why for signal-protocol-c as well?

[gkdr]

I think I need this because gcrypt and sqlite3 provide precompiled libraries for Windows, but libsignal-protocol-c does not.

I am not the one building this for Windows, so I'm not entirely sure right now, but if I remember correctly from my other plugin I just used a completely different set of targets. Aside from just offering the possibility to anyone building from source, I think I will just mostly need it as an example for when I finally get around to it. So I don't think you need the consider anything specific, just adding it back is fine.

[henry-nicolas]

I made further changes so the Makefile behavior is platform dependant now.

Building on Windows platform will keep the statically linked signal-protocol-c library.
Building on other platform will use the shared signal-protocol-c library.

Let me know if you would like any further changes.

[henry-nicolas]

@gkdr - also , if and when you accept this PR, you could bump your release number then we would just re-use the tagged release pristine and drop all downstream patches we have

[henry-nicolas]

@gkdr could you please review this PR?

Currently it is distributed downstream (inside Debian packaging), would be easier if that was part of upstream releases.

[Neustradamus]

I think that this PR must be merged quickly.

@henry-nicolas, @fortysixandtwo, @gkdr: I hope before the Debian 11 freeze which arrives very soon...

Note: The next Debian 12 will be in 2023...

Linked to:

• Use dynamically linked libraries axc and omemo lurch#151

[henry-nicolas]

@Neustradamus - no rush here, the patch is used in downstream Debian.
I assume @gkdr will merge it once happy with the changes, that will just ease a bit downstream distribution as I would no longer need to maintain the corresponding patch. But it is not blocking anything.

[gkdr]

@Neustradamus what does "very soon" mean?

@henry-nicolas i'm sorry, i lost track of this.

[henry-nicolas]

@gkdr @Neustradamus allow me to clarify the timing aspects.

Next Debian release will see its freeze start in mid February.
This means that, should @gkdr wish for axc to be distributed pristine, then this MR should be reviewed/updated/merged prior to that date.

However, I feel there's no rush here as we have a working solution. As long as we can get the review progressed, I am happy.

[gkdr]

hi @henry-nicolas, sorry once again for the wait.
the submodule is still marked deleted currently, is it okay to readd it?
also, you asked for a version bump. considering the changes, i think it should just bump the patch version number. is that enough for your needs?

[henry-nicolas]

@gkdr no problem for the wait :-)

I re-added the submodule (sorry that I had forgot that). It is totally fine to bump the patch version number, thanks.

[gkdr]

make clean-all test currently does not work because the prerequisites do not include the $(AX_PATH) any longer. would it be wrong/annoying to just readd it for the tests, or do you have another suggestion on how to resolve this?

[henry-nicolas]

That's fixed now. I declared the test target conditionnaly in the Makefile (as for headers definition earlier):

• if pkgconfig founds libsignal-protocol-c, then the git submodule is not used and the test target is kept as-is.
• otherwise, the git submodule is used and the test target uses $(AX_PATH)

[gkdr]

thanks, i think this works for both of us now :D

[henry-nicolas]

thanks, i think this works for both of us now :D

Great, thanks for all the follow-up :-)

[Neustradamus]

@gkdr: Thanks for the merging!
@henry-nicolas: Thanks for your work!

Last step is: gkdr/lurch#151 :)
cc: @fortysixandtwo.