Migrate to the buffer_fixed! macro

[newpavlov]

Depends on RustCrypto/traits#1799.

[tarcieri]

Why are you removing --release from all the CI configs? That should probably go in a separate commit.

There are definitely valid reasons to test release builds, like debug assertion macros having interactions which would cause failures in releases.

If anything we should test both.

[newpavlov]

I think the use of --release flag is a leftover from the time we did not have opt-level = 2 in the workspace Cargo.toml. I don't think it's worth to test both, since with our configuration --release mostly just disables debug assertions.

like debug assertion macros having interactions which would cause failures in releases.

How does it work? Release builds by default remove debug assertions. Modulo compiler bugs and pathologically badly written code (like having side effects inside debug_assert!) if code works with debug assertions, it should continue to work without them.

Either way, I am fine with doing it in a separate PR.

[tarcieri]

pathologically badly written code (like having side effects inside debug_assert!)

It would be good if tests could actually catch cases like this, rather than leaving it to a reviewer to do so

[newpavlov]

I don't think we had any bugs like this in RustCrypto.

[tarcieri]

It's a bugclass we can easily test for and prevent

[newpavlov]

But we do not do it anywhere else. And AFAIK most Rust projects do not do it either (I can not name even a single such project out of my head). The use of --release here is just a historic artifact. I do not think it's worth to double our CI times to test for a bug class which we never encountered in practice.

[tarcieri]

And AFAIK most Rust projects do not do it either (I can not name even a single such project out of my head)

https://github.com/search?q=%22cargo+test+--release%22&type=code

[newpavlov]

Please take a careful look at the search results. Almost all of them have zero relation to CI testing. And it looks like most of them are just a way to run tests with optimizations instead of changing opt-level for dev builds.

I am not saying that were may be projects in the wild which run CI tests both with and without --release, but in my opinion it's most certainly a very uncommon setup.

[tarcieri]

We’re developing high assurance software and test all sorts of other combinations. I am at a loss why you consider this so controversial.

[newpavlov]

Because AFAIK we do not use this combination anywhere else in our repositories. I am also at loss why you are so protective of this historic artifact which was used to just improve CI times. There are other much better ways to improve our CI like running Miri whenever possible, assuring that our code does not contain panics, or running tests on more exotic targets. Running tests with --release looks like a very low value thing to me compared to the cost of doubling CI times (it may be fine in this repo, but would be quite painful for example in the hashes repository).

If you really think that it's a worthwhile idea, then we should discuss it separately and assuming we achieve agreement, gradually apply it to all our repositories.

[tarcieri]

Yes, and it would’ve been nice to discuss in a purpose-dedicated issue instead of being merged in an unrelated PR

[newpavlov]

You can open an issue in the meta repository. Meanwhile #187 makes CI consistent with other repositories and I believe should be merged without waiting for results of the discussion.

[tarcieri]

Opened RustCrypto/meta#23