update rustfmt from 2020-06-02 to 2020-07-26

[jclulow]

The current expected version of rustfmt (from the nightly channel as at 2020-06-02) is not available for illumos systems. By advancing forward to 2020-07-26, we can use a version that appears to be available on all interesting platforms:

 $ for plat in linux darwin illumos windows; do curl -s -i https://static.rust-lang.org/dist/2020-07-26/channel-rust-nightly.toml | grep "^url.*https://.*rustfmt.*x86_64.*$plat"; done | sed -e 's/.*nightly-//' -e 's/.tar.gz.*//' | sort
x86_64-apple-darwin
x86_64-pc-windows-gnu
x86_64-pc-windows-msvc
x86_64-unknown-illumos
x86_64-unknown-linux-gnu
x86_64-unknown-linux-musl

No source changes appear to be required, at least according to:

cargo +nightly-2020-07-26 fmt -- --check

Users should be able to get the new toolchain via rustup install nightly-2020-07-26.

[ahl]

Looks good to me, but I'd like @davepacheco to sign off

[davepacheco]

We should probably document our policy and plan here, if for no other reason than I keep forgetting it. From my recollection:

rustfmt changes how it formats code even in micro and minor semver updates. As a result, a project must either pick a specific version of rustfmt that must be used for formatting its code (that version might change over time, but it's only ever one specific version at a time) or else accept that it's going to see constant reformatting of the code as developers with different rustfmt versions push changes that are otherwise unrelated to formatting. Constant churn sounds painful and defeats the purpose of uniform style, so we've opted for using a specific rustfmt version.

We enforce this in rustfmt.toml by specifying the required_version option. This causes rustfmt to fail explicitly if you run the wrong version. Tragically, this option requires a "nightly" build of rustfmt. (This is not because support for this flag depends on the version that's in the nightly toolchain -- stable versions of rustfmt have been new enough to support this option for a long time -- but because use of unstable options at all requires using the nightly toolchain as a matter of policy.) Because of this and because we use a few other unstable options, we require that code be formatted using a "nightly" version of rustfmt.

The upshot of all this is: Dropshot is expected to build and pass tests with most Rust versions, but in order to format the code, you need a specific nightly toolchain. @ahl, does all this match your recollection?

---

I feel there must be a better way to deal with this, but we haven't found it yet. What do other projects do? It's annoying to need the nightly toolchain for rustfmt, but if we didn't require it, then it seems like we'd wind up with a tighter coupling to the Rust stable toolchain that people are using. (Maybe that's what most people do? Assume developers on the latest Rust stable?) Plus, losing required_version makes it easy to use the wrong version locally, which can be really painful: if you've got a big outstanding change and accidentally run the wrong rustfmt just once -- your complex set of changes are now merged with a bunch of spurious formatting changes. (Maybe this can be undone by reformatting with the right version? I didn't think to try this.)

Regardless, there still is a specific rustfmt version we require, and we ought to document this in the README, and also which version you need for it. Maybe something like:

To contribute to Dropshot, you'll want to have installed the nightly-2020-07-26 toolchain. When formatting code, be sure that you use cargo +nightly-2020-07-26 fmt or rustfmt +nightly-2020-07-26. Your normal build and test can use a stable Rust.

It'd be great to do that here, but not necessary to merge this PR.

[steveklabnik]

What do other projects do?

Mostly just run on stable, and if something changes the format output, push a commit that fixes it. I don't tend to experience a lot of churn in formatting between versions.

your complex set of changes are now merged with a bunch of spurious formatting changes.

You can have more than one commit, where one contains the formatting changes only.

[ahl]

The upshot of all this is: Dropshot is expected to build and pass tests with most Rust versions, but in order to format the code, you need a specific nightly toolchain. @ahl, does all this match your recollection?

Yes, @davepacheco, that matches my recollection and understanding.

[ahl]

What do other projects do?

Mostly just run on stable, and if something changes the format output, push a commit that fixes it. I don't tend to experience a lot of churn in formatting between versions.

We've perhaps just been unlucky, but we've hit quite a few changes that subtly changed formatting in stable. This became annoying because CI would run the most recent nightly, but our dev machines had slightly older nightly builds installed.

[davepacheco]

What do other projects do?

Mostly just run on stable, and if something changes the format output, push a commit that fixes it. I don't tend to experience a lot of churn in formatting between versions.

I gather that in this case, there is a specific version required, it's always "whatever's distributed with latest stable", and it's just assumed and not typically documented?

I would love to answer the question: how many times would stable rustfmt have reformatted Dropshot in the last year? The easiest way I can think to do this is to download every stable toolchain from 2020, do a fresh clone of Dropshot, reformat it with the first one, then reformat it with each subsequent one in series, saving commits along the way.

your complex set of changes are now merged with a bunch of spurious formatting changes.

You can have more than one commit, where one contains the formatting changes only.

I was describing a painful thing that happens during a common workflow, not saying there's not a better way to do it. In my experience, I do a bunch of editing, then I run cargo +nightly fmt, repeat, etc., and then accidentally run cargo fmt, and I wind up in the situation I described. Yes, it's avoidable, but it relies on not accidentally running the wrong command at the wrong time.

[davepacheco]

To summarize: looks good. We should update the README too. I can do this in a follow up if you prefer.

[jclulow]

To summarize: looks good. We should update the README too. I can do this in a follow up if you prefer.

I'll take a swing! Thanks.

[davepacheco]

Moved the rustfmt discussion over to #77.