eclipse / paho.mqtt.rust

paho.mqtt.rust
Other
527 stars 102 forks source link

Automatically build the Paho C library #3

Closed fpagliughi closed 5 years ago

fpagliughi commented 6 years ago

Currently, the Rust crate assumes that the proper version of the Paho C library is already built and installed on the system. It would be better if it checked whether this is true or not, and if not, download and build the C library. Alternately, it could always download and build the correct version into the local output directory. This should work across all supported platforms.

The supported Paho C library version is currently v1.2.0 https://github.com/eclipse/paho.mqtt.c/tree/v1.2.0

jnicholls commented 6 years ago

I volunteer for this. I typically prefer a submodule, so in this case, we would submodule the C library and build it through cargo. This will tie us to the appropriate version of the C library that we are compatible with. An environment variable can be provided to cargo to override the location of the C source and even the library itself if someone wants to provide their own library (at their own risk of incompatibility and potentially not even be able to build/link), but that can be a second step and another ticket. Thoughts?

jnicholls commented 6 years ago

Additionally the rabbit hole will grow to want to support different targets, and cross compilation. For example, if someone is building for an x86_64-unknown-linux-musl target for a static binary, the C library will ideally be built against the musl toolchain so it can be linked in.

fpagliughi commented 6 years ago

That would be awesome, @jnicholls! A native build would be more than sufficient for the initial release. But please note that this is an Eclipse project, so in order to contribute you would need to sign the Eclipse ECA (https://www.eclipse.org/legal/ECA.php). If that's a problem, let me know.

siscia commented 6 years ago

Cheers guys!

Justin noticed the issue andare the repo.

I am wondering why you don't simply embed everything into the repo and compile the c library during the build process.

In this way you will control exactly what version of paho.c you are using and it will avoid unpleasant surprises...

Just a suggestion :)

jnicholls commented 6 years ago

Yep, that is the idea I suggested as well Simone, via a submodule and a build.rs process. Frank, that should not be a problem, I'll get the ECA signed. How should I deliver the executed agreement?

On Tue, Jan 9, 2018 at 6:41 AM, Simone Mosciatti notifications@github.com wrote:

Cheers guys!

Justin noticed the issue andare the repo.

I am wondering why you don't simply embed everything into the repo and compile the c library during the build process.

In this way you will control exactly what version of paho.c you are using and it will avoid unpleasant surprises...

Just a suggestion :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eclipse/paho.mqtt.rust/issues/3#issuecomment-356260977, or mute the thread https://github.com/notifications/unsubscribe-auth/AABPq-t9ylvSsFHmUdbntj5KSOYokSFkks5tI1B5gaJpZM4RRoMU .

fpagliughi commented 6 years ago

There's a FAQ at: https://www.eclipse.org/legal/ecafaq.php

It says:

Log into the Eclipse projects forge (you will need to create an account with the Eclipse Foundation if you have not already done so); click on "Eclipse Contributor Agreement"; and Complete the form. Be sure to use the same email address when you register for the account that you intend to use on Git commit records.

siscia commented 6 years ago

Any news on this?

If it is necessary for the release of a stable version I can take a shot to it.

fpagliughi commented 6 years ago

Yes, I would like to have this for a v1.0, although I just let the code sit here for a while to wait for any comments or suggestions. But it is past time to get it rolled up for the Eclipse release this year. I was just starting to look at what other libraries do to see if I could do it myself.

@jnicholls are you still interested in this?

Also: For my own use, anyway, cross-compiling is becoming an important consideration. ARM (RPi, BeagleBone, etc) compile times are horrendous. On that note, I might likely want to get bindgen out of the build as that has a lot of dependencies. I'll raise another ticket for that.

fpagliughi commented 6 years ago

BTW, are there any crates that get this "right", which can serve as an example?

siscia commented 6 years ago

No I don't believe any crates get this right, it really depends on your definition of right and use cases.

I kinda like the approach of rusqlite, they generate the binding via bindgen "manually" and commit them along with the sqlite source file.

In compilation sqlite is compiled using the cc crate.

The object are then stored (you don't recompile everything every single time).

This seems a quite convenient approach here giving all the control to the maintainer of the library. The downside is if the user wants to use a different version of the C library he needs to recompile everything by itself.

fpagliughi commented 6 years ago

@siscia Thanks for pointing me to rusqlite. That looks like a great project to emulate. It does everything. By default it uses the installed library, but lets you compile the library if you want. And cross-compiling works. It uses a pre-generated bindgen file, but lets you regenerate, I believe.

I like it.

But it's a little complex, and, even with help, I would want to take some time to understand it before deploying.

So I think for v1.0 it would be best to go for the simplest working solution: pre-generate the bindings and link against an installed version of the C library. As long as that works for native and cross compiling, I'll be pretty happy.

Then do a a more complex build for v1.1

siscia commented 6 years ago

Folks, I just published a crate, paho_mqtt_c-sys, that should contains everything needed to build everything from scratch.

I didn't test it yet, but if you have more time than me you could try it out.

Here the crates.io link: https://crates.io/crates/paho_mqtt_c-sys

And here the github page without readme: https://github.com/siscia/paho_mqtt_c-sys

fpagliughi commented 6 years ago

@siscia That looks pretty cool. I think we should go a few steps beyond that here, though, particularly after looking at how a number of other projects wrap C libraries, such as rusqlite and some others.

Here are a few things I'd like to strive towards:

1. Get bindgen out of the default build (but leave it as an option)

After doing native builds on some smaller ARM boards (BeagleBone, UDOO Neo, etc), I want to get rid of the huge overhead of bindgen in the build. I'd prefer to use pre-generated bindings, and only use bindgen in the build if specifically requested on a different version of the C library (primarily for forward development, I'm thinking).

2. Use the installed version by default

If the Paho C library is installed and the correct version, just use that. I've found this to be very handy for cross-compiling when you know that the underlying C library is/will-be installed on the target.

3. Rebuild from source when requested

This would use the cmake crate to build the Paho C sources. This would default to the currently-supported library and wouldn't necessarily need to run bindgen.

I'd really like this to work for both native and cross-compiled builds.

4. Let the user specify a different path for the headers and library

This would assume a different version of the C lib and run bindgen with the build. This would be useful for working on the C library in parallel with the Rust one.

What do you all think?

siscia commented 6 years ago

Hi,

  1. in my experience it makes everything a little more cumbersome to have the binding generated by bindgen outside the build process. You don't pay the price at every single iteration. I understand that when you compile natively in low-performance architecture it may be a hassle, but personally I don't feel like it is worth. Also because for those architectures the solution is to cross compile.

  2. I would suggest strongly against this, you really want to control against what you are building, and using your own library allow to do so. Rely on what is already installed in the system is going to make very hard to maintains the rust version.

  3. Should be possible, and already the case. This line here: https://github.com/siscia/paho_mqtt_c-sys/blob/master/build.rs#L39 should indicate that the build process should be re-run if the content of paho.mqtt.c/ change. If that is not the case is some sort of bug.

  4. Can be useful, but is something somehow common? How many times did you felt this need?

Just my two cents!

Cheers, Simone

fpagliughi commented 6 years ago

Thanks, @siscia. I always need to remember that I am biased as a developer and often performing full rebuilds. Due to the way the C library extends structs with each new version, I've been assuming that each Rust release will be strongly tied to a specific C version. Thus the massive overhead of bindgen will always produce the same bindings.rs output each time.

I'm assuming that we'll have it both ways, but perhaps the difference will be in which one is ultimately the default. Maybe we even have it so that the default is to use the cached version for debug builds, and run bindgen for release builds. My thinking is that for debug, you're rebuilding often for development, but for release, you want to be sure that you have everything proper for release.

As for 4., I'm thinking that would be helpful for development. Often I find the need to test against different feature branches of the C lib, and I scatter them in my development tree... or just compiling against the latest develop branch.

Have you used the CMake crate for cross-compiling? Does it work OK?

siscia commented 6 years ago

No unfortunately I have never used the CMake crate, the standard CC crate have always served my needs well enough.

Terkwood commented 5 years ago

Thanks for making this effort!

After struggling to build master branch on Mac OS X and Debian, I was able to build this branch with a very simple cargo build command. Even better, I was able to target armv7-unknown-linux-gnueabihf (I'm running services and an MQTT broker on a Raspberry Pi). Haven't got a chance to actually use any of the logic yet, but this was a big step forward, and I'm excited to continue experimenting.

Greatly appreciated! 🌈 ☀️

fpagliughi commented 5 years ago

When you say "this branch" do you mean the new-build branch? If so, it's not quite done yet, but coming along nicely, thanks to all the input I got on this issue. Hopefully I'll have something in a few days really worth testing. I plan put it up as a pull request and ask you guys to review before merging it to the development branch.

I'll definitely be looking at making it easy to use on an ARMv7, both compiled natively and cross-compiled. At the moment, on my desk, I have an RPi2, BeagleBone Black and Green, UDOO Neo, Nvidia TK1, and (brand new) Pico Pi iMX7... all waiting to give this thing a test drive!

Terkwood commented 5 years ago

When you say "this branch" do you mean the new-build branch?

Yes, I tested new-build branch!

Glad to hear that you have so many devices (toys?!) available for testing. I'll admit I'm a little jealous!

Will keep an eye out for the upcoming PR & for testing requests. Thanks very much!

fpagliughi commented 5 years ago

This is making some more progress. I reconfigured the feature set a little, created default features for "bundled" and "ssl", and got the non-SSL build working (I think). I'll do more testing this week.

It seems that the cmake crate is automatically handling cross-compilation. That's awesome! I don't have SSL dev libraries loaded into my cross-compiler at the moment, but I seem to be able to build for ARMv7 without SSL enabled with this:

cargo build --target=armv7-unknown-linux-gnueabihf --no-default-features --features="bundled" --examples
fpagliughi commented 5 years ago

@jnicholls @siscia @Terkwood I just put up a pull request #23 for the updated build system. I'm sure it still needs some tweaking, but I would love some feedback at this point.

Thanks

Terkwood commented 5 years ago

Great to hear! I'm pretty lazy during the holiday season, but am also motivated to see how this looks. Please bear with me within the temporal continuum. I'll give this a spin 😁

fpagliughi commented 5 years ago

Ha, yeah. I'm the opposite. Since I don't have to work on projects for clients over the holidays, I can pay attention to some real code! So I'm looking to get some stuff done in the next week. I'd like to get this build system done, and a few other issues, then move to an initial "official" Eclipse release. Then immediately start working on futures, websockets, and MQTT v5.

fpagliughi commented 5 years ago

OK. I merged this into the 'develop' branch. Please do check it out and let me know if there are any issues.