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

--crate-type does not override lib properties on manifest #6160

Closed
bltavares opened this issue Oct 10, 2018 · 8 comments
Closed

--crate-type does not override lib properties on manifest #6160

bltavares opened this issue Oct 10, 2018 · 8 comments
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself.

Comments

@bltavares
Copy link

I've faced this recently, and I was surprised by the --crate-type flag behavior. I'm trying to work on a cross-platform library building script to be added on projects on crossgen and it would be really nice if we could contain all the cross-platform bits on the build script.

The Cargo.toml was a default --lib project:

[project]
name = "example"
version = "0.0.1"

[lib]

[dependencies]

When compiling with cargo rustc --lib --release -- --crate-type staticlib I expected this to take over the manifest and add the extra output type. To my surprise, no libexample.a was produced tho.

During the experiments, passing in an invalid crate type as RUSTFLAGS seems to show that cargo is passing multiple --crate-type options down to rustc:

$  RUSTFLAGS="--crate-type example" cargo rustc --lib --release
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___ --print=file-names --crate-type example --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro` (exit code: 1)
--- stderr
error: unknown crate type: `example`

Should the --crate-type override what is defined on cargo manifest? Is this something to add on the cargo rustc argument list instead of the other side of --?

This would help a lot to output different targets, such as C libs and wasm.

There are related issues:

@bltavares
Copy link
Author

This seems to come from this line:

let crate_type_process = process.clone();
const KNOWN_CRATE_TYPES: &[&str] =
&["bin", "rlib", "dylib", "cdylib", "staticlib", "proc-macro"];
for crate_type in KNOWN_CRATE_TYPES.iter() {
process.arg("--crate-type").arg(crate_type);
}

Am I heading to the right direction?

bltavares added a commit to bltavares/crossgen that referenced this issue Oct 10, 2018
This commit contains changes on the generated lib template to build
cross-platform libs. With this changes, I was able to setup a [Travis]
build with GitHub [Releases] of a cross-platform lib.

It is not possible yet to build a different crate type only using
command line args. It requires a modification on `Cargo.toml` to include
new types of library outputs. I've already opened an issue on [cargo](rust-lang/cargo#6160) to see what should be the case here.

Meanwhile, lib authors must change `Cargo.toml` and include the extra
`crate-type` attribute with all the libs. Sadly, this also means that
`LTO` optimisation is not available if the lib uses `lib` or `rlib`
attributes.

To avoid modifying the `Cargo.toml`, the sugestion would be that on a
future PR we could add to the deploy script a modification on the
`Cargo.toml` manifest to include the corresponding crate-type, and only
such crate type for that target. This means we would be able to bring
LTO and output smaller libs.

Realated to:
- yoshuawuyts#11

[Travis]: https://travis-ci.org/bltavares/rust-over-jna-example/builds/439439854
[Releases]: https://github.com/bltavares/rust-over-jna-example/releases/tag/initial
bltavares added a commit to bltavares/crossgen that referenced this issue Oct 10, 2018
This commit contains changes on the generated lib template to build
cross-platform libs. With this changes, I was able to setup a [Travis]
build with GitHub [Releases] of a cross-platform lib.

It is not possible yet to build a different crate type only using
command line args. It requires a modification on `Cargo.toml` to include
new types of library outputs. I've already opened an issue on [cargo](rust-lang/cargo#6160) to see what should be the case here.

Meanwhile, lib authors must change `Cargo.toml` and include the extra
`crate-type` attribute with all the libs. Sadly, this also means that
`LTO` optimisation is not available if the lib uses `lib` or `rlib`
attributes.

To avoid modifying the `Cargo.toml`, the sugestion would be that on a
future PR we could add to the deploy script a modification on the
`Cargo.toml` manifest to include the corresponding crate-type, and only
such crate type for that target. This means we would be able to bring
LTO and output smaller libs.

Realated to:
- yoshuawuyts#11

[Travis]: https://travis-ci.org/bltavares/rust-over-jna-example/builds/439439854
[Releases]: https://github.com/bltavares/rust-over-jna-example/releases/tag/initial

Signed-off-by: Bruno Tavares <connect+github@bltavares.com>
@alexcrichton
Copy link
Member

Thanks for the report! Unfortunately though --crate-type will not work in RUSTFLAGS, because it affects output artifacts Cargo needs to know about it ahead of time, not through RUSTFLAGS.

We could improve the error message here though for sure!

@alexcrichton alexcrichton added the A-diagnostics Area: Error and warning messages generated by Cargo itself. label Oct 10, 2018
@bltavares
Copy link
Author

@alexcrichton thanks for replying.

In regard of the expected behavior for the flags. Should a lib that have not defined a crate-type produce both the rlib and staticlib if I call cargo rustc --release -- --crate-type staticlib?

Nowaday, both type of creates are produced. I would expect that the flags override whatever is defined on Cargo.toml, not combined.

This also means that it is also not possible to pass cargo rustc --release -- --crate-type staticlib -C lto, as if there is a rlib on the build plan the feature gets disable.

What is the expected behavior of the CLI flag?

@alexcrichton
Copy link
Member

@bltavares it's unfortunately basically not expected for manual specification of --crate-type to work at all. Cargo treats rustflags as a black box, and --crate-type is one of those flags which can just cause compilation to go awry in a whole manner of ways.

@bltavares
Copy link
Author

I think it was a mismatch of expectations more than the error message.

From Rust Docs:

If one or more command line flags are specified, all crate_type attributes will be ignored in favor of only building the artifacts specified by command line.

This looks to be applied to rustc only (if I understood it correctly), while I had the expectation to also be applied to cargo rustc.

Should I focus on a workaround editing the Cargo.toml to change the crate-type attribute on those build phases instead of making --crate-type a flag which cargo knows about?

@alexcrichton
Copy link
Member

Unfortunately there's not really a great way forward for something like this, but if editing Cargo.toml works for now that's probably "best", although it's by no measure a great solution

@bltavares
Copy link
Author

Given how hard this would be, I would be ok to close this issue, even tho the error message is confusing.

Would it be ok if I open a PR on Cargo with a note on the docs regarding this behavior?

@alexcrichton
Copy link
Member

Ok sounds good to me! And certainly yeah, PRs improving docs are always appreciated!

bltavares added a commit to bltavares/crossgen that referenced this issue Nov 17, 2018
This commit contains changes on the generated lib template to build
cross-platform libs. With this changes, I was able to setup a [Travis]
build with GitHub [Releases] of a cross-platform lib.

It is not possible yet to build a different crate type only using
command line args. It requires a modification on `Cargo.toml` to include
new types of library outputs. I've already opened an issue on [cargo](rust-lang/cargo#6160) to see what should be the case here.

Meanwhile, lib authors must change `Cargo.toml` and include the extra
`crate-type` attribute with all the libs. Sadly, this also means that
`LTO` optimisation is not available if the lib uses `lib` or `rlib`
attributes.

To avoid modifying the `Cargo.toml`, the sugestion would be that on a
future PR we could add to the deploy script a modification on the
`Cargo.toml` manifest to include the corresponding crate-type, and only
such crate type for that target. This means we would be able to bring
LTO and output smaller libs.

Realated to:
- yoshuawuyts#11

[Travis]: https://travis-ci.org/bltavares/rust-over-jna-example/builds/439439854
[Releases]: https://github.com/bltavares/rust-over-jna-example/releases/tag/initial

Signed-off-by: Bruno Tavares <connect+github@bltavares.com>
bltavares added a commit to bltavares/crossgen that referenced this issue Nov 18, 2018
s command `cargo` subcommand is exclusively intended to be used to help
with [working arround](rust-lang/rust#51009)
how crate types are defined, in order to help with cross-platform
builds.

It is currently [not possible](rust-lang/cargo#6160) to define a
single `crate-type` override on `cargo build`, which causes libs
intended to be used on other languages to compile more than one type of
crate.

This commit adds a dependency on this [new CLI app](https://github.com/bltavares/cargo-crate-type) to be able to workaround the limitations of Cargo.

We are now able to enable the `-C lto` optimization again, as we will
only compile a single type of artifact

Signed-off-by: Bruno Tavares <connect+github@bltavares.com>
yoshuawuyts pushed a commit to yoshuawuyts/crossgen that referenced this issue Dec 18, 2018
* Improve lib template on Travis

This commit contains changes on the generated lib template to build
cross-platform libs. With this changes, I was able to setup a [Travis]
build with GitHub [Releases] of a cross-platform lib.

It is not possible yet to build a different crate type only using
command line args. It requires a modification on `Cargo.toml` to include
new types of library outputs. I've already opened an issue on [cargo](rust-lang/cargo#6160) to see what should be the case here.

Meanwhile, lib authors must change `Cargo.toml` and include the extra
`crate-type` attribute with all the libs. Sadly, this also means that
`LTO` optimisation is not available if the lib uses `lib` or `rlib`
attributes.

To avoid modifying the `Cargo.toml`, the sugestion would be that on a
future PR we could add to the deploy script a modification on the
`Cargo.toml` manifest to include the corresponding crate-type, and only
such crate type for that target. This means we would be able to bring
LTO and output smaller libs.

Realated to:
- #11

[Travis]: https://travis-ci.org/bltavares/rust-over-jna-example/builds/439439854
[Releases]: https://github.com/bltavares/rust-over-jna-example/releases/tag/initial

Signed-off-by: Bruno Tavares <connect+github@bltavares.com>

* Define the type of artifact to be produced by cargo.

s command `cargo` subcommand is exclusively intended to be used to help
with [working arround](rust-lang/rust#51009)
how crate types are defined, in order to help with cross-platform
builds.

It is currently [not possible](rust-lang/cargo#6160) to define a
single `crate-type` override on `cargo build`, which causes libs
intended to be used on other languages to compile more than one type of
crate.

This commit adds a dependency on this [new CLI app](https://github.com/bltavares/cargo-crate-type) to be able to workaround the limitations of Cargo.

We are now able to enable the `-C lto` optimization again, as we will
only compile a single type of artifact

Signed-off-by: Bruno Tavares <connect+github@bltavares.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself.
Projects
None yet
Development

No branches or pull requests

2 participants