-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
[docs] Added documentation for dependency overrides #11253
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @awelc ! I've added a Jira for myself to add this topic to the navigation, too.
Co-authored-by: ronny-mysten <118224482+ronny-mysten@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @awelc ! Content looks good, modulo some small comments.
crypto = { git = "https://github.com/crypto.org/crypto.git" , rev = "v2.0.0"} | ||
``` | ||
|
||
This situation represents the [_diamond dependency conflict_](https://jlbp.dev/what-is-a-diamond-dependency-conflict) where two indirect dependencies in turn depend on different versions of the same package. If all packages involved are developed independently, then the developer of the `user` package has a problem -- the Move compiler cannot discern which version of the `crypto` package to use and, as a result, building of the `user` package fails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I picked this link originally when I was writing our internal design document, but this may not be suitable for our public-facing docs. Thoughts @ronny-mysten / @randall-Mysten ? I think it's fine to drop the link entirely, to be honest, because we're explaining what it is below.
crypto = { git = "https://github.com/crypto.org/crypto.git" , rev = "v2.0.0" , override = true } | ||
``` | ||
|
||
Note that overriding transitive dependencies in such a manner might not always be successful. In particular, the `vault` package in our example might truly depend on the functionality of the `crypto` package that is only available in version `1.0.0` of this package (e.g, was removed in later package revisions), in which case the build would also be unsuccessful. By offering the ability to enforce dependency overrides we merely put another tool into the developer's toolbox in the hope that it would allow resolution of transitive dependency conflicts in the majority of cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't have to be in this PR, but we should also explain the on-chain constraint around versioning and compatibility, and relatedly, the lack of a relationship between on-chain and local versions. I.e. in practice, if folks are using semantic versioning, then v2.0.0
is not going to be compatible with v1.0.0
and so it won't be a package upgrade in the technical sense, and you won't be able to perform the override, and they may choose an override that makes everything compile, but again find themelves in hot water because it's not a strictly greater version on-chain.
This is another place where Automatic Address Management (or an extension of it) will help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree! And I tried to think what the best place for the additional information would be and I am currently leaning towards a separate "page", perhaps titled "Package Versioning" (or "Code Versioning" to avoid title overlap with "Package Upgrades"). I will start writing it soon-ish (unless someone preempts me, but then please let me know :-) ), but will likely as @tzakian to fill in the compatibility part. If we decide to put it with "Dependency Overrides" after all, we can indeed always augment this document in a separate PR.
Co-authored-by: Ashok Menon <ashok@mystenlabs.com>
## 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) - [x] 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 --------- Co-authored-by: Ashok Menon <ashok@mystenlabs.com> Co-authored-by: Ronny Roland <ronmilton@mystenlabs.com>
Description
A description of how recently landed (#11181) dependency overrides work.
Please let me know if you need this changes, extended, etc.
I am also not sure if this should be a separate file (as it is intended for now) or if it should be folded into an existing document (e.g., https://github.com/MystenLabs/sui/blob/main/doc/src/build/package-upgrades.md), and if the former is the case, if there is any additional work required to have it properly displayed on the developer portal.
Test Plan
It's just the docs
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)
Release notes