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

Ensure correct toolchain per clone. #6054

Merged

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Jun 29, 2018

Previously, if the Rust toolchain was changed, re-bootstrapping would
proceed correctly in the first clone encountering the change but it
would fail in other clones. Ensure the correct toolchain is set when
building the symlink farm (but not needing to bootstrap the toolchain)
to cover this case.

Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Previously, if the Rust toolchain was changed, re-bootstrapping would
proceed correctly in the first clone encountering the change but it
would fail in other clones. Ensure the correct toolchain is set when
building the symlink farm (but not needing to bootstrap the toolchain)
to cover this case.
…tions of the toolchain are not or are corrupt.
@jsirois jsirois force-pushed the rust_toolchain/fix_multi_clone_use_case branch from 3c6071d to 8caa0f7 Compare June 29, 2018 16:43
@jsirois
Copy link
Contributor Author

jsirois commented Jun 29, 2018

Went a bit further here and did it really right. Probably worth another look.

function set_rust_toolchain() {
(
cd "${REPO_ROOT}"
"${RUSTUP}" override set "${RUST_TOOLCHAIN}" >&2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more explicit to use the rust-toolchain file: https://github.com/rust-lang-nursery/rustup.rs#the-toolchain-file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. Good idea.

This seals the current repo Rust toolchain version in a standard discoverable place. Also rename `bootstrap_rust.sh`'s exported function
to reflect what it actually now does.
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

This apparently needs to re-bootstrap the native engine binary and
supporting changes to test infra were needed to ensure the new
rust-toolchain file was available in the test buildroot as result.
Copy link
Contributor

@baroquebobcat baroquebobcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it'll be very cool. Thanks!

@jsirois jsirois merged commit 2870ed4 into pantsbuild:master Jun 29, 2018
@jsirois jsirois deleted the rust_toolchain/fix_multi_clone_use_case branch June 29, 2018 23:44
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.

3 participants