Skip to content
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

Separate the "program" feature of solana-sdk into a new crate called solana-program #12989

Merged
merged 7 commits into from
Oct 24, 2020

Conversation

mvines
Copy link
Member

@mvines mvines commented Oct 19, 2020

The "program" feature of solana-sdk needs to go away.

Source backwards compatibility is retained by making solana-sdk re-export solana-program definitions, and the solana-sdk "program" feature becomes vestigial.

This work will allow a program to declare solana-program as a [dependency] and then the full-blown solana-sdk as a [dev-dependency], enabling access to useful things like BanksClient directly from program unit tests.

@mvines mvines added the noCI Suppress CI on this Pull Request label Oct 19, 2020
@mvines mvines force-pushed the ps branch 4 times, most recently from a809c2b to 1038cdd Compare October 20, 2020 03:07
@mvines mvines force-pushed the ps branch 2 times, most recently from de9cbc2 to f158914 Compare October 21, 2020 04:21
@mvines mvines removed the noCI Suppress CI on this Pull Request label Oct 22, 2020
@mvines mvines force-pushed the ps branch 10 times, most recently from f3f417a to b66cf1c Compare October 22, 2020 18:24
@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

Merging #12989 into master will decrease coverage by 0.0%.
The diff coverage is 76.8%.

@@            Coverage Diff            @@
##           master   #12989     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         367      372      +5     
  Lines       86406    86522    +116     
=========================================
+ Hits        71023    71066     +43     
- Misses      15383    15456     +73     

@mvines
Copy link
Member Author

mvines commented Oct 22, 2020

Getting close! The program feature is now basically purged from all monorepo programs by switching from solana-sdk to solana-program-sdk

@mvines mvines requested a review from jackcmay October 23, 2020 16:40
@@ -4,47 +4,36 @@
// Allows macro expansion of `use ::solana_sdk::*` to work within this crate
extern crate self as solana_sdk;

Copy link
Member Author

Choose a reason for hiding this comment

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

Changes to sdk/src/lib.rs are interesting to review

pub mod serialize_utils;

// Modules not usable by on-chain programs
#[cfg(feature = "everything")]
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the #[cfg(feature = "everything")]s into inner cfgs in each file to clean up lib.rs

Copy link
Contributor

Choose a reason for hiding this comment

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

Though does require searching through all the files to figure out if everything.

#[deprecated(
since = "1.4.3",
note = "use solana_program_sdk::entrypoint::entrypoint instead"
)]
macro_rules! entrypoint {
Copy link
Member Author

Choose a reason for hiding this comment

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

This sucks a little. I had to keep an entrypoint macro in here to avoid breaking users

Copy link
Contributor

Choose a reason for hiding this comment

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

Which users are you worried about, wouldn't anyone picking this sdk need to start using solana-program-sdk anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

wouldn't anyone picking this sdk need to start using solana-program-sdk anyway?

Currently you can just upgrade solana-sdk and it'll work (with some deprecated compiler warning). I expect this will be the normal upgrade path for downstream:

  1. Next year upgrade to a new solana-sdk
  2. Notice the deprecated compiler warnings
  3. Switch to solana-program-sdk in 5 years

@@ -89,7 +89,7 @@ pub(crate) struct ComputedBankState {
pub pubkey_votes: Arc<PubkeyVotes>,
}

#[frozen_abi(digest = "2ZUeCLMVQxmHYbeqMH7M97ifVSKoVErGvRHzyxcQRjgU")]
#[frozen_abi(digest = "31bz55FBVWtVQjPWz8eeyduFXMCFhVZx5uL8wyLk3hht")]
Copy link
Member Author

Choose a reason for hiding this comment

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

A bunch of frozen_abi digest updates due to files moving around, not an issue....

@mvines
Copy link
Member Author

mvines commented Oct 23, 2020

This is ready to land. Steps:

@mvines mvines force-pushed the ps branch 4 times, most recently from 9384a50 to 18c73e1 Compare October 23, 2020 21:15
@mvines mvines changed the title Separate the "program" feature of solana-sdk into a new crate called solana-program-sdk Separate the "program" feature of solana-sdk into a new crate called solana-program Oct 24, 2020
@mvines mvines merged commit a495684 into solana-labs:master Oct 24, 2020
@mvines mvines added the v1.4 label Oct 24, 2020
@mvines mvines deleted the ps branch October 24, 2020 15:42
mergify bot added a commit that referenced this pull request Oct 24, 2020
…d `solana-program` (bp #12989) (#13131)

* Add solana-program-sdk boilerplate

(cherry picked from commit 3718771)

# Conflicts:
#	sdk/Cargo.toml

* Initial population of solana-program-sdk

(cherry picked from commit 63db324)

# Conflicts:
#	Cargo.lock

* Port programs to solana-program-sdk

(cherry picked from commit fe68f7f)

# Conflicts:
#	programs/bpf/Cargo.lock
#	programs/bpf/rust/128bit/Cargo.toml
#	programs/bpf/rust/128bit_dep/Cargo.toml
#	programs/bpf/rust/alloc/Cargo.toml
#	programs/bpf/rust/call_depth/Cargo.toml
#	programs/bpf/rust/custom_heap/Cargo.toml
#	programs/bpf/rust/dep_crate/Cargo.toml
#	programs/bpf/rust/deprecated_loader/Cargo.toml
#	programs/bpf/rust/dup_accounts/Cargo.toml
#	programs/bpf/rust/error_handling/Cargo.toml
#	programs/bpf/rust/external_spend/Cargo.toml
#	programs/bpf/rust/instruction_introspection/Cargo.toml
#	programs/bpf/rust/invoke/Cargo.toml
#	programs/bpf/rust/invoked/Cargo.toml
#	programs/bpf/rust/iter/Cargo.toml
#	programs/bpf/rust/many_args/Cargo.toml
#	programs/bpf/rust/many_args_dep/Cargo.toml
#	programs/bpf/rust/noop/Cargo.toml
#	programs/bpf/rust/panic/Cargo.toml
#	programs/bpf/rust/param_passing/Cargo.toml
#	programs/bpf/rust/param_passing_dep/Cargo.toml
#	programs/bpf/rust/rand/Cargo.toml
#	programs/bpf/rust/ristretto/Cargo.toml
#	programs/bpf/rust/sanity/Cargo.toml
#	programs/bpf/rust/sha256/Cargo.toml
#	programs/bpf/rust/sysval/Cargo.toml

* Only activate legacy program feature for the solana-sdk crate

(cherry picked from commit 85c51f5)

* Run serum-dex unit tests

(cherry picked from commit 92ce381)

* Rename solana-program-sdk to solana-program

(cherry picked from commit dd711ab)

# Conflicts:
#	programs/bpf/rust/128bit/Cargo.toml
#	programs/bpf/rust/128bit_dep/Cargo.toml
#	programs/bpf/rust/alloc/Cargo.toml
#	programs/bpf/rust/call_depth/Cargo.toml
#	programs/bpf/rust/custom_heap/Cargo.toml
#	programs/bpf/rust/dep_crate/Cargo.toml
#	programs/bpf/rust/deprecated_loader/Cargo.toml
#	programs/bpf/rust/dup_accounts/Cargo.toml
#	programs/bpf/rust/error_handling/Cargo.toml
#	programs/bpf/rust/external_spend/Cargo.toml
#	programs/bpf/rust/instruction_introspection/Cargo.toml
#	programs/bpf/rust/invoke/Cargo.toml
#	programs/bpf/rust/invoked/Cargo.toml
#	programs/bpf/rust/iter/Cargo.toml
#	programs/bpf/rust/many_args/Cargo.toml
#	programs/bpf/rust/many_args_dep/Cargo.toml
#	programs/bpf/rust/noop/Cargo.toml
#	programs/bpf/rust/panic/Cargo.toml
#	programs/bpf/rust/param_passing/Cargo.toml
#	programs/bpf/rust/param_passing_dep/Cargo.toml
#	programs/bpf/rust/rand/Cargo.toml
#	programs/bpf/rust/ristretto/Cargo.toml
#	programs/bpf/rust/sanity/Cargo.toml
#	programs/bpf/rust/sha256/Cargo.toml
#	programs/bpf/rust/sysval/Cargo.toml

* Update frozen_abi hashes

The movement of files in sdk/ caused ABI hashes to change

(cherry picked from commit a495684)

* Resolve merge conflicts

Co-authored-by: Michael Vines <[email protected]>
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