10

Cargo `--crate-type` CLI Argument by seanmonstar · Pull Request #3180 · rust-lan...

 2 years ago
source link: https://github.com/rust-lang/rfcs/pull/3180
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Copy link

Contributor

seanmonstar commented on Oct 7

edited by ehuss

Add the ability to provide --crate-type <crate-type> as an argument to cargo build. This would have the same affect of adding crate-type in the Cargo.toml, while taking higher priority than any value specified there.

Rendered

Copy link

stevelr commented on Oct 9

Please document the commands line syntax to specify multiple values (a subset of the list values in Cargo.toml)

Is it
--crate-type cdylib,rlib
or
--crate-type cdylib --crate-type rlib
?

Copy link

stevelr commented on Oct 9

edited

Wondering out loud: this might be the hook needed for 'cargo test' to be able to run #tests in source files in a wasm32 project.

The developer might need to add --target in addition to --crate-type to build the tests for the local platform, but it would be nice to be able to use 'cargo test' in wasm projects.

Copy link

Member

joshtriplett commented on Oct 19

I think this should work for cargo install as well: it makes sense to do cargo install --crate-type staticlib --root somedir to install a .a file and subsequently link to it.

Copy link

Member

joshtriplett commented on Oct 19

We talked about this in today's @rust-lang/cargo meeting, and we can understand where this is coming from, but we'd like to understand why it wouldn't suffice to simply have a separate crate exporting the cdylib/staticlib interface. If that's possible, we would prefer that approach. If that wouldn't suffice for your use case, we'd like to understand that aspect of your use case in more detail.

Copy link

Member

joshtriplett commented on Oct 19

One further thought from the meeting: it may make sense to restrict this only to cargo rustc, which already has the ability to apply to only one crate (e.g. if there's a lib and multiple bins in the same crate).

Copy link

GuilhermeWerner commented on Oct 19

edited

We talked about this in today's @rust-lang/cargo meeting, and we can understand where this is coming from, but we'd like to understand why it wouldn't suffice to simply have a separate crate exporting the cdylib/staticlib interface. If that's possible, we would prefer that approach. If that wouldn't suffice for your use case, we'd like to understand that aspect of your use case in more detail.

I believe that having to create two packages just to change the crate type is not very ergonomic.

For example, I work on a mobile project where a lib in rust is compiled for android and ios, ie a cdylib for android and a staticlib for ios. What we do is declare the lib as crate-type = ["cdylib", "staticlib"], however this is not ideal, as it generates a .a that will not be used on android, and generates a warning for ios, as that cdylibs are not supported in the platform.

One further thought from the meeting: it may make sense to restrict this only to cargo rustc, which already has the ability to apply to only one crate (e.g. if there's a lib and multiple bins in the same crate).

Currently passing --crate-type to cargo rustc doesn't overwrite what's in Cargo.toml, it just adds one more crate-type to the rustc invocation.

It would be nice if the behavior was changed to overwrite what is in Cargo.toml

Copy link

Contributor

Author

seanmonstar commented on Oct 20

we'd like to understand why it wouldn't suffice to simply have a separate crate exporting the cdylib/staticlib interface.

So, in hyper's case (just 1 of the cases in the motivation), the exposed C API uses private implementation details. Shifting it to a separate crate would mean parts of the API could only depend on public details (for example, currently it can check the internal error kind, but publicly I suppose it would need to depend on the unstable Display format?). There are also a couple features exposed in the C API that we figured out at a low level, but haven't settled on how to expose them in idiomatic Rust.

But even if hyper could solve all of those problems on its own, the other motivating reason would still exist: cross-compiling.

Copy link

CryZe commented on Oct 21

edited

we'd like to understand why it wouldn't suffice to simply have a separate crate exporting the cdylib/staticlib interface.

There's a big long standing bug that causes all sorts of problems around reexports and explicit cdylib / staticlib top level crates. If you don't use LTO you have to export all of the publicly visible symbols from the top level crate directly, reexporting doesn't work.

rust-lang/rust#50007

Copy link

Member

joshtriplett commented 19 days ago

@seanmonstar Would cargo rustc --crate-type alone solve your use case?

Copy link

Contributor

Author

seanmonstar commented 19 days ago

Currently, cargo rustc --crate-type cdylib fails with an error, argument '--crate-type' which wasn't expected.

If I use cargo rustc -- --crate-type cdylib, it compiles all the dependencies, and then errors with crate 'http' required to be available in rlib format, but was not found in this form, once for each dependency.

Do you mean would it be fine to make that work? Probably. I'm not stuck on it being cargo build. It's just that currently nothing at the command-line works.

Copy link

Contributor

ehuss commented 19 days ago

I think there may have been some confusion in some of the comments above. We are suggesting to reword the RFC so that it only adds --crate-type to cargo rustc (cargo rustc --crate-type cdylib) since the cargo rustc command is designed for building a specific target, and is also intended as a lower-level interface. That would circumvent some of the concerns around cargo build as it can be somewhat general purpose, and builds multiple targets. I think we would be willing to approve the RFC now if it switched from cargo build to cargo rustc.

Copy link

Contributor

Author

seanmonstar commented 19 days ago

Oh ok, sure! I indeed was a little confused, but that would be perfectly fine. I think. I must admit, I don't know all the finer details that makes them different. It feels like cargo rustc mostly just allows passing more arguments to rustc, but otherwise feels very similar to cargo build.

Copy link

Contributor

ehuss commented 17 days ago

Thanks! This looks like a good start to me.

@rfcbot fcp merge

Copy link

rfcbot commented 17 days ago

edited by Eh2406

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Copy link

rfcbot commented 17 days ago

bellThis is now entering its final comment period, as per the review above. bell

Copy link

Eh2406 commented 17 days ago

As artifact dependencies get stabilized, I would love for normal dependencies to know that they only need the rlib. And then for the unit graph to figure out what types are needed. So if a library has crate-type = ["lib", "staticlib", "cdylib"] that means that a cargo build in the library will make all 3, but does not imply that a project that depends on the library needs to build all 3. Such a change, if it worked out as well as I think it will, would reduce the motivation for this RFC. But this RFC would still be useful even then so +1.

Copy link

rfcbot commented 7 days ago

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Copy link

Contributor

ehuss commented 7 days ago

Huzzah! The @rust-lang/cargo team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/cargo#10083

ehuss

merged commit cfa5d53 into

rust-lang:master 7 days ago

seanmonstar

deleted the cargo-crate-type-cli branch

6 days ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK