Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R-universe r-polars build fails because rust-polars is no longer backwards compatible with rustc 1.65.0 stable #208

Closed
sorhawell opened this issue May 10, 2023 · 7 comments · Fixed by #280
Labels
enhancement New feature or request

Comments

@sorhawell
Copy link
Collaborator

sorhawell commented May 10, 2023

R-universe build fails
https://github.com/r-universe/rpolars/actions/runs/4884403053

I tried to downgrade to rust 1.65 stable locally, but I got no errors so far.
Something with simd is mentioned in some of the compiler errrors, but not sure what it is about.

EDIT: In a clean build I was able to reproduce the error by downgrading to rustc 1.65 stable

@sorhawell
Copy link
Collaborator Author

sorhawell commented May 10, 2023

why R-universe fails

Upstream rust-polars has adopted crate argminmax 0.6.1 which shows to not be backwards compatible with rustc 1.65.0 stable but only 1.66 stable. The minimal previous minimal rustc version was 1.62 for polars.

argminmax 0.6.1 currently needs rustc >= 1.66.0 to not give the following compiler error

    Compiling argminmax v0.6.1
  thread 'rustc' panicked at 'assertion failed: key.param_env.is_const()', compiler/rustc_const_eval/src/const_eval/eval_queries.rs:270:5
  stack backtrace:
     0:     0x7f75faf69a43 - <std::sys_common::backtrace::_print::DisplayBacktrace as 
     .
     .
     .

possible solutions (in likely preferred ordering):

  • kindly ask argminmax to become backwards compatible with 1.65 stable
  • kindly ask polars to feature gate argminmax crate
  • kindly ask R universe to upgrade to rustc 1.66.0 (problematic because a CRAN release would then be more time away)
  • fork rust-polars where argminmax was never adopted (could work in the short run, would blow up over time)

other follow-up

  • we ourselves should probably add an "R-universe-like" build to our regular PR checks to discover such backwards compatibility issues earlier.

@sorhawell sorhawell changed the title R-universe build fails since polars 0.5 R-universe r-polars build fails because argminmax is not backwards compatible with rustc 1.65.0 stable May 10, 2023
@sorhawell sorhawell changed the title R-universe r-polars build fails because argminmax is not backwards compatible with rustc 1.65.0 stable R-universe r-polars build fails because rust-polars is no longer backwards compatible with rustc 1.65.0 stable May 10, 2023
@sorhawell
Copy link
Collaborator Author

@eitsupi
Copy link
Collaborator

eitsupi commented May 11, 2023

Thanks for taking a look at this.

Perhaps Debian testing will skip Rust 1.64 and 1.65, so this problem will be solved when we can build this package in CRAN machine anyway.
https://tracker.debian.org/pkg/rustc

we ourselves should probably add an "R-universe-like" build to our regular PR checks to discover such backwards compatibility issues earlier.

I too thought that the handling of SIMD disabled should be tested more.

If someone wants to work on it, one way is to rewrite Cargo.toml by detecting the available Rust state by the configure script like the gifski package, I think.
https://github.com/r-rust/gifski/blob/68911d927fb34c43dede24ce1baab73346e8bf04/configure#L11-L22

@sorhawell
Copy link
Collaborator Author

Perhaps Debian testing will skip Rust 1.64 and 1.65, so this problem will be solved when we can build this package in CRAN machine anyway.

R-universe build can run on 1.65.0 when setting opp-level=1 specifically for argminmax. Probably we should cross our fingers for Debian skipping 1.65, and otherwise we can mitigate issue.

@sorhawell
Copy link
Collaborator Author

seems to work alright now

@eitsupi
Copy link
Collaborator

eitsupi commented Jun 30, 2023

Related to #279

@eitsupi
Copy link
Collaborator

eitsupi commented Jun 30, 2023

Debian testing migration to Rust 1.66 is in progress.
This means that if we want to do a CRAN release, MSRV is sufficient for Rust 1.66.

However, Ubuntu is currently Rust 1.65, so if we consider binary installation with the Posit Package Manager, we will need to resolve this issue and support Rust 1.65.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants