Update bindgen to version 0.56

[decathorpe]

This update does not require other code changes.

[ionut-arm]

Hi, thanks for the patch!

It seems the new version of bindgen pulls in a new version of clang-sys that in turn pulls in a libloading version that needs nightly enabled (which we don't support officially). Thus the Ubuntu build fails. I'm curious why the Fedora one passes, does Fedora implicitly run everything with nightly? That might lead to some differences between the two. CC: @puiterwijk

Also, we need the commits to be signed, as described in the Contributing guidelines

[decathorpe]

I don't know why this leads to problems on ubuntu. Fedora ships both libloading 0.5 and 0.6, and both are fine without nightly (which is not even available for RPM builds, since only the stable Rust compiler is packaged), and clang-sys 1.0.3 builds fine with it as well, and even passes its test suite now.

Also, we need the commits to be signed

I will add the Signed-off-by.

[decathorpe]

Oh, it looks like the Rust compiler version on ubuntu is just too old: The #[non_exhaustive] attribute has been available on stable Rust since 1.40.0, released in Dec 2019: https://blog.rust-lang.org/2019/12/19/Rust-1.40.0.html

[ionut-arm]

Ah, that makes sense... We'll need to decide what to do about this, if we just disregard older versions of the compiler

[decathorpe]

It's weird though. It looks like various Rust versions are installed with rustup during the CI tests, and the last one is 1.38.0-x86_64-unknown-linux-gnu installed - rustc 1.38.0 (625451e37 2019-09-23) (which is really old, and too old to have the new attribute).

EDIT: I found where this happens: https://github.com/parallaxsecond/rust-tss-esapi/blob/master/tests/all-ubuntu.sh#L45
Why do you need to test old "legacy" toolchains like that?

[ionut-arm]

We decided to check that the library can be built with "legacy" compilers to maintain some backwards compatibility. Otherwise we run the risk - as exhibited here - that someone down the line who really needs or wants an old compiler (for some reason) cannot update past a certain version.

The thing is that we didn't have a particular version to tie ourselves too. We have to decide whether we want to stay compatible with 1.38.0 or move to a newer version. I'm ambivalent about 1.38.0, but I definitely think we should have some legacy support. Maybe a "sliding window" of supported versions - current version + X previous versions - would work better. Also maybe there's some community guidance on this for Rust projects.

[decathorpe]

The thing is that we didn't have a particular version to tie ourselves too. We have to decide whether we want to stay compatible with 1.38.0 or move to a newer version. I'm ambivalent about 1.38.0, but I definitely think we should have some legacy support. Maybe a "sliding window" of supported versions - current version + X previous versions - would work better. Also maybe there's some community guidance on this for Rust projects.

Yeah, this is usually called the MSRV (minimum supported Rust version) by projects. Depending on the project, I found values as low as 1.13 (in libc) or as high as "only the three most recent stable releases are supported", which seems to be the most common approach, as far as I can tell.

[ionut-arm]

Yeah, this is usually called the MSRV (minimum supported Rust version) by projects. Depending on the project, I found values as low as 1.13 (in libc) or as high as "only the three most recent stable releases are supported", which seems to be the most common approach, as far as I can tell.

Nice, that sounds like something we should have, a MSRV policy. It's relevant for all the parallaxsecond repos, but it's worth debating whether the same policy should be applied to all projects.

Something like this would be good to have

[hug-dev]

Hey 👋 !

One delicate thing with updating bindgen as well is that so many crates use it and because it links with clang-sys and you can only have one version of that, you can easily end-up with conflicts if two dependencies of your tree use a different version of bindgen 😢
Ideally, as described in #85, we would not even use bindgen as build time to solve these kind of problems.

I would agree otherwise with something less restrictive for the versions we support, the Embedded WG has updated their MSRV policy by only requiring building on current stable.

[decathorpe]

One delicate thing with updating bindgen as well is that so many crates use it and because it links with clang-sys and you can only have one version of that, you can easily end-up with conflicts if two dependencies of your tree use a different version of bindgen

I wonder, why is it only possible to have one version of clang-sys? Is it special somehow?

[hug-dev]

*-sys crate like clang-sys usually have their links field in their Cargo.toml pointing to the library on the system they link to.

This field is used by Cargo to make sure that there are not two packages linking to the same library, to prevent duplicate symbols.

I don't think this problem should block this PR if you need the new version of bindgen for your own work though!

[decathorpe]

It did not. I have already updated all fedora packages to use clang-sys 1.0.3 and bindgen 0.56, and submitted pull requests like this to the all the affected projects :)

[ionut-arm]

Hey 👋 !

One delicate thing with updating bindgen as well is that so many crates use it and because it links with clang-sys and you can only have one version of that, you can easily end-up with conflicts if two dependencies of your tree use a different version of bindgen 😢
Ideally, as described in #85, we would not even use bindgen as build time to solve these kind of problems.

I would agree otherwise with something less restrictive for the versions we support, the Embedded WG has updated their MSRV policy by only requiring building on current stable.

Maybe time has come to generate the bindings and commit them in the tree in a separate, -sys crate 🤓

[ionut-arm]

Ok, an update - it seems reasonable to stick with current stable and drop all backwards compatibility checks, as it should be trivial for users to update rustc on their platforms. If other requirements arise from parties interested in the crate, we could always pick up the convo again.

Could you please remove the 1.38.0 check from the Ubuntu script?

[ionut-arm]

Closing this as I've created a new PR (#159) which uses the commit here with another commit to fix things on top.