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

Find a way to update serde_derive #203

Closed
eqrion opened this issue Aug 29, 2018 · 13 comments
Closed

Find a way to update serde_derive #203

eqrion opened this issue Aug 29, 2018 · 13 comments

Comments

@eqrion
Copy link
Collaborator

eqrion commented Aug 29, 2018

From #159

I thought we had an issue filed with exactly what is going on here, but we don't exactly. I'll summarize it here for reference and file an issue later.

cbindgen depends on syn which depends on proc-macro-2 because syn is geared to be used in proc-macros not random binaries for parsing rust code. proc-macro-2 is the problematic library that causes this crash because it depends on internal compiler libs.

cbindgen doesn't need any of the functionality from syn that uses proc-macro-2 though, so we had a feature added to syn called proc-macro that we could use to disable this dependency. But disabling that feature is not enough to prevent proc-macro-2 from being linked.

Unfortunately proc-macro-2 is used by serde_derive as a build time proc-macro crate, and cargo does feature flag resolution once for build dependencies and binary dependencies. So our feature flag gets overridden because serde_derive needs that flag enabled. Even though serde_derive only happens at compile time, and we want the feature disabled at run time.

But not all serde_derive's have this dependency. Which is why we pin to a version which doesn't yet have it.

That's what rust-lang/cargo#2589 is about, and why it's still relevant. If that's fixed we could use the feature flag and update serde_derive.

I don't think we could easily drop the serde_derive requirement either. It's used for parsing cbindgen.toml which isn't required if all you care about is build.rs. But it is used for parsing Cargo.lock which is required for most uses of the crate.

This has been a major pain, and I'd love to find an answer to it.

@cuviper
Copy link
Contributor

cuviper commented Sep 10, 2018

I've updated the pinned version to 1.0.58 in #207, but the core problem is not resolved.

Would you be open to a PR that manually deserializes the necessary structs in cbindgen? I know there are quite a few, but I'm somewhat inclined to just see how it goes...

@sylvestre
Copy link
Contributor

@eqrion @cuviper This issue is preventing us to upload cbindgen in Debian & Ubuntu.
We have serde_derive 1.0.70 in Debian and it would be tricky for us to backport 1.0.58

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 15, 2018

@sylvestre I wrote #222 to drop the serde_derive dependency using a hack. Hopefully this resolves the issue for you.

@infinity0
Copy link

What's wrong with relaxing the dependency to [dependencies.serde_derive] version = 1.0? I couldn't figure this out from the huge explanation above.

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 17, 2018

What's wrong with relaxing the dependency to [dependencies.serde_derive] version = 1.0? I couldn't figure this out from the huge explanation above.

If we have a dependency on proc-macro-2 with feature 'proc-macro' enabled, the cbindgen binary will crash because it can't find compiler libraries. We have a dependency on syn which uses 'proc-macro-2' with that feature disabled, so we're fine.

If we update serde_derive, it will add a dependency on 'proc-macro-2' with that feature enabled causing the linked version of 'proc-macro-2' to enable that feature causing us to crash. serde_derive only needs this at build time, but cargo doesn't generate separate dependency graphs for build dependencies and runtime dependencies., so the features are unioned together.

@infinity0
Copy link

the cbindgen binary will crash because it can't find compiler libraries.

why does it crash, and can we make it not crash?

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 17, 2018

It crashes because libproc-macro (a rustc internal library) is added as a linker dependency, and cbindgen can't find it when run outside of cargo run.

% target/debug/cbindgen -h
target/debug/cbindgen: error while loading shared libraries: libproc_macro-76e776d93c7dc9f0.so: cannot open shared object file: No such file or directory
%

There's no guarantee that the rustc libraries should even exist to run cbindgen either.

@infinity0
Copy link

infinity0 commented Oct 18, 2018

I see, I think we can probably work around it in Debian since it's possible for us to keep multiple versions of the rustc internal libs around on the system. We already ship rust internal libs in /usr/lib so the linker can pick it up at runtime without any extra flags:

$ ls -gG /usr/lib/x86_64-linux-gnu/libproc_macro-f1cfd9a3f1fc2e7c.so 
-rw-r--r-- 1 556464 Sep 23 10:16 /usr/lib/x86_64-linux-gnu/libproc_macro-f1cfd9a3f1fc2e7c.so

Then the rust-cbindgen package will automatically get a Depends: libstd-rust-X.YY which can be co-installed with any future rustc compiler libs. Of course we'd want to do a rebuild against the newest rustc, but things will remain working until we do.

@sylvestre TL;DR I'm pretty sure things with Just Work if we relax the dependency to 1.0 in Debian.

@infinity0
Copy link

infinity0 commented Oct 18, 2018

cbindgen$ cargo build
 Downloading serde_derive v1.0.80
[..]
   Compiling serde_derive v1.0.80
