Skip to content

Conversation

@kevinheavey
Copy link
Contributor

@kevinheavey kevinheavey commented Feb 8, 2025

This is a draft PR for now as it needs a mollusk update Ready for review

One breaking change: the re-export of solana-program is replaced by a re-export of a few types. Hopefully for most users the re-export works as before

@kevinheavey
Copy link
Contributor Author

Ok, tests should pass now as I've switched to an updated fork of mollusk

@kevinheavey kevinheavey marked this pull request as ready for review February 16, 2025 21:32
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me, just the question about whether we should break the re-exports

Comment on lines +16 to +38
/// Export current sdk types for downstream users building with a different sdk
/// version
pub mod solana_program {
#![allow(missing_docs)]
pub mod entrypoint {
pub use solana_program_error::ProgramResult;
}
pub mod instruction {
pub use solana_instruction::{AccountMeta, Instruction};
}
pub mod program_error {
pub use solana_program_error::{PrintProgramError, ProgramError};
}
pub mod program_option {
pub use solana_program_option::COption;
}
pub mod program_pack {
pub use solana_program_pack::{IsInitialized, Pack, Sealed};
}
pub mod pubkey {
pub use solana_pubkey::{Pubkey, PUBKEY_BYTES};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this is needed? With the crates now split up and following semver, I hope that we can stop doing these re-exports in program crates generally.

We can do a new major version for spl-token no problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would still like to break fewer things in a major release. This way hopefully very few people will need to change code

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough! We can revisit then

@joncinque joncinque merged commit 6d18ff7 into solana-program:main Feb 26, 2025
12 checks passed
joncinque added a commit to joncinque/token-2022 that referenced this pull request Mar 4, 2025
#### Problem

The token-2022 program still uses solana-program and solana-sdk, which
are heavy dependencies.

#### Summary of changes

The component crates exist to do everything needed in token-2022, so we
can remove solana-program usage entirely.

NOTE: since some spl-token-2022's dependencies still use solana-program,
there is no huge gain just yet. That will happen when we upgrade the
upstream dependencies and then upgrade token-2022 to use those.

Also NOTE: this is a breaking change since we've removed the re-export
of the sdk types. Considering we're now respecting semver, this
shouldn't be a problem, but we did add a version of re-exports in
solana-program/token#26, so maybe we should do
the same thing here. Let me know!
joncinque added a commit to solana-program/token-2022 that referenced this pull request Mar 5, 2025
* program: Remove solana-program / sdk

#### Problem

The token-2022 program still uses solana-program and solana-sdk, which
are heavy dependencies.

#### Summary of changes

The component crates exist to do everything needed in token-2022, so we
can remove solana-program usage entirely.

NOTE: since some spl-token-2022's dependencies still use solana-program,
there is no huge gain just yet. That will happen when we upgrade the
upstream dependencies and then upgrade token-2022 to use those.

Also NOTE: this is a breaking change since we've removed the re-export
of the sdk types. Considering we're now respecting semver, this
shouldn't be a problem, but we did add a version of re-exports in
solana-program/token#26, so maybe we should do
the same thing here. Let me know!

* Fixup tests to run program in bpf mode
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.

2 participants