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

Strategy for original rustfmt crate on crates.io #4343

Open
calebcartwright opened this issue Jul 22, 2020 · 16 comments
Open

Strategy for original rustfmt crate on crates.io #4343

calebcartwright opened this issue Jul 22, 2020 · 16 comments

Comments

@calebcartwright
Copy link
Member

There's an ancient rustfmt crate on crates.io in addition to the current/maintained rustfmt-nightly crate, and the former's latest version is v0.10.0 which was published over 2.5 years ago.

This has been a source of confusion for users, particularly those new to the Rust ecosystem who go to crates.io looking for rustfmt, so I'm wondering if we should try to deprecate it/yank versions or if there's some valid usage patterns for that crate I'm just not aware of.

Thoughts?

@topecongiro
Copy link
Contributor

I prefer to keep rustfmt over rustfmt-nightly as a valid name as the latter naming is for historical reasons.

I would like to publish to both rustfmt-nightly and rustfmt, advocate for using rustfmt over rustfmt-nightly and yank rustfmt-nightly at some point the future.

cc @nrc @steveklabnik who own https://crates.io/crates/rustfmt.

@calebcartwright
Copy link
Member Author

Ahh okay, SGTM (and sorry for the duplicate 😄)

@steveklabnik
Copy link
Member

I would like to publish to both rustfmt-nightly and rustfmt, advocate for using rustfmt over rustfmt-nightly and yank rustfmt-nightly at some point the future.

This would be my personal preferred strategy as well, but in a policy sense, I am not sure who is in charge of doing this. My guess is @rust-lang/devtools ?

@ehuss
Copy link
Contributor

ehuss commented Jul 22, 2020

Is there not a concern that users try to install rustfmt and break their installation? I've seen that happen, where the user runs cargo install rustfmt, and now they are using a 3 year old version, and don't understand why it sometimes doesn't work. If the rustfmt crate was yanked (or replaced with one that generates a compile error explaining the situation), that wouldn't happen. If the rustfmt crate is a new version, you kinda have a different problem where "my rustfmt works different from this other person's and I don't know why".

It's probably not too important, but something to be careful of.

@calebcartwright
Copy link
Member Author

calebcartwright commented Jul 22, 2020

Is there not a concern that users try to install rustfmt and break their installation?

That was the concern for me in opening this issue, as this definitely happens including as recently as yesterday. I think in the short term even something as simple as publishing a 0.10.1 with a deprecation notice in the readme that says "Don't use this, use rustup" would be helpful (though I like the compile error idea better) while we figure out what we should do longer term.

@Manishearth
Copy link
Member

I'm okay with renaming rustfmt-nightly to rustfmt. I would slightly caution against shipping a binary here.

I would like to eventually start a discussion about rust-lang/rust#70651 for rustfmt, because the current situation with updating over rustc-ap crates is not great. If that's implemented then it's going to be really tricky to publish rustfmt to crates, and it's worth looking at how people use the crate and if something that uses the tool can instead be used.

@davepacheco
Copy link

Sorry if this is the wrong place to ask, but: I just found the rustfmt-nightly crate on crates.io and wanted to use it to see how different versions of rustfmt might format some code differently. I see that the last version published was 1.4.21, but the latest rustfmt is 1.4.30, and Rust stable 1.48 and 1.49 both shipped with rustfmt newer than the one on crates.io. Was it intentional that this crate stopped being updated? (I know that I can use rust toolchain to add the versions I want, but that will download and store a lot more than I need just to test different rustfmt versions.) Thanks!

@calebcartwright
Copy link
Member Author

Was it intentional that this crate stopped being updated? (I know that I can use rust toolchain to add the versions I want, but that will download and store a lot more than I need just to test different rustfmt versions.) Thanks!

Not intentional, I just don't have access to push to crates 😄 Have started enumerating the install options in the Changelog and you'll note that the crates.io option is still listed as pending for all the recent releases.

If/when I get access to update the crate I'm happy to go back and publish these pending versions and any new ones that come along.

If you just want to grab a rustfmt binary though for some testing, you might find the GH releases (which contain pre-built binaries for several platforms) easier than trying to get a corresponding nightly needed to grab a specific version from crates.io

@thomaseizinger
Copy link
Contributor

Not intentional, I just don't have access to push to crates smile Have started enumerating the install options in the Changelog and you'll note that the crates.io option is still listed as pending for all the recent releases.

If/when I get access to update the crate I'm happy to go back and publish these pending versions and any new ones that come along.

