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

WIP: Rust bindings generation & cmake integration #2826

Merged
merged 8 commits into from Aug 24, 2019

Conversation

PhilippGackstatter
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter commented Jul 24, 2019

This PR adds the elektra-sys crate for the Rust bindings. (See also #2730)
This crate only contains the raw bindings. A separate elektra crate will be added shortly in a future PR.

My goals for this PR are

  • Generate bindings with rust-bindgen during the build phase
  • Integrate cargo (rusts build tool) into the existing CMake build system
    • Building
    • Testing
  • Add some basic tests for the bindings
  • Change docker scripts such that cargo projects can be built

I will look into publishing the crate to crates.io later, once I am closer to the completion of both crates.

Basics

Check relevant points but please do not remove entries.
Do not describe the purpose of this PR in the PR description but:

  • Short descriptions should be in the release notes (added as entry in
    doc/news/_preparation_next_release.md which contains _(my name)_)
    Please always add something to the the release notes.
  • Longer descriptions should be in documentation or in design decisions.
  • Describe details of how you changed the code in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, should be in the commit messages.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

  • I added unit tests
  • I ran all tests locally and everything went fine
  • affected documentation is fixed
  • I added code comments, logging, and assertions (see Coding Guidelines)
  • meta data is updated (e.g. README.md of plugins and METADATA.ini)

Review

Reviewers will usually check the following:

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the build server is happy and also you
    say that everything is ready to be merged.

@PhilippGackstatter PhilippGackstatter changed the title Rust bindings generation & cmake integration WIP: Rust bindings generation & cmake integration Jul 24, 2019
@PhilippGackstatter
Copy link
Contributor Author

Some open questions

  1. Do I only need to generate bindings for the kdb.h header, or are there other ones that I need for the low-level api, without plugin support?
  2. Do I have to change all the docker scripts in order to install rustup (which is used to install cargo and rustc)? I suppose there is no automated way to do that.
  3. Rust-bindgen offers two ways to generate bindings. One is via commandline, which is therefore a manual process and needs to be repeated if anything in the C-API changes. The other is via a build script, which is run every time cargo build runs. That means bindings are regenerated on each build. This is what is currently implemented. However, it requires everyone who uses the bindings to have the necessary headers that elekra needs. I imagine, if someone just installs elektra but did not compile it, he may not meet all the necessary requirements. Maybe it make more sense to regenerate the headers every once in a while, manually, since the C-API doesn't change very much any more?

@PhilippGackstatter PhilippGackstatter mentioned this pull request Jul 31, 2019
17 tasks
@PhilippGackstatter PhilippGackstatter mentioned this pull request Aug 23, 2019
14 tasks
@PhilippGackstatter
Copy link
Contributor Author

Everything passes except for 6 - testmod_dbus (Failed) and 87 - testmod_dbusrecv (Failed). I've already rebased to a working master build. Why is this still failing?

@sanssecours
Copy link
Member

Everything passes except for 6 - testmod_dbus (Failed) and 87 - testmod_dbusrecv (Failed). I've already rebased to a working master build. Why is this still failing?

Those two tests are unreliable on systems with high load (see also issue #2439). I just restarted the failing build job.

@markus2330 markus2330 merged commit 8754078 into ElektraInitiative:master Aug 24, 2019
@markus2330
Copy link
Contributor

Thank you!

@markus2330
Copy link
Contributor

@PhilippGackstatter I hope it is okay that I merged this?

Can you rebase the other PR on top of this?

@PhilippGackstatter
Copy link
Contributor Author

@PhilippGackstatter I hope it is okay that I merged this?

Yes, totally.

Can you rebase the other PR on top of this?

I'll try!

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.

None yet

3 participants