-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Patch ntapi to restore windows build #31961
Conversation
Codecov Report
@@ Coverage Diff @@
## master #31961 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 760 760
Lines 207407 207407
=======================================
+ Hits 169917 169939 +22
+ Misses 37490 37468 -22 |
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.
Thank you! I think this one can be the stopgap! but yeah, we need some proper docs for it.
the only concern: please don't rug 😞
@CriesofCarrots do you have any concerns?
btw, a friendly note: this ntapi patch is based on 0.3.6 (which we're using) + a fixed commit (solana-labs/ntapi@5980bba)
No, I'm okay using the patch for now. |
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.
🪖
@@ -3,7 +3,7 @@ name: release-artifacts-auto | |||
on: | |||
push: | |||
branches: | |||
# - master | |||
- master |
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.
hehe, i included this change as well in this pr... 🙏
* Patch ntapi to restore windows build * Update Cargo.lock... * Add comment for justification of this patching MSxDOS/ntapi#11 MSxDOS/ntapi#12 * Revert "ci: stop windows building on master temporarily (solana-labs#31353)" This reverts commit 2dcdfff. * Use solana-labs fork * Ugh..
* Shift crossbeam comment for upcoming 2nd patch... (#31963) * Patch ntapi to restore windows build (#31961) * Patch ntapi to restore windows build * Update Cargo.lock... * Add comment for justification of this patching MSxDOS/ntapi#11 MSxDOS/ntapi#12 * Revert "ci: stop windows building on master temporarily (#31353)" This reverts commit 2dcdfff. * Use solana-labs fork * Ugh.. * Patch ntapi more thoroughly (#31970) * Patch spl-token-cli build as well... * Patch sbf/Cargo.toml for consistency * Remove --locked for cli-arg based patch... (#31971) * Bump patched ntapi from v0.3.6 to v0.3.7 (#31981) * Revert "ci: stop windows build (#31893)" This reverts commit 30f9e43.
Problem
It's been awhile we temporarily stopped Windows build due to incompatibility between one of transitive deps (
ntapi
) and the solana monorepo's rust toolchain version (v1.69)....:#31353
#31893
Unfortunately, it's not easy to update to
ntapi v0.4.x
(which contains a fix for the incompatibility), due to its being middle of our gigantic dep. graph (beneath our somewhat outdatedtokio
...). On top of ti, the crate maintainer hasn't been responsive with regard to backporting the fix intov0.3.x
.Summary of Changes
Patch
ntapi
: solana-labs/ntapi@5980bba :the cherry-picked commit is quite small; so it's low risk. also, carrying the patch won't incur much maintenance burden onto us because the upstream crate is itself kind of being unmaintained...
all in all, i think this is win for restoring window build to detect any build issue on Windows as soon as possible. Also, Windows' solana v1.16.x binaries will be available again (after bp-ing this to the branch).
the build fix is confirmed at my fork:
https://github.com/ryoqun/solana/actions/runs/5172165189/jobs/9316255905#step:4:655 :