gkdr / lurch

XEP-0384: OMEMO Encryption for libpurple.
GNU General Public License v3.0
289 stars 32 forks source link

Use dynamically linked libraries axc and omemo #151

Closed fortysixandtwo closed 3 years ago

fortysixandtwo commented 4 years ago

Hi,

as part of the DebianOnMobile team, we are aiming at bringing lurch into debian. See the packaging [1] which is currently in the NEW queue. This PR is related to [2] and [3]. In principle two things have been done to the Makefile:

Cheers

PS: I have seen that there are already pull requests [4] and [5]. Would you be willing to carry the packaging from [1] upstream?

[1] https://salsa.debian.org/DebianOnMobile-team/purple-lurch [2] https://github.com/gkdr/axc/pull/17 [3] https://github.com/gkdr/libomemo/pull/30 [4] https://github.com/gkdr/lurch/pull/136 [5] https://github.com/gkdr/lurch/pull/141

fortysixandtwo commented 3 years ago

Hey @gkdr! Does the CI configuration live outside of the tree? (Because I haven't figured out where it might be coming from)

From looking at the log we would need to also install the dependencies libaxc-dev and libomemo-dev https://ci.appveyor.com/project/gkdr/lurch/builds/33903430#L96 Although I must say, I have no idea if those libraries are available in Ubuntu 1{6,8}.04 which seems being used in the CI (?)

gkdr commented 3 years ago

hi @fortysixandtwo, thanks for the contribution and sorry for taking so long to reply.

the CI config comes from the .appveyor.yml (which only exists in the dev branch currently). for the same reasons as in gkdr/axc#17 (i.e. the windows build), it would be better if the possibility to just statically link everything together remains intact. running the tests like that would also solve the problem with the CI.

fortysixandtwo commented 3 years ago

Sure thing, I'll cook something up and update it.

Neustradamus commented 3 years ago

Arch Linux: @freswa waits this PR.

gkdr commented 3 years ago

hi @fortysixandtwo,

i just noticed you had another question i didn't answer. however, i also don't understand it. i was advised not to include packaging information in a repository like this. what would be the reason to include it after all, in your opinion?

codecov-io commented 3 years ago

Codecov Report

Merging #151 (19bc529) into dev (6b6cdca) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #151   +/-   ##
=======================================
  Coverage   20.22%   20.22%           
=======================================
  Files           4        4           
  Lines        2309     2309           
=======================================
  Hits          467      467           
  Misses       1842     1842           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6b6cdca...19bc529. Read the comment docs.

fortysixandtwo commented 3 years ago

So I finally came around to updating the PR. Statically linking is now still possible and dynamic linking is only done if the required libraries have been found.

Regarding the packaging files: I don't really know what I was thinking "back then" and I don't think there is any upside to this :)

gkdr commented 3 years ago

thank you, i like the way the linking can be controlled! now i just gotta figure out why the hell the coverage changed :thinking:

fortysixandtwo commented 3 years ago

I shamelessly took inspiration from https://github.com/gkdr/axc/pull/17 for the general approach :)

fortysixandtwo commented 3 years ago

Regarding the coverage: Seems I needed to rebase against the dev branch, my bad!

fortysixandtwo commented 3 years ago

Leaving a friendly ping @gkdr

gkdr commented 3 years ago

thanks, i need that sometimes :grimacing: and thanks for figuring out the coverage, luckily it was easy enough :slightly_smiling_face:

Neustradamus commented 3 years ago

@gkdr: Thanks!