Add cargo-fmt into image

[tyranron]

There is already rustfmt inside image, so we can use it like rustfmt .... Unfortunately cargo-fmt is missing which makes impossible to run something like:

docker run --rm -v "$(pwd)":/app -w /app rustlang/rust:nightly-slim cargo fmt --all

That feels like partially done feature, and to actually use rustfmt on CI within a project everyone have to come up with custom Docker image where the only difference is:

# rustup component add rustfmt-preview
info: downloading component 'rustfmt-preview'
  3.9 MiB /   3.9 MiB (100 %)   1.8 MiB/s ETA:   0 s                
info: installing component 'rustfmt-preview'

Maybe it's worth to add one inside nightly images by default? It's only 4 MiB.

[sfackler]

The general approach for these images is to keep them as much of a "standard" installation as possible. When rustup installs rustfmt by default, we'll pick that up. As you've noted, it's pretty easy to pull in yourself when desired.

[tyranron]

@sfackler it's quite easy, yes, but not pretty at all, as has a bunch of downsides:

• undesired network I/O every time you need to run this command;
• for docker run we should compose it via --entrypoints sh ... -c 'rustup component add rustfmt-preview && cargo fmt --all' which disables proper signal handling (sh does not propagate them to its child processes) and results in problems with command cancellation.

I understand the point of being close to "standard" installation. Still, things with Docker images are slightly different than with normal installation. On normal installation we will use rustup component add rustfmt-preview and forget it. With docker image we should use it every single time we run the command.
It's not the point to add to the image every single rustup component, but the cargo-fmt is too common thing (really, who uses rustfmt directly instead of cargo fmt?), and I think it's worth it.

The only cost is ~4MiB. The benefit is much better ergonomic for Docker image users.

[sfackler]

Why do you need to start over from the base image every time you format?

Let's imagine someone filed the exact same issue as this, but they wanted the image to ship with rustup target add i686-unknown-linux-gnu to easily cross compile to 32 bit Linux. The only cost is ~50MiB. Should we add this as well? What about armv7-unknown-linux-gnueabihf? aarch64-unknown-gnu?

[tyranron]

@sfackler

Why do you need to start over from the base image every time you format?

To not come up with yet another one Docker image just for cargo fmt command.
Commonly, this is required in CI pipelines for cargo fmt -- --check. The other case - when someone uses docker-wrapped toolchain for stuff.

Let's imagine someone filed the exact same issue as this, but they wanted the image to ship...

Nope, that's not the same. I wrote about "too common thing" for cargo-fmt.
I do not propose to add to the image clippy and x86_64-unknown-linux-musl target despite the fact I'm using them. I understand your point that having every single component in a standard Docker image is bad. I've just proposed to make rustfmt (which is present inside the image already, but partially) usable enough without painful hacks for image users. That's it.

If that still doesn't make sense, please, just close the issue. It's not so critical to continue arguing on it.

[tyranron]

@sfackler taking into account your opinion in previous posts and that discussion became stale, I can conclude that proposed changes won't gonna happen and are not desirable.

So, I'm closing this issue for now to not bother you and the team.

[mzabaluev]

To not come up with yet another one Docker image just for cargo fmt command.

What's the downside? Incremental images are cheap and very friendly to caching used by CI environments.

[tyranron]

@mzabaluev maintaining is always the downside.