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

Select distribution strategy for rustfix/cargo-fix #52272

Closed
alexcrichton opened this issue Jul 11, 2018 · 4 comments
Closed

Select distribution strategy for rustfix/cargo-fix #52272

alexcrichton opened this issue Jul 11, 2018 · 4 comments
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition
Milestone

Comments

@alexcrichton
Copy link
Member

Currently we distribute cargo fix subcommand through cargo install cargo-fix. I discussed locally on discord though with @aturon and @Mark-Simulacrum that I'd personally prefer to distribute cargo fix by default to users. In any case, a decision should be made!

I believe we have one of three options here to us:

  • Do nothing - continue to ship cargo fix through cargo install. This has the benefit of not needing any work to get it done. It has a downside, however, of requiring users to opt-in to acquiring cargo fix. Additionally it introduces the possibility for version skew where cargo fix is either too old or too new by accident.
  • Ship a rustup component - here we'd enable something like rustup component add cargo-fix or automatically enable it by default for new installations. This allows us to control versioning and provide it by default, but it requires a relatively large amount of rustbuild/dist work.
  • Merge cargo-fix into Cargo - here we'd merge the cargo fix subcommand inside the rustfix repository to Cargo itself upstream, depending on the rustfix crate from crates.io. This has the benefits of shipping a rustup component but it's a little easier to do in terms of distribution. The downside of this approach is that, like a component, it ties rustfix to the trains so getting out a hotfix is more fuss (needs a full Rust release)

I'm personally inclined to go the route of "merge cargo-fix into Cargo". I don't believe rustfix has had many major changes and/or big bugs reported, so merging it into Cargo should require little-to-no fuss and the need for a hotfix should be pretty low. I'm curious to hear what others think though!

cc @killercup

@alexcrichton alexcrichton added the A-edition-2018-lints Area: Lints supporting the 2018 edition label Jul 11, 2018
@killercup
Copy link
Member

killercup commented Jul 11, 2018

Merging cargo-fix into cargo sounds good to me. The code base is still quite small -- especially if we were to only move the cargo specific parts into cargo and keep the rustfix (library) crate separate (it's also not that big, though). One reason to keep the rustfix lib separate is that it is also used in compile test.

We might want to refactor some of the CLI code to re-use the existing cargo infrastructure but this is neither that big of an effort nor necessary for the first integration PR.

@joshtriplett
Copy link
Member

We should consider doing the same for cargo fmt.

@alexcrichton
Copy link
Member Author

I've created a PR for moving this into Cargo at rust-lang/cargo#5723

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 17, 2018
This commit imports the `cargo fix` subcommand in rust-lang-nursery/rustfix
directly into Cargo as a subcommand. This should allow us to ease our
distribution story of `cargo fix` as we prepare for the upcoming 2018 edition
release.

It's been attempted here to make the code as idiomatic as possible for Cargo's
own codebase. Additionally all tests from cargo-fix were imported into Cargo's
test suite as well. After this lands and is published in nightly the `cargo-fix`
command in rust-lang-nursery/rustfix will likely be removed.

cc rust-lang/rust#52272
bors added a commit to rust-lang/cargo that referenced this issue Jul 17, 2018
Import `cargo fix` directly in to Cargo

This commit imports the `cargo fix` subcommand in rust-lang-nursery/rustfix
directly into Cargo as a subcommand. This should allow us to ease our
distribution story of `cargo fix` as we prepare for the upcoming 2018 edition
release.

It's been attempted here to make the code as idiomatic as possible for Cargo's
own codebase. Additionally all tests from cargo-fix were imported into Cargo's
test suite as well. After this lands and is published in nightly the `cargo-fix`
command in rust-lang-nursery/rustfix will likely be removed.

cc rust-lang/rust#52272
@Mark-Simulacrum
Copy link
Member

Closing as this seems done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition
Projects
None yet
Development

No branches or pull requests

4 participants