[ci] check formatting

[gnzlbg]

No description provided.

[BurntSushi]

I guess this PR should include a run of rustfmt over the code?

[gnzlbg]

@BurntSushi It currently runs cargo fmt -- --write-mode=diff which returns an error if the formatting is not correct. EDIT: I got it wrong. Yes, I want to add a commit to this PR with a run of rustfmt but I wanted to check first if it installed properly in all targets (clippy is currently failing).

[gnzlbg]

So it seems that rustfmt is also failing to install. @alexcrichton does this happen often (that clippy and rustfmt fail to install on nightly)? Is there a better way to deal with this?

[gnzlbg]

@BurntSushi what do you think about the way it's done now? I could do the same for clippy.

[BurntSushi]

I guess technically that's OK, but there is a fundamental tension here if we're trying to use code that doesn't reliably build:

1. If we fail the build on build failures of said tools, then merging PRs can become quite annoying.
2. If we don't fail the build, then nobody is ever going to look at real failures, which kind of defeats the purpose of having it in the first place.

IMO, if we're going to put stuff into CI, then it needs to be reliable.

I would definitely be curious to hear @alexcrichton's thoughts on the matter! Perhaps there is another way.

[gnzlbg]

I also want to hear what @alexcrichton has to say.

In particular, even if clippy and rustfmt work on stable one day, will we be able to use them? This library uses unstable Rust features by design.

[gnzlbg]

It seems that cargo fmt --all does not format stdsimd-test, nor the macros inside it...

[gnzlbg]

So I've added a commit that reformats the whole library so that you guys can check it out. I can live with most of it, but some of the x16 and x32 vectors being reformatted into 1 element per line is a bit ridiculous in my opinion. I could probably live with it but... I wouldn't say it looks nice.

[BurntSushi]

Ug. Yeah. I don't like that at all. I could probably deal with it if we only ever worked with 128 bit vectors, but this formatting style is going to become unbearable with 256 bit and 512 bit vectors. We really need to be able to control line breaking for those.

[gnzlbg]

Clang format has a very convenient way to disable formatting in these cases:

// clang-format off
 ... do whatever you want ...
// clang-format on

I'll try to do the same in these places.

[gnzlbg]

@BurntSushi so i've fixed all those places, there is a #[rustfmt_skip] attribute that does the same as the clang-format options.

[alexcrichton]

Yeah unfortunately I don't have a lot of experience here in terms of running rustfmt on CI. From what I hear though the output is still being tweaked over time so an update of rustfmt one day could otherwise break the CI here.

In terms of style I've personally sort of given up at this point and am just willing to accept whatever rustfmt does. I won't like some things but I'm pretty sure I'll like others, and having it all happen automatically tends to make it easier to just get used to than try to work around.

I'm sort of up in the air about whether to enable it or not here. I could go either way! I don't have a particularly strong preference.

[BurntSushi]

I'm willing to give this a try, with #[rustfmt_skip], primarily because this crate contains a lot of repetitive code and it would be nice to not worry about formatting. However, I would like this to be on a trial basis.

[gnzlbg]

@alexcrichton could you check what is going on here?

I have installed the latest rustup and the latest rustfmt-nightly.

Doing cargo fmt on my machine produces a different formatting than in the build bot (which should also be using the same rustfmt version).

[gnzlbg]

Also one normal build is failing with error: An unknown error occurred, @BurntSushi could you maybe restart it (it's the first one; the RUSTFMT build failing is ok, the formatting is different for some unknown reason)?

[gnzlbg]

Duh... there were new merges to master while I was working on this, which introduced formatting differences. All should be green now :)

Keeping this branch in sync with master is a PITA because every commit to master makes this branch fail.

[alexcrichton]

I think instead of this conditional you should be able to just specify script: above, right? (in the include: matrix)

[gnzlbg]

What do you mean? I added the conditional to avoid running the docker tests in the build that just checks formatting. I don't like this much either (in particular if we add clippy on top).

Maybe a better way could be to move this to a ci/run.sh script that reads the environment variables (e.g. RUSTFMT, CLIPPY, etc) and does the right thing.

[gnzlbg]

Anyways since basically every commit breaks this one, and updating it sometimes requires manual edits, I'd rather merge it first, and improve the build scripts later.

[alexcrichton]

I believe you can avoid this if/else statement if you add a script: directive to the env: that you added in this same file

[gnzlbg]

I think i've fixed this here and in the enable_clippy PR. The build is green except for i586, is that failure "normal" ?

[gnzlbg]

This is all green, if we want to tune the formatting, we should do so before merging this.

[gnzlbg]

@BurntSushi thoughts on this and the clippy branches?

[gnzlbg]

@alexcrichton @BurntSushi what should I do about this and the clippy commit ?

[alexcrichton]

@gnzlbg I'm ok with both, but I figured @BurntSushi should sign off as well

[BurntSushi]

I'm OK too. Looks like this PR needs to be updated yet again though... Sorry :-(

[gnzlbg]

It will proceed to get this into a mergeable state (takes 15min), please don't merge anything else in the mean time.

[gnzlbg]

@BurntSushi so i've updated this and rebased, can we merge this?

[gnzlbg]

@BurntSushi seems like the osx build stalled but all should be green here.