Is there any plan on this happening?
I would really like to use the new imports_granularity feature through the rustfmt dprint plugin which depends on rustfmt as a lib and pulls it from crates.io.

@calebcartwright
Copy link
Member Author

Is there any plan on this happening?

Not really. The folks that have access to the crate have not been involved with the project for a while, and I didn't have much luck the last time I inquired about getting added. Will give it another go at some point, but this has not been a particularly high priority item

I would really like to use the new imports_granularity feature through the rustfmt dprint plugin which depends on rustfmt as a lib and pulls it from crates.io.

While I understand the desire and benefits to have formatting capabilities available as a library, I want to reiterate caution given the current state of things.

rustfmt itself is coupled to the auto publish crates for access to the rustc internals, so by depending on the rustfmt crate all of those challenges associated with the auto publish crates will cascade down to you (and anything else that depends on your project). Notably that means nightly only, fairly frequent compilation failures on nightly, as well as some rustc internal requirements that trickle through like having to set CFG_RELESE and CFG_RELEASE_CHANNEL.

If you really want to go ahead and use the latest version of rustfmt as a lib then I suppose you could always specify the dependency as a git reference using one of the corresponding tags, but I'm rather skeptical about the viability of the rustfmt-as-a-lib use case given the current context and constraints.

@Manishearth
Copy link
Member

Yeah I think we should be trying to deprecate rustfmt-as-a-crate

@thomaseizinger
Copy link
Contributor

rustfmt itself is coupled to the auto publish crates for access to the rustc internals, so by depending on the rustfmt crate all of those challenges associated with the auto publish crates will cascade down to you (and anything else that depends on your project). Notably that means nightly only, fairly frequent compilation failures on nightly, as well as some rustc internal requirements that trickle through like having to set CFG_RELESE and CFG_RELEASE_CHANNEL.

If you really want to go ahead and use the latest version of rustfmt as a lib then I suppose you could always specify the dependency as a git reference using one of the corresponding tags, but I'm rather skeptical about the viability of the rustfmt-as-a-lib use case given the current context and constraints.

I haven't been involved in the initial development of the dprint plugin crate but at least the current state of things doesn't look very complicated: https://github.com/dprint/dprint-plugin-rustfmt

Yeah I think we should be trying to deprecate rustfmt-as-a-crate

Please don't deprecate rustfmt-as-a-crate. Being able to use rustfmt as a library provides a lot of value to us because it means we can use dprint as a single formatting tool across our codebases that often times mix several languages with each other.
Additionally, it is very nice to not being able to be restricted by what rustfmt considers stable and nightly. For example, the currently published version of rustfmt (v1) makes it impossible to use unstable formatting features on a stable toolchain (I know that this might change with v2 but that is not available yet).

As a result and before we discovered dprint, we had to always install two toolchains (1 nightly and stable) to be able to use features like merge_imports while at the same time compiling our code on stable. Ever since we use dprint, the amount of tooling configuration that we need to deal with in our codebases went down quite a bit.

I am not sure how many other people have similar requirements. I am just pointing out that not having rustfmt as a crate makes usecases like @dsherret's dprint harder to realize. (He has some plugins that invoke native binaries instead of being pure wasm plugins but wasm plugins are arguably the much nicer solution but they do require rustfmt to be available as a library.)

@dsherret
Copy link

@thomaseizinger in the case of dprint-plugin-rustfmt, luckily I believe it could probably get away with building from the git tag because the plugin is published to a WASM file on github releases instead of to crates.io. That said, not having this crate published on crates.io might not be so nice for others.

@calebcartwright
Copy link
Member Author

calebcartwright commented Jan 29, 2021

at least the current state of things doesn't look very complicated: https://github.com/dprint/dprint-plugin-rustfmt

That project is using a fairly old version, which predates some of the rustc updates, like the CFG_RELEASE vars. I suspect you'll run into this quickly if/when you update.

Being able to use rustfmt as a library provides a lot of value to us because it means we can use dprint as a single formatting tool across our codebases that often times mix several languages with each other.

Could you elaborate on how this specific use case is enabled by the rustfmt lib as a crate, that wouldn't be available by running the bin or using the lib from git? Editors/IDEs support running rustfmt (as well as various other formatters for different languages), and the vast majority of them, including RA IIRC, invoke the rustfmt binary.

Additionally, it is very nice to not being able to be restricted by what rustfmt considers stable and nightly. For example, the currently published version of rustfmt (v1) makes it impossible to use unstable formatting features on a stable toolchain (I know that this might change with v2 but that is not available yet)

Sorry, but it's a bug not a feature that unstable options can be used on stable, and it could very well be removed in a future release. Additionally, the stability of configuration options/the cadence at which option are stabilized feels like an orthogonal topic.

The same bug that allows this in the library exists in the CLI too:

rustfmt foo.rs --config merge_imports=true

When and if the bug gets fixed, it is fixed everywhere, including any versions published to crates.io.

I am just pointing out that not having rustfmt as a crate makes usecases like @dsherret's dprint harder to realize. (He has some plugins that invoke native binaries instead of being pure wasm plugins but wasm plugins are arguably the much nicer solution but they do require rustfmt to be available as a library.)

That seems a bit like an implementation choice, with viable alternatives.

I've shared this previously in a few threads, but will reiterate again here that this is ultimately a matter of weighing tradeoffs.

Making rustfmt available for lib consumption from crates.io is not easy and has significant drawbacks for all rustfmt users on nightly, not just the handful of scenarios where folks would like a lib from crates. Doing this is what results in rustfmt being broken and unavailable on nightly at a regular cadence (again, not available for anyone, not just someone trying to build the crate). It also eats up a ton of the rustfmt team's bandwidth because changes in rustc land all the time that in turn cause downstream breakage in rustfmt. We then have to retroactively figure out what changed in rustc and the associated impacts and changes we need to make. These updates then have to be coordinated across 4 (sometimes 5) separate projects, just to get rustfmt available again. We've also had cases where this complicates the ability to backport fixes onto beta/stable.

We'd love to be able to support the types of use cases you've shared, and if there were zero drawbacks to continuing to make rustfmt available on crates.io then we wouldn't even be having the conversation. The challenge, however, is that it is the cause of a lot of very real problems both for users and maintainers, and we have to consider whether continuing to support these types of edge cases is worth the negative side effects.

(edit: typo fixes)

@thomaseizinger
Copy link
Contributor

Being able to use rustfmt as a library provides a lot of value to us because it means we can use dprint as a single formatting tool across our codebases that often times mix several languages with each other.

Could you elaborate on how this specific use case is enabled by the rustfmt lib as a crate, that wouldn't be available by running the bin or using the lib from git? Editors/IDEs support running rustfmt (as well as various other formatters for different languages), and the vast majority of them, including RA IIRC, invoke the rustfmt binary.

As @dsherret pointed out, the dprint plugin is published as a wasm binary anyway and as such can actually use git dependencies.
This means at least for this usecase, a release on crates.io is not critical.

Additionally, it is very nice to not being able to be restricted by what rustfmt considers stable and nightly. For example, the currently published version of rustfmt (v1) makes it impossible to use unstable formatting features on a stable toolchain (I know that this might change with v2 but that is not available yet)

Sorry, but it's a bug not a feature that unstable options can be used on stable, and it could very well be removed in a future release. Additionally, the stability of configuration options/the cadence at which option are stabilized feels like an orthogonal topic.

The same bug that allows this in the library exists in the CLI too:

rustfmt foo.rs --config merge_imports=true

When and if the bug gets fixed, it is fixed everywhere, including any versions published to crates.io.

Well, at least through cargo fmt, it is precisely not possible to use unstable formatting features on a stable channel which is why we had to install two toolchains in the past.
FWIW, it never made sense to me that stable/unstable formatting features are coupled to the rustc release channels. I would like to build software against the stable compiler release because it receives a lot more testing. On the other hand, I have no issues using unstable formatting features if they provide sufficient value. In this particular case, merge_imports served us very well for 2+ years.

Unfortunately, precisely this is not possible with cargo as the primary interface. I've never actually dealt with the rustfmt binary itself.

I am just pointing out that not having rustfmt as a crate makes usecases like @dsherret's dprint harder to realize. (He has some plugins that invoke native binaries instead of being pure wasm plugins but wasm plugins are arguably the much nicer solution but they do require rustfmt to be available as a library.)

That seems a bit like an implementation choice, with viable alternatives.

As noted above, I didn't see that git dependencies are an option here.

I've shared this previously in a few threads, but will reiterate again here that this is ultimately a matter of weighing tradeoffs.

Making rustfmt available for lib consumption from crates.io is not easy and has significant drawbacks for all rustfmt users on nightly, not just the handful of scenarios where folks would like a lib from crates. Doing this is what results in rustfmt being broken and unavailable on nightly at a regular cadence (again, not available for anyone, not just someone trying to build the crate). It also eats up a ton of the rustfmt team's bandwidth because changes in rustc land all the time that in turn cause downstream breakage in rustfmt. We then have to retroactively figure out what changed in rustc and the associated impacts and changes we need to make. These updates then have to be coordinated across 4 (sometimes 5) separate projects, just to get rustfmt available again. We've also had cases where this complicates the ability to backport fixes onto beta/stable.

We'd love to be able to support the types of use cases you've shared, and if there were zero drawbacks to continuing to make rustfmt available on crates.io then we wouldn't even be having the conversation. The challenge, however, is that it is the cause of a lot of very real problems both for users and maintainers, and we have to consider whether continuing to support these types of edge cases is worth the negative side effects.

Thank you for sharing your perspective! I didn't fully see the complexity that is involved in creating / maintaining these releases and I totally get that you want to free yourself from as much of this complexity as possible :)

At least for the particular topic at hand, a library interface and git tags should be sufficient. Is the rustfmt binary + lib split something you consider keeping (even if the lib is not published anywhere) or are you thinking of moving everything into a binary crate?

@calebcartwright
Copy link
Member Author

Thank you for sharing your perspective! I didn't fully see the complexity that is involved in creating / maintaining these releases and I totally get that you want to free yourself from as much of this complexity as possible :)

I want to stress that the focus of these discussions is users and the community, not the maintainers.

The current approach prioritizes having the ability to also distribute rustfmt via crates.io, but has drawbacks for everyone in the community that utilizes rustfmt.

The status quo is what results in broken toolstates on nightly and rustfmt being unavailable through the main channel (rustup), often for extended periods of time. This is a common point of confusion, particularly for folks that are new to Rust.

Yes it's true that there's a lot of overhead with the current approach that eats up bandwidth, and that means the rustfmt team has less capacity to implement new features, fix bugs, and work on stabilizing configuration options. However, that's entirely secondary.

Again I appreciate the benefits of having rustfmt available as a crate and understand that it's useful in certain cases. We're just considering whether the status quo is tenable, and whether the use cases that benefit from being able to consume rustfmt from crates.io outweigh all of the drawbacks shared on this issue thread.

Is the rustfmt binary + lib split something you consider keeping (even if the lib is not published anywhere) or are you thinking of moving everything into a binary crate?

Correct, we have no intention of changing that regardless of what may or may not be done around crates.io.


The rest of these items are quite far off the focus of this issue, so this will be my last response to these particular topics on this thread. I'm happy to continue the discussion if you'd like, but would ask that you move it to a different issue and/or Discord.

additional responses

Well, at least through cargo fmt, it is precisely not possible to use unstable formatting features on a stable channel which is why we had to install two toolchains in the past.

That's not correct. The bug exists in an override function that's exposed through the library, and thus to rustfmt, and accordingly to cargo fmt, via the cli --config option:

$ cargo +stable fmt --version
rustfmt 1.4.25-stable (0f29ff6 2020-11-11)
$ cargo +stable fmt -- --check --config merge_imports=true
Diff in ..../src/main.rs at line 1:
-use foo::bar;
-    use foo::baz;
+use foo::{bar, baz};
 
 fn main() {
     println!("Hello, world!");

FWIW, it never made sense to me that stable/unstable formatting features are coupled to the rustc release channels. I would like to build software against the stable compiler release because it receives a lot more testing. On the other hand, I have no issues using unstable formatting features if they provide sufficient value. In this particular case, merge_imports served us very well for 2+ years.

I can understand that point of view, and I'm glad you've had positive experiences with merge_imports!

For future reference, we have documentation that enumerates the requirements for stabilizing options. We won't call a new configuration stable/complete unless and until it can clear that threshold, regardless of whether it's 2 weeks old or 2 years old. It's pretty common within the Rust ecosystem for some features to only be available on nightly, and the rustfmt configuration options (which are how certain formatting features are surfaced) are following that same paradigm (aforementioned bug aside).

Specifically with merge_imports there are known bugs that still exist to this day, and it had also been clear for quite some time that the nature of the option did not fully meet the community need (#3362). This is why we now have imports_granularity, and that evolution is something we wouldn't have been able to do (or at least would've been signficantly more challenging to do) had merge_imports been stabilized prematurely.

Finally, the question/suggestion about whether or not stable rustfmt should support an explicit opt-in to permit using unstable config options on stable has surfaced a few times before, but it's typically been met with a lot of pushback.

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

No branches or pull requests

8 participants