-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Introduce automatic ABI maintenance mechanism (1/2; prepare) #10335
Introduce automatic ABI maintenance mechanism (1/2; prepare) #10335
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10335 +/- ##
=========================================
- Coverage 81.4% 81.3% -0.2%
=========================================
Files 290 294 +4
Lines 67610 68228 +618
=========================================
+ Hits 55081 55487 +406
- Misses 12529 12741 +212 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garious / @aeyakovenko - I'm almost 100% sure that ABI management is going to be a thing that both of you will rage about tripping over after it's rolled out (maybe more @aeyakovenko ;) ), so I suggest you both participate in building the shackles. Care to review?
} | ||
::log::warn!("Not testing the abi digest under SOLANA_ABI_BULK_UPDATE!"); | ||
} else { | ||
assert_eq!(#expected_digest, format!("{}", hash), "Possibly ABI changed? Confirm the diff by rerunning before and after this test failed with SOLANA_ABI_DUMP_DIR"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the starting point!
For devx matter, I've taken extra effort to easy the burden, btw. The implementation is really advanced/complex (at least for half-year rust programmer), well worth for its own crate but it's really encapsulated, I believe. For devx, managing abi sanely just boils down to this test assertion: https://github.com/solana-labs/solana/pull/10335/files#r433615921 Basically, this is where you'll start to being tripped over. When you encounter this failure on CI, just run the abi test twice and Or just page @ryoqun or remove As the last resort, if you dare, take the look of the internal this PR is about to add. :) Btw, this pr stranded for several months unfortunately for more important snapshot thing and it evolved for each rebase, I'm pretty sure this mechanism can detect most of abi changes nicely and updating is one-line change for engineers. |
Wow, cool. I'll merge this finally! :) |
0d3fa06
to
c4f259e
Compare
@garious @aeyakovenko Because I got +1, I've merged this preparation pr and master is green at the moment. I guess it's now the last call for actually enabling abi maintenance (or burden). For more concrete example for what's needed by engineers to maintain the abi, please take a look at #8012, which I've rebased already. |
…labs#10335) * Introduce automatic ABI maintenance mechanism * Compile fix... * Docs fix... * Programs compilation fix... * Simplify source credit Co-authored-by: Michael Vines <[email protected]> * Cargo.lock... Co-authored-by: Michael Vines <[email protected]>
Problem
ABI is hard to track.
Summary of Changes
This is the preparation pr for the next rollup of
frozen_abi
Part of #7738