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.

Sign up for GitHub

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

Jump to bottom

Added help for rustup toolchain link as documented in issue #954. #1017

Merged
merged 5 commits into from
May 9, 2017
Merged

Added help for rustup toolchain link as documented in issue #954. #1017

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift click to select a range
ffc4738
Added help for `rustup toolchain link` as documented in issue #954.
Mar 29, 2017
27202ac
Better help and basic error checking for `rustup toolchain link` (#954).
Apr 3, 2017
bdc7c3a
Moves error-checking from `toolchain_link` into `install_from_dir`.
Apr 5, 2017
c28b805
Merge branch 'master' into fix-issue-954
Apr 10, 2017
bd7c369
Fixes failing tests.
Apr 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/rustup-cli/help.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 30,7 @@ the same as `rustup toolchain install`.
'toolchain' specifies a toolchain name, such as 'stable', 'nightly',
or '1.8.0'. For more information see `rustup help toolchain`.";

pub static TOOLCHAIN_INSTALL_HELP: &'static str =
pub static INSTALL_HELP: &'static str =
r"
Installs a specific rust toolchain.

Expand Down Expand Up @@ -78,7 78,23 @@ inferred, so the above could be written:
$ rustup default stable-msvc

Toolchain names that don't name a channel instead can be used to name
custom toolchains with the `rustup toolchain link` command.";
custom toolchains with the `rustup toolchain link` command. This can
also be used to symlink toolchains from local builds. For more
information see `rustup toolchain help link`.";
Copy link
Contributor

Choose a reason for hiding this comment

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

These two sentences seem slightly redundant. What about something like "rustup can also manage symlinked local toolchain builds, which is often used for developing Rust itself. For more information see rustup toolchain help link.


pub static TOOLCHAIN_LINK_HELP: &'static str =
r"
Symlinks a toolchain from a local directory specified by 'path' and
gives it a custom name specified by 'toolchain' if it does not name a
standard release channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand what this means? The text just below seems to imply that nightly is a perfectly fine name to use, but the text here seems to suggest that I can't use a "standard release channel" name?

Copy link
Author

@typesanitizer typesanitizer Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

(Okay, I don't fully understand all the code but here is what I think is correct.)
What it means is that if you give it a name such as a standard release channel, say nightly then it will parse and understand what you are trying to say (https://github.com/rust-lang-nursery/rustup.rs/blob/master/src/rustup/config.rs#L388).
You can certainly use nightly but that would mean that your actual nightly compiler (if you have one) will be replaced by the one at 'path'.
If you give it some unknown name which it doesn't recognize, say nightly1, then it will think that you are trying to give a custom name to the toolchain and will oblige.

Copy link
Author

@typesanitizer typesanitizer Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The call order is toolchain_link (in rustup-cli/rustup-mode.rs) -> get_toolchain (in rustup/config.rs) -> Toolchain::from (in rustup/Toolchain.rs) -> resolve_toolchain (in rustup/config.rs, linked earler) -> PartialToolchainDesc::from_str (in rustup-dist/src/dist.rs).

Copy link
Contributor

@jonhoo jonhoo Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I see. Then I think the text should be clearer about this. How about:

Symlinks a toolchain from a local directory specified by 'path' and gives it a custom name specified by 'toolchain'. If toolchain matches the name of a standard release channel such as stable, nightly, 1.8.0, etc., then path will be used in place of the standard binaries for those toolchains. For example, if a toolchain symlink is set up called nightly, any directory with a nightly override will now also use the new symlink.

Also, I notice the text currently says

For more information see rustup help toolchain
Does rustup help toolchain actually give more information at the moment?

Copy link
Author

@typesanitizer typesanitizer Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

How about: ...

Good point. Your explanation is much clearer than mine.

Does rustup help toolchain actually give more information at the moment?

rustup help toolchain has a full explanation of the toolchain name being split as <channel>[-<date>][-<host>] alongwith a couple of examples and the meanings of the individual terms. This is also referred to by rustup help installso I thought I should follow suit.

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, I wonder if we should also repeat some, if not all, of TOOLCHAIN_LINK_HELP in rustup help toolchain as well (if that's not already the case).

Copy link
Author

Choose a reason for hiding this comment

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

if that's not already the case

Currently, TOOLCHAIN_HELP (used for rustup help toolchain) only tells what each subcommand does in a succinct manner and each subcommand is responsible for its own documentation (or lack thereof). The current documentation for link inside TOOLCHAIN_HELP is just:

SUBCOMMANDS:
...
link Create a custom toolchain by symlinking to a directory
...
Toolchain names that don't name a channel instead can be used to name
custom toolchains with the rustup toolchain link command. This can
also be used to symlink toolchains from local builds. For more
information see rustup toolchain help link.

I feel that this is sufficient information to indicate to the user whether toolchain link is something they want to use, although the wording could be improved to make the dual purpose (overwriting or aliasing) clearer. Just making the utility clearer doesn't require the full details; e.g. how the path is to be specified.


'toolchain' specifies a toolchain name, such as 'stable', 'nightly',
or '1.8.0'. For more information see `rustup help toolchain`.

'path' is specified as `/path/to/rust/build/$triple/stage1` where
'$triple' should be replaced with the desired triple. Only a build of
`rustc` is required to be present at the given 'path'.
Copy link
Contributor

Choose a reason for hiding this comment

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

toolchain link can work with toolchains from any source, and this advice is specifically about linking the toolchain directly from the build directory. Can you clarify? Maybe like, "When used for development of Rust itself, toolchains can be linked directly out of the build directory, specifying path as ...".

";

pub static OVERRIDE_HELP: &'static str =
r"
Expand Down
3 changes: 2 additions & 1 deletion src/rustup-cli/rustup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 128,7 @@ pub fn cli() -> App<'static, 'static> {
.after_help(SHOW_HELP))
.subcommand(SubCommand::with_name("install")
.about("Update Rust toolchains")
.after_help(TOOLCHAIN_INSTALL_HELP)
.after_help(INSTALL_HELP)
.setting(AppSettings::Hidden) // synonym for 'toolchain install'
.arg(Arg::with_name("toolchain")
.required(true)))
Expand Down Expand Up @@ -167,6 167,7 @@ pub fn cli() -> App<'static, 'static> {
.multiple(true)))
.subcommand(SubCommand::with_name("link")
.about("Create a custom toolchain by symlinking to a directory")
.after_help(TOOLCHAIN_LINK_HELP)
.arg(Arg::with_name("toolchain")
.required(true))
.arg(Arg::with_name("path")
Expand Down