fix: avoid unstable duration_constructors so torchft builds on Rust < 1.91#334
Conversation
… 1.91 Duration::from_mins/from_hours were unstable (duration_constructors, rust-lang/rust#120301) until Rust 1.91, so building on older stable Rust failed with E0658 (reported on 1.88). Replace the four uses with the identical from_secs(...) values so torchft builds on older stable Rust. Closes meta-pytorch#330 Signed-off-by: ajinkyajawale14499 <26092430+ajinkyajawale14499@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
src/net.rscallsDuration::from_mins(1).Duration::from_mins/from_hours(theduration_constructorsfeature, rust-lang/rust#120301) were unstable until Rust 1.91, so buildingtorchft on older stable Rust fails with:
This was reported on Rust 1.88 (#330). CI did not catch it because every workflow installs
--default-toolchain=stable, which is now >= 1.91.The issue suggests documenting Rust 1.91 as the minimum. This PR instead removes the need for it:
from_mins(1)is exactlyfrom_secs(60), so switching to the stable constructor lets torchft buildon the older stable Rust the reporter (and many distro/conda/CI environments) actually have, rather
than forcing everyone onto a bleeding-edge toolchain for a trivial constant.
Changes
The API was used at four sites (one in production, three in tests), all converted to the identical
from_secs(...)value:src/net.rs—from_mins(1)->from_secs(60)(HTTP/2 keep-alive interval)src/timeout.rs(tests) —from_hours(3)/from_mins(3)->from_secs(3 * SECONDS_IN_HOUR)/from_secs(3 * SECONDS_IN_MINUTE), reusing the module's existing constants so the test mirrors theparser it covers
src/lighthouse.rs(test) —from_hours(10)->from_secs(10 * 60 * 60)No behaviour change: each replacement is the same
Duration.Verification
rustup toolchain install 1.88.0main:cargo +1.88.0 check --testsreproducesE0658at all four sites.cargo +1.88.0 check --testscompiles clean (build floor dropped to <= 1.88).cargo +nightly fmt --checkclean;cargo +stable check --testsgreen.grep -rn "from_mins\|from_hours\|from_days" --include='*.rs'returns nothing.Follow-up
If you would still like the minimum Rust version documented explicitly, I am happy to add a
rust-versionfield toCargo.tomlplus a README note in a follow-up. Removing the unstable APIfirst means that floor can stay conservative instead of being pinned to 1.91.
Closes #330