Bug 1608670 - cbindgen is not installable due to broken deps on libproc_macro.so
Summary: cbindgen is not installable due to broken deps on libproc_macro.so
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: rust-cbindgen
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Igor Raits
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-07-26 06:31 UTC by Igor Raits
Modified: 2018-09-25 21:15 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-07-28 10:07:10 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Igor Raits 2018-07-26 06:31:31 UTC
⋊> ~/P/f/r/rust-cbindgen on master ⨯ sudo dnf install x86_64/cbindgen-0.6.1-1.fc29.x86_64.rpm                                                                                                              08:27:53
[sudo] password for brain: 
Last metadata expiration check: 0:42:35 ago on Thu 26 Jul 2018 07:45:28 AM CEST.
Error: 
 Problem: conflicting requests
  - nothing provides libproc_macro-9d1587d1f15e5104.so()(64bit) needed by cbindgen-0.6.1-1.fc29.x86_64

I am not sure how we can fix this.

Comment 1 Igor Raits 2018-07-26 06:32:18 UTC
Josh, even it is unstable ABI, can we add it to Provides?

Or do you have an idea how to get rid out of this linking?

Comment 2 Josh Stone 2018-07-26 17:39:05 UTC
(In reply to Igor Gnatenko from comment #1)
> Josh, even it is unstable ABI, can we add it to Provides?

As we've been chatting on IRC, I really don't want to do this...  The sonames will change with every 6-week release, plus interim point releases, and even within a release I can't promise that some bugfix patch won't change the exported symbols.

> Or do you have an idea how to get rid out of this linking?

If the things which need cbindgen can just use it as a build-dep crate, then we don't need to ship the cbindgen executable at all.  Otherwise, I think we need upstream (cbindgen and/or cargo) to find a better way to separate the syn dependencies built with and without proc-macro linking.

Comment 3 Colin Walters 2018-09-05 18:31:25 UTC
> If the things which need cbindgen can just use it as a build-dep crate, then we don't need to ship the cbindgen executable at all.

Hum, like execute it out of a `build.rs`?  

This is for https://github.com/projectatomic/rpm-ostree/pull/1516

Comment 4 Igor Raits 2018-09-05 19:06:11 UTC
(In reply to Colin Walters from comment #3)
> > If the things which need cbindgen can just use it as a build-dep crate, then we don't need to ship the cbindgen executable at all.
> 
> Hum, like execute it out of a `build.rs`?  

I believe so.

Comment 5 Josh Stone 2018-09-05 19:18:08 UTC
(In reply to Colin Walters from comment #3)
> > If the things which need cbindgen can just use it as a build-dep crate, then we don't need to ship the cbindgen executable at all.
> 
> Hum, like execute it out of a `build.rs`?  

Yes, using the API instead of the command line.

https://docs.rs/cbindgen/

Comment 6 Colin Walters 2018-09-05 20:32:31 UTC
Hum, but the point of this is for the *outer* C layer to see the inner Rust library.  If I added this as a build.rs step to the rust lib, it'd need to be built before building the main rust lib, even though it's not used by that.

(Although now that I try it that's not really a big deal, because 95% of the time is spent compiling dependencies of both, not the final library)

That said, it may be better to go to Cargo workspaces.

Either way, adding it to our cargo-based deps clearly solves the vendoring issue we have (there's no rust-cbindgen in epel), so I'll pursue that.

Comment 7 Colin Walters 2018-09-05 20:49:27 UTC
Although, on the flip side, I'd rather not build cbindgen myself - if I used cargo workspaces it's going to require that my app's dependencies are compatible with its dependencies (though they are today).

I guess I could avoid workspaces and just add it as src/cbindgen, but then I'll have to teach our vendoring tools how to add those sources too to our vendored tarball.

I'm missing some context for the original issue...does this link to some compiler internals or so?  Did the option come up of shipping it with our rust package come up?

Comment 8 Josh Stone 2018-09-05 21:40:19 UTC
(In reply to Colin Walters from comment #7)
> I'm missing some context for the original issue...does this link to some
> compiler internals or so?

It does, though unintentionally.  It uses syn without the proc-macro feature, which means it should not need to link to internal compiler libraries.  But then serde_derive is in its dependency tree, which uses syn to implement a #[derive(...)] plugin, so it does enable the proc-macro feature.  Since features are additive, cbindgen gets dragged into linking this way too.

It's normally fine for derive plugins to use libproc_macro, because they're only built and used transiently at compile time.  The problem is that this escapes to the final artifacts in the cbindgen executable.

Upstream cbindgen sort of solved this by pinning serde_derive to an older version that uses older syn.  Only the older syn gets compiled with the proc-macro feature enabled, and the newer syn used by cbindgen can compile without that feature, thus avoiding the linking issue.  Igor patched that pinned version out though, so we could build with just the latest serde_derive.

> Did the option come up of shipping it with our rust package come up?

That's possible, I suppose, though I'm not keen to increase that scope.  We do have to ship rustfmt and rls with the compiler, and will soon have clippy too, because they really do need to link directly to compiler libraries.  But those tools are all part of the rustc-src tarball already.

Another possibility for you is to just generate that C header as part of your release process upstream -- similar to how projects often provide bison/yacc generated files in release tarballs.

Comment 9 Colin Walters 2018-09-06 13:38:33 UTC
> Another possibility for you is to just generate that C header as part of your release process upstream -- similar to how projects often provide bison/yacc generated files in release tarballs.

Yes.  Though personally, I find things like that to be an evil trap when one needs to patch the downstream package.  Then one gets confused why changes aren't taking effect...

There's a reason the GPL talks about "preferred form of modification"...which is definitely not tarballs with pre-built stuff and spec files ;)

Anyways, we ended up doing:

https://github.com/projectatomic/rpm-ostree/pull/1516/commits/11224713d04eba3a56c7e56e76c52d55cb6dd18c

Comment 10 Robert-André Mauchin 🐧 2018-09-14 01:50:47 UTC
Can this be solved? cbindgen is needed to build the latest firefox-nightly.

Comment 11 Josh Stone 2018-09-14 17:08:30 UTC
We're considering shipping an older/compat version of serde_derive, so cbindgen can use it's "pinning" trick to separate the proc-macro feature enablement.  I also have a PR to bump that pinned version to something a bit newer:
https://github.com/eqrion/cbindgen/pull/207

Comment 12 Josh Stone 2018-09-17 16:27:59 UTC
Igor added compat-rust-serde_derive and used it in rust-cbindgen-0.6.3-2.fc30, so now we have the cbindgen executable again, independent of compiler libs.  Thanks!

That's for rawhide -- if folks need it for F29, let us know.  F28 still has cbindgen-0.4.3 from before this became a problem at all.

Comment 13 Josh Stone 2018-09-18 18:27:32 UTC
I went ahead and built the update for F29 too:
https://bodhi.fedoraproject.org/updates/FEDORA-2018-74f5272e60


Note You need to log in before you can comment on or make changes to this bug.