rust-lang / libm

A port of MUSL's libm to Rust.
Other
547 stars 97 forks source link

`libm` version `0.2.9` requires rust upgrade #330

Closed tlovell-sxt closed 3 weeks ago

tlovell-sxt commented 3 weeks ago

When running RUN cargo install subkey using rust 1.76.0, I get the following build error.

error: unsupported output in build script of `libm v0.2.9`: `cargo::rustc-check-cfg=cfg(assert_no_panic)`
Found a `cargo::key=value` build directive which is reserved for future use.
Either change the directive to `cargo:key=value` syntax (note the single `:`) or upgrade your version of Rust.
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.
warning: build failed, waiting for other jobs to finish...
error: failed to compile `subkey v20.0.0`, intermediate artifacts can be found at `/tmp/cargo-installywVOcP`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

Is this expected? Does our dependency that depends on libm need to change - or should 0.2.9 be yanked?

tlovell-sxt commented 3 weeks ago

Looks like it actually requires rust 1.77.0, my bad. Verified this by running

rustup default 1.76 && cargo install subkey --force # fails
rustup default 1.77 && cargo install subkey --force # succeeds

updated the original message accordingly.

Still - should increased rust version requirements necessitate a breaking version increment?

tgross35 commented 3 weeks ago

Bumping MSRV is usually not considered a breaking change in the ecosystem. It is true that it has semver implications; however, the gist is that bundling MSRV bumps with actual breaking changes that require user intervention would be worse for the ecosystem than bumping MSRV in minor versions, and only changing major versions when API breaks. There is some more here https://github.com/rust-lang/api-guidelines/discussions/231.

This crate also doesn't have a MSRV policy. All that being said; upgrading the MSRV wasn't really intentional. If the only thing blocking builds with an older Rust version is :: vs. : in build.rs, feel free to put up a PR that changes this back and adjusts the "stable" job in CI to test with an older version.

tgross35 commented 3 weeks ago

The change was easy and didn't trade anything off, so I applied that. I also documented the MSRV as 1.63 (current Debian stable) and added a CI check. This just released as 0.2.10.

We may still want to bump the version in the future and this will likely be done without a semver-breaking change, as discussed above, but at least it can't happen by accident anymore.

juntyr commented 3 weeks ago

The change was easy and didn't trade anything off, so I applied that. I also documented the MSRV as 1.63 (current Debian stable) and added a CI check. This just released as 0.2.10.

We may still want to bump the version in the future and this will likely be done without a semver-breaking change, as discussed above, but at least it can't happen by accident anymore.

Thank you for the quick fix <3

tgross35 commented 3 weeks ago

Just for future reference, what Rust versions is everyone using this crate with? And what for?

I am wondering how much of its purpose as a standalone crate will be obsoleted once we are eventually able to move the math functions to core.

juntyr commented 3 weeks ago

Just for future reference, what Rust versions is everyone using this crate with? And what for?

I am wondering how much of its purpose as a standalone crate will be obsoleted once we are eventually able to move the math functions to core.

I’m using it through ndarray -> num-traits -> libm (this is an optional alternative to depending on std) and through twofloat -> libm (this is currently non-optional but I think could also use std instead). My own crate has a CI-checked MSRV 1.76.

In no-std projects, my reliance on libm is much greater and moving more functions to core would definitely help there.