[..]
   Compiling cbindgen v0.6.4 (file:///home/infinity0/var/lib/rust/debcargo-build/cbindgen)
    Finished dev [unoptimized + debuginfo] target(s) in 44.44s
$ ldd -r target/debug/cbindgen
	linux-vdso.so.1 (0x00007ffdc812f000)
	libproc_macro-f1cfd9a3f1fc2e7c.so => /usr/lib/x86_64-linux-gnu/libproc_macro-f1cfd9a3f1fc2e7c.so (0x00007f102f6e6000)
	libsyntax-7c988e0bb240f29d.so => /usr/lib/x86_64-linux-gnu/libsyntax-7c988e0bb240f29d.so (0x00007f102f2c8000)
[.. etc ..]
$ dpkg-shlibdeps target/debug/cbindgen
dpkg-shlibdeps: warning: can't extract name and version from library name 'libproc_macro-f1cfd9a3f1fc2e7c.so'
[..]
dpkg-shlibdeps: warning: can't extract name and version from library name 'libstd-adf5983c479e1d65.so'
dpkg-shlibdeps: warning: binaries to analyze should already be installed in their package's directory
dpkg-shlibdeps: warning: package could avoid a useless dependency if target/debug/cbindgen was not linked against libm.so.6 (it uses none of the library's symbols)
$ cat debian/substvars 
shlibs:Depends=libc6 (>= 2.14), libgcc1 (>= 1:3.0), libstd-rust-1.29 (>= 1.29.0+dfsg1)

Yeah this is totally fine. The dependency on libstd-rust-1.29 will keep this package around on the system even if the user installs rustc 1.30. We don't even need to modify any packaging files, on top of the patch to relax the dependency from =1.0.58 to 1.0.

@konstin
Copy link
Contributor

konstin commented Dec 1, 2018

It looks like this has been fixed with the latest nightly through rust-lang/rust#49219 🎉. This is the output after updating both serde_derive and syn:

$ cargo +stable build
    Finished dev [unoptimized + debuginfo] target(s) in 0.91s                                                                                                                
$ ldd target/debug/cbindgen
	linux-vdso.so.1 (0x00007ffc4766e000)
	libproc_macro-aeedf6e3b1dd6762.so => not found
	libstd-a021829e87e39dcf.so => not found
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa08f23d000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa08f025000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa08ec34000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa08ff71000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa08e896000)
$ rustc +nightly -V
rustc 1.32.0-nightly (d09466ceb 2018-11-30)
$ cargo +nightly build
    Finished dev [unoptimized + debuginfo] target(s) in 0.78s
$ ldd target/debug/cbindgen
	linux-vdso.so.1 (0x00007ffd1b5f4000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f8f544b9000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f8f542b1000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f8f54092000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f8f53e7a000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f8f53a89000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f8f55289000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f8f536eb000)

@vadixidav
Copy link

Now that they fixed that, can we just use the latest serde_derive? I will try forking and if it works fine I'll send a PR.

@cuviper
Copy link
Contributor

cuviper commented Dec 7, 2018

May want to wait for that to hit stable -- it's currently in the beta branch headed for 1.32.

vadixidav pushed a commit to vadixidav/cbindgen that referenced this issue Dec 7, 2018
@vadixidav
Copy link

I ran into an issue where I couldn't actually build due to the version restrictions. For now I'll use my fork, which seems to be working for me on the latest nightly.

nbigaouette-eai added a commit to nbigaouette-eai/restson-rust that referenced this issue Jan 13, 2019
A conflict with cbindgen exists which prevent restson to be used
with it. `cbindgen` [pins `serde_derive` to `1.0.58`](https://github.com/eqrion/cbindgen/blob/44e9b2112a06e46ddda6073e237d3a56df39c3e2/Cargo.toml#L24-L27)
due to mozilla/cbindgen#203
so depending on `^1.0.80` prevents using both crates together.

This patch simply relaxes the dependency on `serde_derive` to `1.0`,
which is API compatible with both `1.0.80` and `1.0.58`.

Closes spietika#14
spietika pushed a commit to spietika/restson-rust that referenced this issue Jan 14, 2019
A conflict with cbindgen exists which prevent restson to be used
with it. `cbindgen` [pins `serde_derive` to `1.0.58`](https://github.com/eqrion/cbindgen/blob/44e9b2112a06e46ddda6073e237d3a56df39c3e2/Cargo.toml#L24-L27)
due to mozilla/cbindgen#203
so depending on `^1.0.80` prevents using both crates together.

This patch simply relaxes the dependency on `serde_derive` to `1.0`,
which is API compatible with both `1.0.80` and `1.0.58`.

Closes #14
@Mrmaxmeier Mrmaxmeier mentioned this issue Jan 18, 2019
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 a pull request may close this issue.

6 participants