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

Build supports a crate-type flag. #7900

Closed
wants to merge 1 commit into from

Conversation

dvc94ch
Copy link

@dvc94ch dvc94ch commented Feb 18, 2020

So it seemed like #7899 it would be easy to fix, but the same error as in #7899 occurs. It works well for compiling examples, but fails to compile a library with crate required to be in rlib format. What is the difference between how libs are handled and how examples are handled?

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2020
@alexcrichton
Copy link
Member

Thanks for the PR here! We discussed this a bit yesterday during the cargo triage meeting, and the conclusion was that we're currently not really in a great place to consider this. This issue has a lot of historical precedent scattered throughout various issues in the issue tracker, so we want to be sure to carefully approach this and not quickly try to fix one case without considering others.

From a technical perspective this PR doesn't include any documentation updates, tests, or detailed rationale for why this feature is being designed the way it is. We tried to infer that the motivation for this PR is related to platform-specific crate types (e.g. Android and cdylibs), but that problem space is even larger and also requires careful consideration.

Our conclusion was that it is likely best to approach this feature in an RFC-style format, one where motivation, prior art, detailed design, etc, are all laid out clearly. That should help in figuring out the best way to design this feature!

@bors
Copy link
Contributor

bors commented Feb 20, 2020

☔ The latest upstream changes (presumably #7820) made this pull request unmergeable. Please resolve the merge conflicts.

@dvc94ch
Copy link
Author

dvc94ch commented Feb 21, 2020

It's a small PR that doesn't include any design other than making the minimal amout of changes. The reason why the flag was overriden in the compiler part of the code is because packages and targets are borrowed inside of units which makes it impossible to modify the crate-type after the crates to build have been selected.

Our conclusion was that it is likely best to approach this feature in an RFC-style format, one where motivation, prior art, detailed design, etc, are all laid out clearly.

That certainly feels like overkill and that what ought to be something fairly simple will take years to ship. The rfc process has been used for a couple of years now, I wonder what the fastest rfc to shipping code time was?

For the time being the best solution that has been found is using multiple Cargo.tomls and selecting the android one with --manifest-path. It's a bit unfortunate as they need to be kept in sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants