Shell out to rustfmt instead of embedding it?

[sunshowers]

Hi! My understanding is that currently RLS embeds a copy of rustfmt. Would it be possible to add an optional mode where it shells out to rustfmt at a given location instead?

For some context: we would like to start checking built copies of rustfmt (for all our platforms) into our monorepo. The idea is that all our Rust code is always kept autoformatted. A rustfmt version bump happens atomically with any formatting changes introduced in it.

The biggest blocker we have right now is RLS: since RLS embeds a copy of rustfmt, it's possible that the versions of rustfmt in RLS and in the repo are out of sync. It would be really cool if RLS could shell out to the copy of rustfmt in our repo instead of embedding it.

Simply using the stdio mode of rustfmt might work!

[nrc]

I think this makes sense now that rustup can install rustfmt, we can expect the client to install that component as well as the RLS.

I'm not exactly sure how this should work - we'd need to check that the various options we currently use are supported on the command line (I think they are) and add them if not (in particular, we need to be able to pass some code from memory, rather than disk, but I guess we should be able to use the stdin mode for that). We'd probably also want to be able to configure the path to rustfmt.

[sunshowers]

Do you think shelling out to rustfmt should be an optional mode, or do you think RLS should switch to doing that for everyone?

[nrc]

If possible (and I think it is, but haven't checked) I would like to just have one way to do it.

[lnicola]

I think removing the built-in support will can the experience worse for users. For example, rustfmt usually needs something like export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH or the equivalent for other platforms -- and a rustfmt binary can get out of sync with those.

I would rather have rls continue to use the compiled version instead.

EDIT: To be fair, rls already needs the library path to be set.

[sinkuu]

rustfmt-nightly no longer depends on the compiler. rust-lang/rustfmt#2341 (comment)