fix: install cargo udeps differently #82
Conversation
giladchase
force-pushed
the
gilad/change-udeps-installation
branch
from
94c26d6 to
1e87f91
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
=======================================
Coverage 54.54% 54.54%
=======================================
Files 11 11
Lines 275 275
Branches 275 275
=======================================
Hits 150 150
Misses 123 123
Partials 2 2 ☔ View full report in Codecov by Sentry. |
giladchase
force-pushed
the
gilad/change-udeps-installation
branch
2 times, most recently
from
817d7bc to
2743e43
Compare
giladchase
force-pushed
the
gilad/change-udeps-installation
branch
4 times, most recently
from
8d67c2a to
c80e291
Compare
- Instead of compiling from source, use a cached cargo install github action (which appears to be cached by default!), and then run udeps from cargo. - This is the recommended installation method from udeps' readme. - Motivation: Previous method of installation seemed to hit compilation errors [here] (https://reviewable.io/reviews/starkware-libs/mempool/42#-), that weren't reproducible when running through cargo udeps. - NOTE: The github action used for cargo-install only has ~100 stars, which isn't a lot, but I found a few well-used projects that use them, like tokio-rs/prost and wez/wezterm, so it's probably OK. - ALTERNATIVE: switching from udeps to machete. The cairo team did and they're pleased with the results.
giladchase
force-pushed
the
gilad/change-udeps-installation
branch
6 times, most recently
from
7d4c1fb to
3a396a7
Compare
There was a problem hiding this comment.
ALTERNATIVE: switching from udeps to machete. The cairo team did and they're pleased with the results.
@elintul thinks we should consider moving to machete, it is faster and should be less vulnerable to bugs like the one we hit.
Machete doesn't compile the repo like udeps (which is why it's less vulnerable to compilation issues), it just greps for missing dependencies, which they do surprisingly well according to compiler team.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @dafnamatsry and @Yael-Starkware)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @dafnamatsry)
instead of compiling from source, use a cached
cargo installgithub action (which appears to be cached by default!), and then run udeps from cargo.this is the recommended installation method from udeps' readme.
motivation: Previous method of installation seemed to hit compilation errors [here] (https://reviewable.io/reviews/starkware-libs/mempool/42#-), that weren't reproducible when running through
cargo udeps.NOTE: The github action used for cargo-install only has ~100 stars, which isn't a lot, but I found a few well-used projects that use them, like tokio-rs/prost and wez/wezterm, so it's probably OK.
ALTERNATIVE: switching from udeps to machete. The cairo team did and they're pleased with the results.
This change is