sdk: Create new Cargo workspace for sdk/ #4685
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinheavey
reviewed
Jan 29, 2025
| lto = "thin" | ||
|
|
||
| [workspace] | ||
| members = [ |
There was a problem hiding this comment.
thoughts on making an sdk/crates dir so everything can just be moved in there and this field can be members = ["crates/*"]?
There was a problem hiding this comment.
Personally, I'm going to get way more annoyed about having to type crates (or c<TAB>) every single time I want to open a file in the repo over having to update this field whenever a new crate is added.
But I might be in the minority.
joncinque
force-pushed
the
sdkworkspace
branch
5 times, most recently
from
b34e49d to
56450ad
Compare
joncinque
force-pushed
the
sdkworkspace
branch
6 times, most recently
from
f5519d5 to
6c40fe4
Compare
joncinque
force-pushed
the
sdkworkspace
branch
from
6c40fe4 to
afb3c99
Compare
yihau
previously approved these changes
Feb 1, 2025
joncinque
added a commit
that referenced
this pull request
Feb 3, 2025
#### Problem During the work to extricate the sdk from the rest of the monorepo in #4685, there were a couple of issues with CI. First, the `dummy-for-ci-check` feature doesn't exist in any sdk crate, so the `clippy` and `check` steps were failing when trying to activate the feature. Next, the frozen-abi tests were failing because the sdk doesn't activate `solana-packet`'s frozen-abi feature when its frozen-abi is active. #### Summary of changes Add a `dummy-for-ci-check` to `solana-sdk`, and correctly activate `frozen-abi` on `solana-packet`.
joncinque
force-pushed
the
sdkworkspace
branch
3 times, most recently
from
a688edf to
9e62b32
Compare
|
@yihau thanks for the quick review on this! I've updated the PR description to make the differences clear -- I hope it's ok that we're not uploading bench and coverage reports just yet. I wanted to figure that out in the new repo once it was ready. |
#### Problem We want to move everything under `sdk/` into a standalone repository, but all of the crates are currently part of the monorepo workspace, which makes the extraction impossible. #### Summary of changes Remove all crates in `sdk/` from the top-level workspace, but keep the local dependency. Create a new `Cargo.toml` for the sdk workspace along with all required dependencies and patches.
#### Problem The monorepo contains a lot of testing steps for all of the packages. Once the sdk lives in its own repos, it will need to run adequate CI, just as before. #### Summary of changes Add all of the steps run over the sdk in the monorepo as scripts in `sdk/scripts`, along with a setup step and workflow file. These steps include: * checking crate ownership (check-crates.sh) * running hack checks (check-hack.sh) * cargo check with nightly toolchain (check-nightly.sh) * clippy (check-clippy.sh) * crate sorting in Cargo.toml (check-sort.sh) * dcou checks (check-dev-context-only-utils.sh) * rustfmt check (check-fmt.sh) * auditing crates (check-audit.sh) * miri tests (test-miri.sh) * frozen-abi tests (test-frozen-abi.sh) * tests with stable toolchain (test-stable.sh) * wasm tests (test-wasm.sh) * coverage tests (test-coverage.sh) * shellcheck (check-shell.sh) * sanity checks (check-porcelain.sh and check-nits.sh) * publish order (order-crates-for-publishing.py) * benches (test-bench.sh) A few differences with the monorepo testing: * coverage reports are not uploaded * bench results are not uploaded * version check is omitted, since the sdk will version crates separately * ssh key check is omitted, since there's no net scripts Future work: * downstream tests into solana-program repos * semver checks * spellchecking
joncinque
force-pushed
the
sdkworkspace
branch
from
9e62b32 to
784445b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We want to move everything under
sdk/into a standalone repository, but all of the crates are currently part of the monorepo workspace, which makes the extraction impossible.Summary of changes
Each commit should make the story clear:
f521e84: Remove all crates in
sdk/from the top-level workspace, but keep the local dependency. Create a newCargo.tomlfor the sdk workspace along with all required dependencies and patches69c949d: Remove unused dependencies in the sdk's Cargo.toml
85d31b3: Update the lockfile, automated
5b6c6f6: Re-do CI through a new set of scripts under
sdk/scripts. This is another monster, and includes the following steps:784445b: Now that the sdk is tested with its own GitHub Actions workflow, don't run its test in normal CI
Important! There are a few differences with the current monorepo testing:
And there is some future work on the CI side:
Note: this builds on #4675 and #4731, leaving in draft until those landThis is good to go!