Skip to content

[docs] Added documentation for package versioning - #11527

Merged
ronny-mysten merged 8 commits into
mainfrom
aw/versioning-docs
May 2, 2023
Merged

[docs] Added documentation for package versioning#11527
ronny-mysten merged 8 commits into
mainfrom
aw/versioning-docs

Conversation

@awelc

@awelc awelc commented Apr 30, 2023

Copy link
Copy Markdown
Contributor

Description

As a follow-up to #11253, added some additional info on package upgrades/versioning that (I believe) is not yet covered anywhere.

I feel like perhaps we should pull some introductory material on upgrade policies from https://github.com/MystenLabs/sui/blob/main/doc/src/build/custom-upgrade-policy.md (@amnn?) but I did not want to make changes that are too disruptive at this point.

@tzakian - please chime in if the description of the compatibility checks, admittedly pretty short, is sufficient or if we should add/change something right away.


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

@awelc
awelc requested review from amnn and tzakian April 30, 2023 21:59
@awelc awelc self-assigned this Apr 30, 2023
@vercel

vercel Bot commented Apr 30, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) May 2, 2023 3:01am
explorer-storybook ⬜️ Ignored (Inspect) May 2, 2023 3:01am
sui-wallet-kit ⬜️ Ignored (Inspect) May 2, 2023 3:01am
wallet-adapter ⬜️ Ignored (Inspect) May 2, 2023 3:01am

@github-actions github-actions Bot added the Type: Documentation Improvements or additions to documentation label Apr 30, 2023

@amnn amnn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this new content @awelc ! It will definitely be helpful for people navigating compatibility. I think the various sections of this document might find their best homes in some existing pages. Here's my take on that, but I'm also curious to get @ronny-mysten or @randall-Mysten's take on that:

  • The intro and section about versions seems to go with the existing dependency overrides topic, perhaps in its own "Versions" section.
  • Alternatively, there is an existing document called "Object and Package Versioning" that this section might belong in.
  • The section on version management, I would suggest renaming to "Compatibility" and putting in the document about custom upgrade policies.

If this seems reasonable, rather than drafting the docs in-place, let's make a document in docs/in-progress, land that, and then stage it in the places we decide.

Comment thread doc/src/build/package-versioning.md Outdated
[dependencies]
another_pkg = { git = "https://github.com/another_pkg/another_pkg.git" , version = "2.0.0"}
```
These fields in the manifest file, however, at this point are only used for user-level documentation purposes and are ignored by the build or publish/upgrade commands. Another way of looking at it is that if a developer publishes a package with a certain package version in the manifest file and then modifies and re-publishes the same package with a different version (using publish command rather than upgrade command), these two packages will be considered different packages, rather than on-chain versions of the same package, and recognized as such by the tooling - for example, none of these packages could be used as a [dependency override](./dependency-overrides.md) to stand in for the other one.

Copy link
Copy Markdown
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 today, AFAICT, if you publish, change, publish, then the (off-chain) tooling will allow you to use the second package as an override of the first, but things will go wrong when you try and publish on-chain. Maybe it's this second part that you're refering to, in which case we should clarify that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is indeed the second part that I meant and tried to skim over the details a bit but I agree that it might have been misleading a bit. I tried rephrasing to reflect what's going on a bit better.

Comment thread doc/src/build/package-versioning.md Outdated
Sui smart contracts are organized into [upgradeable](./package-upgrades.md) packages and, as a result, multiple versions of any given package can exist on-chain. Before a package can be used on the chain, it is [published](move/debug-publish.md#publishing-a-package) at its first (a.k.a, original) version. A new version of a package is created whenever the package is [upgraded](./package-upgrades.md), and an upgrade always happens with respect to the version of the package immediately preceding the upgraded one in the versions history. In other words, an upgrade to the Nth version of a package is always based on the N-1th version of the package (this is enforced by the system) and one can never create the Nth version of the package based on a version that's smaller than N-1th version. For example a package can be upgraded from version 1 to 2 but once this happens only an upgrade from version 2 to 3 is possible and an upgrade from version 1 to 3 is forbidden.

Note that there is also a notion of versioning present in the [manifest](./move/manifest.md) files. You can observe it both in the [package section](./move/manifest.md#package-section) and in the [dependencies section](./move/manifest.md#dependencies-section). For example:
```move

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: This should be toml syntax, not move

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

:-) I borrowed this from the first example in package-upgrades.md not realizing that there is toml file support as well. Fixed in all places that I touched and also in package-upgrades.md.

Comment thread doc/src/build/package-versioning.md Outdated

## Version management

In order to control the evolution of a published package, a developer can implement a [custom upgrade policy](./custom-upgrade-policy.md) but Sui also comes with a set of built-in upgrade policies. For an overview of how an upgrade policy should be applied, see [here](./custom-upgrade-policy.md#upgrade-overview).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This statement makes it seem like "immutable", "dependency-only", "additive", "compatible" exist in the same class of things that people write custom upgrade policies for, which is not true, they are orthogonal. This is my fault for overloading the world "policy", but maybe we can walk that back in the docs.

I think the best way to do that is to skip over the topic of custom upgrade policies here and talk just about the policies for what can be upgraded, maybe under the heading of "Compatibility"? If we take this section and put it on the same page as the custom upgrade policy docs under that heading, that may suffice?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried to finesse it as best I could.

Comment thread doc/src/build/package-versioning.md Outdated
- immutable - this policy is a bit of a misnomer as having it applied means that the package cannot be upgraded at all
- dependency-only - the only thing that can be modified in an upgraded version of this package are its dependencies
- additive - new functionality can be added to the package (e.g., new public functions or structs) but none of the existing functionality can be changed (e.g., the code in existing public functions cannot change)
- compatible - the most relaxed policy where in addition to everything allowed by more restrictive policies, an upgraded version of the package can change the code of existing public functions, signatures of existing functions and definitions of existing types cannot change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some more things that can happen in a Compatible upgrade:

  • Ability constraints on generic type parameters in function signatures can be removed,
  • private, public(friend), and entry functions can be changed, removed, or made public.

This leads me to a question for @tzakian -- do we support making a private function public in additive mode? It seems like we should, technically speaking.

Finally, a table seems like the most suitable way to visualize this information, perhaps followed by a description of what each kind of change is. @ronny-mysten / @randall-Mysten, how feasible would that be?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated but left in the list form for now.

@awelc

awelc commented Apr 30, 2023

Copy link
Copy Markdown
Contributor Author

If this seems reasonable, rather than drafting the docs in-place, let's make a document in docs/in-progress, land that, and then stage it in the places we decide.

It does! I was not sure where to put it all so, in the meantime, followed my own advice from #11253

Moved to the in-progress directory pending further refinement and slicing/dicing.

@amnn amnn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good modulo comments, thanks @awelc !

Comment thread doc/src/build/package-upgrades.md Outdated
Comment thread doc/in-progress/package-versioning.md Outdated
Comment thread doc/in-progress/package-versioning.md Outdated
Comment thread doc/in-progress/package-versioning.md Outdated

Sui comes with a set of built-in package upgrade policies, listed here in the order of strictness:

- immutable - this policy is a bit of a misnomer as having it applied means that the package cannot be upgraded at all

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think it is a misnomer to call it a compatibility policy because it's the absence of an UpgradeCap rather than immutable being a misnomer because it means that the package cannot be upgraded at all -- in that sense the name is consistent.

Comment thread doc/in-progress/package-versioning.md Outdated
Comment thread doc/in-progress/package-versioning.md Outdated
Co-authored-by: Ashok Menon <ashok@mystenlabs.com>
@ronny-mysten

Copy link
Copy Markdown
Contributor

I'm currently updating this PR. Please don't merge yet.

@awelc

awelc commented May 2, 2023

Copy link
Copy Markdown
Contributor Author

I'm currently updating this PR. Please don't merge yet.

Understood! Note that it's supposed to be sliced and diced further, though, when being moved from in-progress directory.

That being said, please feel free to take over the branch or make suggestions in the PR for me to accommodate!

@ronny-mysten

Copy link
Copy Markdown
Contributor

I sliced and diced as @amnn suggested, but thought the package versions bit fit better in the second option he suggested. I could be convinced that's not the case, though.

@ronny-mysten
ronny-mysten merged commit baeee8c into main May 2, 2023
@ronny-mysten
ronny-mysten deleted the aw/versioning-docs branch May 2, 2023 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants