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

Handle active custom toolchain in "show" #621

Merged
merged 2 commits into from
Jul 29, 2016

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Jul 28, 2016

Re #599. This is more complicated than it seems, because show tries to fetch all components of the active toolchain, and a custom toolchain a) doesn't have them, and b) doesn't even have an enforced name form. This set of patches enforces the custom- prefix for custom toolchains and adds the minimum necessary logic to handle them in show. It also updates the README to note the new requirement.

Edit: whoops, wrong approach. I think I'll force push the new commits, the original ones are pointless.

@inejge
Copy link
Contributor Author

inejge commented Jul 28, 2016

Does this perhaps need its own test?

@brson
Copy link
Contributor

brson commented Jul 29, 2016

@inejge Yes, please do add a test. There should be some existing show tests and custom toolchain tests to crib off of. Thanks.

For an active custom toolchain, targets can't be listed because
a custom toolchain doesn't support components. Normally, this error
would terminate 'show', which is surprising, so we silently use
an empty target list instead.
@inejge
Copy link
Contributor Author

inejge commented Jul 29, 2016

OK, I'll rebase the PR on top of 87890db in order to pick up the changes from #620 and group the test with the others in fn link, where I think it logically belongs.

@brson brson merged commit c2ebdf3 into rust-lang:master Jul 29, 2016
@brson
Copy link
Contributor

brson commented Jul 29, 2016

Thanks again @inejge ! Going to deploy this in just a few hours.

@inejge inejge deleted the active-custom branch July 31, 2016 18: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.

2 participants