search

dask-contrib / dask-sql

Distributed SQL Engine in Python using Dask
https://dask-sql.readthedocs.io/
MIT License
376 stars 71 forks source link

Pin to rust 1.72 in CI #1333

Closed charlesbluca closed 1 month ago

charlesbluca commented 1 month ago

Looking at https://github.com/rust-lang/rust/issues/125067, it looks like the recent build issues we've been seeing in GPU CI (link) are a result of us using an outdated glibc version with our Rust toolchain.

This PR pins us to rust 1.72 across CI for now to circumvent these failures, which is consistent with the version used in our conda builds.

codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.27%. Comparing base (7600f60) to head (c6ce8e9). Report is 5 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1333 +/- ## ========================================== - Coverage 85.47% 76.27% -9.21% ========================================== Files 77 77 Lines 4242 4231 -11 Branches 790 696 -94 ========================================== - Hits 3626 3227 -399 - Misses 448 855 +407 + Partials 168 149 -19 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bdice commented 1 month ago

Just FYI -- there is a movement towards using {{ stdlib("c") }} instead of sysroot_{{ target_platform }}. See this cuda-feedstock issue for a short description and this conda-forge issue for more details.

I'm not sure if this affects environments, probably only conda-build recipes. Just wanted to share for awareness.

charlesbluca commented 1 month ago

Thanks for the context @bdice! Yeah have seen that migration work and expect it should be relevant if/when this impacts conda builds, though for now it looks like conda-forge only has up to rust 1.77.2 so may be a few weeks before we can check if this fix is needed / works there.

Though I suppose it doesn't hurt to update the recipe here to add this specification and see what hurdles we encounter

jakirkham commented 1 month ago

Have we raised an issue with the rust-feedstock?

charlesbluca commented 1 month ago

rerun tests

charlesbluca commented 1 month ago

Have we raised an issue with the rust-feedstock?

@jakirkham an issue to add {{ stdlib("c") }} to rust's build dependencies, or something else?

jakirkham commented 1 month ago

Would start with rust itself

Narrowing down dependencies may involve some discussion

charlesbluca commented 1 month ago

Filed in https://github.com/conda-forge/rust-feedstock/issues/192