Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dynamically linked libraries axc and omemo #151

Merged
merged 3 commits into from
Jan 31, 2021

Conversation

fortysixandtwo
Copy link
Contributor

@fortysixandtwo fortysixandtwo commented Jul 4, 2020

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:

  • Switch to dynamically linked libraries for libaxc and libomemo.
  • Order the *.c files, so we can build reproducibly in debian (reprotest fails without this explicit ordering under the locale variation, because the *.c expansion will be locale dependent resulting in different ordering of the rodata and other segments in the compiled binary

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] gkdr/axc#17
[3] gkdr/libomemo#30
[4] #136
[5] #141

@fortysixandtwo
Copy link
Contributor Author

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
Copy link
Owner

gkdr commented Dec 4, 2020

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
Copy link
Contributor Author

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

@Neustradamus
Copy link

Arch Linux: @freswa waits this PR.

@gkdr
Copy link
Owner

gkdr commented Jan 17, 2021

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?

@fortysixandtwo fortysixandtwo force-pushed the dynamically_linked_axc_omemo branch 3 times, most recently from 2c39908 to 404fc45 Compare January 17, 2021 22:25
@codecov-io
Copy link

codecov-io commented Jan 17, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

gkdr commented Jan 19, 2021

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

@fortysixandtwo
Copy link
Contributor Author

I shamelessly took inspiration from gkdr/axc#17 for the general approach :)

@fortysixandtwo fortysixandtwo force-pushed the dynamically_linked_axc_omemo branch from 404fc45 to 19bc529 Compare January 19, 2021 21:44
@fortysixandtwo
Copy link
Contributor Author

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

@fortysixandtwo
Copy link
Contributor Author

Leaving a friendly ping @gkdr

@gkdr
Copy link
Owner

gkdr commented Jan 31, 2021

thanks, i need that sometimes 😬
and thanks for figuring out the coverage, luckily it was easy enough 🙂

@gkdr gkdr merged commit 12098a1 into gkdr:dev Jan 31, 2021
@Neustradamus
Copy link

@gkdr: Thanks!

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 8, 2022
Actually works with pidgin, talking to Conversations via federation!
(0.6.8 did not)


Upstream:

## [0.7.0] - 2021-02-12
### Added
- This file.
- An API reachable through _libpurple_ signals. See `lurch_api.h` for details and usage.
- Testing setup using _cmocka_ and tests for new modules.
- CI setup running the tests in _appveyor_ and reporting coverage results to _codecov_.
- The possibility to dynamically link against the submodule libaries. ([#151](gkdr/lurch#151)) (thanks, [@fortysixandtwo](https://github.com/fortysixandtwo)!)

### Changed
- A new `/command` handler using the API, replacing the old implementation. The commands are a bit different and some are new.
- Updated _libomemo_ submodule to 0.7.1. See the [changelog](https://github.com/gkdr/libomemo/blob/master/CHANGELOG.md) for details.
- Updated _axc_ submodule to 0.3.4. See the [changelog](https://github.com/gkdr/axc/blob/master/CHANGELOG.md) for details.

### Removed
- The `lurch_initialised` setting in the `accounts.xml`.

### BUGFIXES
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants