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

Include path to rustc when generating package metadata #1802

Closed
wants to merge 1 commit into from
Closed

Include path to rustc when generating package metadata #1802

wants to merge 1 commit into from

Conversation

cl91
Copy link

@cl91 cl91 commented Jul 13, 2015

Currently, when generating package metadata to pass on to rustc in the form of -C metadata=, the path to rustc is not included. This means that a package compiled by different compilers will have the same metadata, other things being equal.

This is problematic when we are bootstrapping the Rust compiler with Cargo. Because Cargo generates the same metadata for both the stage1 and stage2 libraries, rustc will complain about multiple possible candidates of the same crate when using the stage1 compiler to compile the stage2 libraries.

We fix this by including the path to the rustc being used to compile the package when generating the package metadata. Because the stage1 and stage2 libraries are compiled by different compilers, this ensures that they come with different metadata.

This is also a good idea in general because you generally want to recompile a package when you switch compilers (although this doesn't happen outside bootstrapping the compiler itself).

Currently, when generating package metadata to pass on to rustc in
the form of `-C metadata=`, the path to rustc is not included. This
means that a package compiled by different compilers will have the
same metadata, when other things are equal.

This is less than ideal as in general it is a good idea to recompile
a package when we switch the compiler. This is particularly problematic
when we are bootstrapping the Rust compiler with Cargo. In this case
it is imperative that the stage1 and stage2 packages have different
metadata, otherwise rustc will complain about multiple possible
candidate of the same crate (one in the stage1 compiler, the other
in the stage2 compiler).
@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 @huonw (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. 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.

@cl91
Copy link
Author

cl91 commented Jul 13, 2015

@alexcrichton

@alexcrichton
Copy link
Member

For the bootstrap I was expecting usage of CARGO_TARGET_DIR as an environment variable to scope the output of the compiler, and that way you'd cache the results of stage1/stage2 without having to totally rebuild the world.

Taking the path into account feels a little sketchy to me as it doesn't correlate well to what version the compiler is except in the case of the bootstrap.

@cl91
Copy link
Author

cl91 commented Jul 13, 2015

@alexcrichton I'd argue that because the whole point of bootstrapping is to recompile everything --- the standard library and the compiler crates, with the stage1 compiler, there is very little one can reuse by doing this. The only thing that does not need to be recompiled is the runtime libraries, ie rust_builtin and friends, jemalloc, and libbacktrace on Linux, which take less than 5 seconds in total to compile on even my slow laptop. The rest of the world must be recompiled by the stage1 compiler.

Moreover, due to the algorithm that rustc uses to load crates (in librustc/metadata/loader.rs), it is illegal to have multiple candidates (with the same metadata) of the same crate. It will be a compiler error if you try to compile the stage2 libraries with the stage1 compiler, without giving them a different set of metadata: the compiler will see two sets of libraries (one in the sysroot of the stage1 compiler, the other in the deps dir of the stage2 libraries being compiled) and complain about multiple candidates of the same crate.

The other option is to have an environment variable say CARGO_BOOTSTRAP_HASH and (if it exists) mix it into the package metadata. I can't say which approach is strictly better than the other, though the latter one does make the intention more obvious (that we need a different set of metadata because we are bootstrapping).

@alexcrichton
Copy link
Member

there is very little one can reuse by doing this

This isn't quite true because I very often build a stage2 compiler but while I'm waiting on stage2 I'm testing with stage1. Each compiler is independent and can be useful in its own right.

@huonw
Copy link
Member

huonw commented Jul 15, 2015

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned huonw Jul 15, 2015
@cl91
Copy link
Author

cl91 commented Jul 15, 2015

This probably isn't a proper solution, so I'm closing it for now.

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 this pull request may close these issues.

4 participants