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

Add Listing of Installed Components #1659

Merged
merged 1 commit into from
Mar 5, 2019

Conversation

naftulikay
Copy link

@naftulikay naftulikay commented Feb 13, 2019

This feature adds the ability to only list installed components rather than all available components.

My use-case is described in #1658; I'm writing automation around rustup and I need to be able to view only installed components so I can selectively install them.

Closes #1658.

@naftulikay naftulikay force-pushed the feature/component-list-installed branch from 44fd704 to 59ad164 Compare February 13, 2019 19:27
@naftulikay naftulikay changed the title [WIP] Add Listing of Installed Components Add Listing of Installed Components Feb 13, 2019
@naftulikay naftulikay force-pushed the feature/component-list-installed branch from 59ad164 to 5cf1bb1 Compare February 13, 2019 19:51
@naftulikay
Copy link
Author

Fixed formatting using cargo fmt.

@dwijnand
Copy link
Member

LGTM, except rather than introduce installed_only: bool I'd do the if block inside component_list in rustup_mode.rs.

@naftulikay
Copy link
Author

naftulikay commented Feb 14, 2019

@dwijnand okay, I'll refactor toward that. I put it separately because I felt like I'd have too many if statements nested within the function, but I'll refactor in that direction.

EDIT: Done.

@naftulikay naftulikay force-pushed the feature/component-list-installed branch from 5cf1bb1 to a0a22b1 Compare February 14, 2019 20:09
@dwijnand
Copy link
Member

Sorry, @naftulikay. I sent you a PR of what I meant: naftulikay#1.

@naftulikay naftulikay force-pushed the feature/component-list-installed branch 2 times, most recently from e8ce945 to 94f188c Compare February 14, 2019 23:06
@naftulikay
Copy link
Author

@dwijnand thank you! I have merged that in, the PR should now reflect this.

For the future, it might be beneficial to instead have a call format like:

rustup component list [--installed] [--available]

This would allow better querying, but I don't have an immediate need for this.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

src/rustup-cli/common.rs Outdated Show resolved Hide resolved
@naftulikay
Copy link
Author

@dwijnand since this is kind of related to #450, at least for my use-case, maybe this merits a broader approach if we're going to creep scope a bit anyway.

What I kind of need out of rustup for my Ansible role for installing a Rust development environment is some machine-readable output (e.g. #450) for detecting the following:

  • Components
    • Installed Components
    • Available Components
  • Targets
    • Installed Targets
    • Available Targets
  • Toolchains
    • Installed Toolchains
    • Available Toolchains
    • Default Toolchain
  • Metadata
    • rustc -Vv

My "API," so to speak, will be YAML something like this:

---
rust_components:
 - rustfmt
 - rls
rust_default_toolchain: stable
rust_toolchains:
 - stable
 - beta
 - nightly
rust_targets:
 - x86_64-unknown-linux-gnu
 - x86_64-unknown-linux-musl

For the things above, I need to detect what's available, detect what's installed, and perform actions only when things are not already installed and are available.

Since obviously this is a much larger piece of work, I'm happy to discuss more in detail, but hopefully my use-case is fairly clear.


--installed and --available filter flags to rustup toolchain, rustup component, and rustup target would go a long way, but again, this is quite a bit of scope creep.

How would you like to proceed? Should I just add the changes suggested above to this PR, or should we split/consolidate work on this? I'm happy to contribute code toward this end.

@kinnison
Copy link
Contributor

kinnison commented Mar 1, 2019

My gut feeling is that eventually I'd like for rustup to have a plumbing-style interface for the purpose of handling both ansible-like automation and also eventually rls-style JSON like streams.

However this is definitely out of scope for now, because we need to undergo a lot of codebase simplification and cleanup before we're ready to add that kind of complex API.

If you're interested in helping to define what such an API might look like then I would welcome you working with us to that end. In the meantime, we need to have the bare minimum of additional API to make things possible for you to continue.

This feature adds the ability to only list installed components
rather than all available components.
@naftulikay naftulikay force-pushed the feature/component-list-installed branch from 94f188c to 8f58bb0 Compare March 1, 2019 22:59
@naftulikay
Copy link
Author

@kinnison Thanks, I really appreciate the response. Please let me know if there's a good place to have that discussion.

@dwijnand I have updated the code to use ?.

@kinnison
Copy link
Contributor

kinnison commented Mar 3, 2019

@naftulikay From my PoV the best place to have that discussion will begin on the Discord for Rustup which you can find here: https://discord.gg/cKdYFVz

We are also trying to have weekly meetings around 20:30 UTC on Tuesdays but we'll be kinda busy in them for the next little while :D

@naftulikay
Copy link
Author

@kinnison I have joined the Discord, can someone grant me access into the #wg-rustup channel?

Please let me know if any changes are required for merge.

@kinnison
Copy link
Contributor

kinnison commented Mar 5, 2019

@naftulikay If you check your discord DMs, Dyno should have told you what you need to do in order to gain the ability to talk in channels.

@nrc nrc merged commit 170d711 into rust-lang:master Mar 5, 2019
@naftulikay naftulikay deleted the feature/component-list-installed branch March 5, 2019 22:03
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