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

Rust Bindings #1891

Open
DaAwesomeP opened this issue Aug 4, 2023 · 4 comments
Open

Rust Bindings #1891

DaAwesomeP opened this issue Aug 4, 2023 · 4 comments

Comments

@DaAwesomeP
Copy link
Member

I'm starting to look into some new projects in Rust. I'm willing to work on and contribute the bindings, but there are some things to think about:

  1. The bindings need to live Cargo, the Rust package manager, as a primary method of installation. This also handles hosting documentation at https://doc.rust-lang.org. You can bundle dependencies in the same way that we currently ship/install the C++ and Python bindings, but this is sort of an anti-pattern in Rust (with the exception being OS distros that need to provide Rust dependencies as a part of other packages but it is typically NOT intended for code compilation for the end user).
  2. The above means that the OLA version may be decoupled from the Rust bindings version. I propose establishing an API version that can be queried from OLA. The bindings would then check if the OLA they are communicating with is supported by the binding version. This would also significantly help with Make the Python API available on PyPi #1451 where similarly the Python bindings could be decoupled if published to Pypi. I will make a separate issue/initial pull for this. Another nice consequence of this is possibly documentation about the underlying socket API that would allow for any binding to be created more trivially in the future.
  3. The above means that possibly there should be a separate repo for the Rust bindings since they wouldn't necessarily be tied to an individual OLA version/OLA API version. It would be strange also (in my opinion) for the Rust binding version to match the OLA version if it supports multiple OLA versions (we can limit it possibly to the last 1 or 2 major/minor versions released or found in popular distros (i.e. all of 0.10.x) for now.
  4. Because of Rust's recompilation/static linking requirements, LGPL may not be the best license. It is not necessarily always possible to replace the binary components of a Rust library with another in the same way you can with C/C++ shared libraries. It seems that MPL is the closest (copyleft). Apache 2.0 and MIT are also popular (but not copyleft).
  5. This should be a native binding, not a C++/shared library wrapper. There are some threading and other bits in the C++ API that don't make sense to wrap in Rust and should be written in Rust. Since it will be very separate code (working at the socket level and not a shared library) it further makes sense to be a separate repo (in my opinion).

I think I will probably start this at some point in my own repo and then I would be happy to transfer ownership or fork to another repo once I have something usable.

@DaAwesomeP
Copy link
Member Author

Existing libraries:

  • jbellerb/libola-rs: Uses the ola package name in Cargo, LGPL, appears to depend on the OLA protobuf definitions as a Git submodule (so each binding version is tied to a specific OLA version)
  • jacobh/roller: Apache 2.0, appears to have copied in a specific version of the OLA protobuf definitions

cc @jbellerb @jacobh

@jbellerb
Copy link

jbellerb commented Aug 5, 2023 via email

@DaAwesomeP
Copy link
Member Author

all OLA protocol messages have a version header, so theoretically there's no need to tie any client library to a specific OLA version (while I include a fairly recent OLA as a submodule, the actual files I'm referencing haven't changed in 5 years!).

That's a good point! I guess we don't really need to worry about that then unless @peternewman sees any imminent changes.

Even though the library would be decoupled from OLA, I don't really see a reason why a Rust client couldn't live in the main repo along with the rest of the other language clients, especially since older versions would still be available on Crates.io.

One thing I really want to avoid is having it built with Autotools/Automake and installed with make install since that isn't really how it should be installed (it should use Cargo and installed by project, not system-wide). If we put it in this repo I think we may be obligated to build with Autotools.

Since it is built separately the version tags don't line up. The whole OLA repo is tagged at one time. So if there is an update to the Rust binding then we have to increase the tag of the OLA version even though the changes won't touch it. Similarly if there is an update to OLA then the Rust bindings will increase in version without any changes.

I think the versioning also could be confusing for the end-user. Since the proto file hasn't changed in so long, users will probably always want the latest Rust bindings regardless of the underlying OLA version. A newer binding version would probably contain bugfixes or enhancements that work with all OLA versions. However, if the version numbers match then a user will probably tend to try to match the versions and install an older Rust binding version (which is not necessarily correct here, especially if they installed OLA from a distro with a much older version of OLA).

Same with the licencing (and I'm cool with relicensing if needed).

I think we first would need to explore re-licensing the Proto definitions file. I think until we change that all code that compiles it in must be LGPL.

My tendency to move it to MPL (specifically for that one Proto file and the Rust binding) is simply from looking at installations using OLA that I have built/been commissioned for so far. I don't think I would be able to complete the same projects with a Cargo-built OLA statically-linked Rust library where I have used the OLA C++ shared library in the past (this is a licensing limitation, not a technical one).

I like tokio, I made the client I intend people to use over tokio (tokio optional features flag, personally prefer features over separate crates), although adding support for another runtime shouldn't be much work since almost all the code can be shared.

To be honest I am just getting into Rust and I haven't looked too thoroughly at your specific implementation. From what I have researched and looked at so far Tokio does seem to be the way to go (as you have implemented it in your repo). My main concern is just that the bindings should use Tokio or another Rust-native method instead of simply wrapping the C++ shared library/bindings. The C++ API contains some bits that would be difficult to integrate well into Rust.

@yoe
Copy link
Contributor

yoe commented Aug 20, 2023

  1. Because of Rust's recompilation/static linking requirements, LGPL may not be the best license. It is not necessarily always possible to replace the binary components of a Rust library with another in the same way you can with C/C++ shared libraries. It seems that MPL is the closest (copyleft). Apache 2.0 and MIT are also popular (but not copyleft).

I'm not sure I understand this (perceived) requirement. Can you clarify?

I don't believe the LGPL requires that it is possible to replace a library; it merely states that if you use a LGPL library from a program (as opposed to from another library), then the parts of the GPL that apply to availability of source etc do not apply to you (that is, you're not required to provide source code and your code is not GPL). It does not, however, state how an application links to the library; it mentions an "interface" without defining that, and a "combined work" which is a work that uses the interface combined with the library.

So libraries that do static linking can be covered under the LGPL in my reading.

Even though the library would be decoupled from OLA, I don't really see a reason why a Rust client couldn't live in the main repo along with the rest of the other language clients, especially since older versions would still be available on Crates.io.

One thing I really want to avoid is having it built with Autotools/Automake and installed with make install since that isn't really how it should be installed (it should use Cargo and installed by project, not system-wide). If we put it in this repo I think we may be obligated to build with Autotools.

Not necessarily; it is certainly possible to EXTRA_DIST the rust code (which would mean it would get shipped with the source tarball without being compiled by a make command in any way). Whether that's a good idea is, of course, a different question ;-)

Since it is built separately the version tags don't line up. The whole OLA repo is tagged at one time. So if there is an update to the Rust binding then we have to increase the tag of the OLA version even though the changes won't touch it. Similarly if there is an update to OLA then the Rust bindings will increase in version without any changes.

I think the versioning also could be confusing for the end-user. Since the proto file hasn't changed in so long, users will probably always want the latest Rust bindings regardless of the underlying OLA version. A newer binding version would probably contain bugfixes or enhancements that work with all OLA versions. However, if the version numbers match then a user will probably tend to try to match the versions and install an older Rust binding version (which is not necessarily correct here, especially if they installed OLA from a distro with a much older version of OLA).

Definitely agree with all of that.

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

No branches or pull requests

3 participants