Skip to content

Conversation

@buffalojoec
Copy link
Contributor

This is just an experimental PR to open up discussion.

Both the Solana SDK and the Solana-Program SDK have been broken up into many tiny crates, and switching over to these in any Rust project drastically improves compile time. Furthermore, building client libraries without the kitchen sink solana-program or solana-sdk dependencies won't bog down compile times for any other libraries or applications built on top of them.

This PR is potentially the first in very many to bust up the Solana SDKs in Codama's generated Rust clients.

@buffalojoec buffalojoec requested a review from lorisleiva April 7, 2025 04:50
@changeset-bot
Copy link

changeset-bot bot commented Apr 7, 2025

⚠️ No Changeset found

Latest commit: e556541

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@buffalojoec
Copy link
Contributor Author

buffalojoec commented Apr 7, 2025

@lorisleiva @febo I'm mostly looking to open this up for discussion, but I think this can actually land as-is. We might be able to incrementally extract child crates like so going forward, if this is the direction we want to take. I think the version bump is the only major issue.

Regardless, what do you think about going in this direction for Codama? I feel like it's something that was bound to happen anyway, since most people are moving away from solana-sdk and solana-program. It would really stink for generated clients to be stuck with the kitchen sinks.

Also, worst case, developers can always adjust their client library's manifest themselves, so if they need more child crates, or if they want to use the kitchen sinks for some reason, they can do so. We don't necessarily have to make libraries like solana-program opt-in at the renderer level, since they depend on the child crates, so there shouldn't be dependency mismatches. The only drawback is a potentially cluttered manifest, but I'm not convinced we should care too much about that.

IMO, Codama should generate clients with the lightest crates possible, and let developers build on those, either through renderer configurations or additional developer-added code.

What do you guys think?

@buffalojoec buffalojoec marked this pull request as ready for review April 18, 2025 02:44
@lorisleiva
Copy link
Member

Hey Joe, sorry for the late reply, I meant to answer later and completely forgot to got back to it. I think this makes sense. I'd love to have @febo's input on this though since I think there's gonna be some slight changes in the core crates that provide types such as Pubkey.

Copy link
Contributor

@febo febo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@febo
Copy link
Contributor

febo commented May 6, 2025

@buffalojoec Let's get all individual crates integrated and remove solana-program dependency, then we can publish a new version of the renderer.

@buffalojoec
Copy link
Contributor Author

@lorisleiva @febo thanks guys! I think the whole effort is a bit of a bear, but maybe we can do it over time? If at any time a patch needs to go out for some other reason, having just Pubkey split off won't cause any trouble.

Sounds like I should merge this then?

@lorisleiva
Copy link
Member

@buffalojoec Actually, could we just wait on this until I decouple the renderer's versioning with the rest of Codama?

One annoying side effect of bundling renderers with the CLI and then including that CLI in the main codama library is that now all packages are linked. So a breaking change upgrade on the Rust renderer will have to force a breaking change upgrade on the entire of Codama even if nothing changed with the nodes.

I'm planning on fixing this by not including the Codama CLI in the main codama library anymore and people will just need to get it from @codama/cli instead.

But also since this is a Rust renderer breaking change, it would be a net benefit to do all the crate changes in one big breaking change upgrade, right?

@buffalojoec
Copy link
Contributor Author

buffalojoec commented May 7, 2025

@lorisleiva OK, no worries. I'm not blocked on this for anything, and I agree I think it makes sense to do it all in one swing anyway.

Since the changes should be all within the manifests and imports sections, we might be able to make a branch and just continue to add support for different "mini" crates until eventually we can evict solana-program and solana-sdk, then look at one big PR.

The idea would be to occasionally rebase it on top of main whenever we need to.

Wdyt?

@lorisleiva
Copy link
Member

@buffalojoec Yeah I think having a feature branch for this makes sense. We also have Graphite for Codama (free as their open source license) so we could also do a big ol' PR stack with Graphite and merge the lot when we're ready. Up to you though, I'm happy with either strategies.

@buffalojoec
Copy link
Contributor Author

@lorisleiva tbh I don't have Graphite setup anymore, so how about we go with the branch for now, and when we are looking ready to prepare a merge we can decide if we want to make each commit a Graphite branch? I've just been working with my commits as if they were Graphite branches anyway.

If you're down, you could cut a new branch off the current head and change the base here, then we can merge it!

@lorisleiva lorisleiva changed the base branch from main to renderers-rust/use-granular-crates May 8, 2025 16:45
@lorisleiva
Copy link
Member

Sounds good! 👌

@lorisleiva lorisleiva merged commit ae3ca97 into codama-idl:renderers-rust/use-granular-crates May 8, 2025
4 checks passed
@github-actions github-actions bot mentioned this pull request Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants