Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

docs: Update backwards compatibility policy for Rust SDK #32655

Closed
wants to merge 3 commits into from

Conversation

joncinque
Copy link
Contributor

Problem

We've had loads of issues with backwards compatibility with the 1.16 release, see #32503 for some more info. Semver could help, but would likely make things much worse.

Summary of Changes

As a first crack at clarifying the backwards compatibility policy for the Rust SDK that balances stability with agility, I've put together some updates based on conversations with @ilya-bobyr -- once he approves, we can circulate this more widely internally, and then externally. This will be kept in draft until it's ready for a larger public.

Fixes #

@joncinque joncinque requested a review from ilya-bobyr July 28, 2023 17:03
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
Comment on lines 202 to 203
Additionally, the types in `solana-program` used by both your crate and your
dependency are treated as the same.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true.
There is nothing special about direct and indirect dependencies.
Each crate just lists a set of dependencies, that may have dependencies of their own.
And the same compatibility rules are used to determine if a single instance of a crate can be used to satisfy several requirements, or if multiple versions are necessary.

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'll remove this whole part as you suggested previously, since there's no need for us to describe cargo

Comment on lines 228 to 231
On the other hand, types from crates with different major versions are treated as
completely different types. If `my-crate` declares `solana-program = "2"` and
`my-dependency` declares `solana-program = "1"`, then the preceding code *will not*
compile.
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally, programs are not used as dependencies :P
Maybe rephrase, using solana-sdk as the example crate?
This way it would be closer to the practical problem at hand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solana-sdk can't be used on-chain, so on-chain programs must use the types from the solana-program crate, and all of the problems from the upgrade stem from the solana-program crate.

So if we want to be practical, solana-program is the better example. But if you find it very unclear, we can change to solana-sdk.

docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
By ensuring that projects will never immediately break, and giving developers
one year to update, this model will keep some stability while avoiding stagnation.

#### But why was the upgrade to 1.16 so difficult?
Copy link
Contributor

Choose a reason for hiding this comment

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

I can understand how this is close to your heart, but do you really think this discussion belongs to the policy doc? :P

We have issues with all the details, and going forward it would not matter any more, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah you're absolutely right 😅 I wanted to provide some historical record so we know what motivated this change. What do you think about adding it as a post-mortem in #32503 ?

Comment on lines 44 to 46
The Rust SDK will not have any future `MAJOR` releases, and will stay on `1.X` to
provide a better developer experience. More details in
[Why not just use semver](#why-not-just-use-semver).
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this is a bit too restrictive.
There seems to be no reason to say we will never increase the major release number.
It does seem quite unlikely at the moment.
But things may change.

We do want to be explicit about the currently chosen default policy of staying on 1.x.
So we can, maybe, say something along the lines of:

We do not expect MAJOR version to be changed. Any breaking changes will be done though the deprecation process described in [...] in a MINOR version.

If you think it is too subtle, it is probably fine to keep the more assertive "no MAJOR version updates" statement as well.

Also, this paragraph seems to contradict the previous one, that explain when happens in a major version update. If we are saying they never happen, there is no point in this explanation.

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 wonder if this is a bit too restrictive.

You're right, I think I was a little too inspired by our chats.

Also, this paragraph seems to contradict the previous one, that explain when happens in a major version update. If we are saying they never happen, there is no point in this explanation.

And you're right about the contradiction -- this document also covers RPC endpoints, and I didn't want to have the policy apply to both RPC and SDK.

I'll make it clearer which parts cover RPC and which parts cover the SDK.

@joncinque
Copy link
Contributor Author

This should be ready for another look

@joncinque joncinque requested a review from ilya-bobyr August 4, 2023 12:29
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/src/developing/backwards-compatibility.md Outdated Show resolved Hide resolved
@joncinque
Copy link
Contributor Author

Thanks for your feedback again, it should be addressed now

We would like all releases to be 100% backwards compatible, allowing everyone
to upgrade at any time with no effort. This model, however, would put overly
strict constraints on the development of the Rust SDK crates, considering we cannot
in practice increase the major version.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand. All the reasons for a different method than semver here seems to lead to that you cannot in practice increase the major version

Why is that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was covered in a previous commit, but I removed it. Adding it here for more motivation:

Cargo will force all crates in a build to use the same major version
of a dependency. For example, if your crate my-crate declares solana-program = "1.10",
and a dependency my-dependency declares solana-program = "1.16", both your crate and your
dependency will be built with version 1.16, at least.

Additionally, the types in solana-program used by both your crate and your
dependency are treated as the same.

For example, if solana-program has a struct declared as:

#[derive(PartialEq)]
pub struct Pubkey(pub [u8; 32]);

And my-dependency has:

pub const DEPENDENCY_PUBKEY = Pubkey([1; 32]);

Then my-crate can say:

use solana_program::Pubkey;
let my_pubkey = Pubkey([1; 32]);
if my_pubkey == my_dependency::DEPENDENCY_PUBKEY {
    println!("They're the same");
}

On the other hand, types from crates with different major versions are treated as
completely different types. If my-crate declares solana-program = "2" and
my-dependency declares solana-program = "1", then the preceding code will not
compile.

To get this code to compile, you need to create functions to convert between the
types in v1 and v2. And whenever v3 is released, the entire problem becomes multiplied.

But either way, I don't think we should move forward with changing this. I'll provide an update in the main semver issue.

@joncinque
Copy link
Contributor Author

Given the strong responses in #32503, it's clear that we need to work hard and gain back some goodwill from downstream developers.

Since program-runtime v2 is aiming to be close by the end of the year, and we need a whole new program SDK for that, we can bump the major version then or create a whole new crate. In the meantime, we can keep this policy, add more downstream targets, and stop breaking people.

@joncinque joncinque closed this Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants