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

lib with multiple crate types appears to override profile.release #2301

Closed
sjudd opened this issue Jan 21, 2016 · 9 comments
Closed

lib with multiple crate types appears to override profile.release #2301

sjudd opened this issue Jan 21, 2016 · 9 comments

Comments

@sjudd
Copy link

sjudd commented Jan 21, 2016

I have a crate with the following in Cargo.toml:

[lib]
crate_type = ["staticlib", "rlib"]

[profile.release]
lto = true

The static library it produces is around 4.4mb.

If remove rlib:

[lib]
crate_type = ["staticlib"]

[profile.release]
lto = true

The static library is around 1.3 mb.

It seems like building both an rlib and a static lib overrides profile.release? Or at least lto?

I'm trying to build both a static lib and an rlib because I want to build both a static library and a binary for the crate and the binary depends on the library.

@alexcrichton
Copy link
Member

Ah this is because the compiler only accepts -C lto for the staticlib output so Cargo will omit passing the -C lto flag if an rlib is requested.

There could likely be a warning here, however.

@sjudd
Copy link
Author

sjudd commented Jan 21, 2016

Just to double check, it seems like what you'd want is -C lto to be used when generating the staticlib and ignored (with a warning) when generating the rlib?

Again just to be clear, what happens now is that -C lto is used to build neither the rlib nor the staticlib when both are requested, but is used to build the staticlib when just the staticlib is requested.

@sjudd
Copy link
Author

sjudd commented Jan 21, 2016

Ah and in fact it looks like there's an explicit check that matches only [staticlib] here:

pub fn can_lto(&self) -> bool {
.

If this isn't intentional I'd be willing to take a look at fixing/changing this, though I'm still relatively new to Rust.

@alexcrichton
Copy link
Member

Yeah this is currently intentional and expected behavior. The compiler is only invoked once despite any number of crate types, and the compiler will reject rustc -C lto --crate-type rlib --crate-type staticlib because it can't perform LTO on an rlib.

To add support for this it'd likely want to start upstream by fixing the compiler to not reject this situation.

@SSheldon
Copy link

SSheldon commented Jun 22, 2016

This is also a problem with the new cdylib crate type; LTO can be performed on cdylibs but will be disabled if you build an rlib.

Looks like LTO will still be performed though with crate_type = ["staticlib", "cdylib"].

@alexcrichton any suggestions for a workaround here? Our use case is that we wanted to write integration tests for our crate, which requires that it be an rlib, but then adding rlib as a crate type disabled LTO. Workarounds I've come up with:

  • duplicate the Cargo.toml, with one version for integration testing that compiles rlib and one that compiles to cdylib
  • split the cdylib layer into another crate which imports the original crate so it can be pure-rust; then we can write integration tests on that pure rust crate

@alexcrichton
Copy link
Member

I believe this is a compiler limitation, you can't -C lto when there's an rlib in play:

$ rustc foo.rs --crate-type rlib --crate-type staticlib -C lto
error: lto can only be run for executables and static library outputs

I'd probably recommend splitting up into a separate crate so you can write tests for one and LTO the other

@carols10cents
Copy link
Member

Closing this since the fix first needs to be made in rust-lang/rust and there's a workaround.

@Lymia
Copy link

Lymia commented Oct 12, 2017

I ran into this issue independently, and it took a bit of work to actually track down the root cause. Even if there needs to be an upstream fix first, you can add a warning in the meantime.

@carols10cents
Copy link
Member

hi @Lymia, could you open a new issue for adding a warning please? thanks!

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

5 participants