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

Package versioning is a huge mess right now #851

Closed
larry0x opened this issue Dec 7, 2022 · 10 comments
Closed

Package versioning is a huge mess right now #851

larry0x opened this issue Dec 7, 2022 · 10 comments

Comments

@larry0x
Copy link
Contributor

larry0x commented Dec 7, 2022

See the following table:

Package Version Dependencies
cw2 1.0.0 cosmwasm-std 1.0.0, cw-storage-plus 0.16
cw20 1.0.0 cosmwasm-std 1.1.0, cw-utils 0.16
cw721 0.16.0 cosmwasm-std 1.1.5, cw2 0.16, cw-storage-plus 0.16, cw-utils 0.16
cw-storage-plus 1.0.1 cosmwasm-std 1.1.6
cw-utils 1.0.0 cosmwasm-std 1.0.0, cw2 0.16
cw-multi-test 0.16.1 cosmwasm-std 1.0, cw-storage-plus 0.16, cw-utils 0.16

As you see the versioning is not intuitive at all. E.g. cw20 is on 1.0.0 and depends on cosmwasm-std 1.1.0, all good. cw2 is also at 1.0.0 so it should depend on cosmwasm-std 1.1.0 as well, right? But nope, it's on 1.0.0.

This is especially tricky to deal with when some package versions include bugs that need to be avoided: cw-storage-plus 1.0.0 (already fixed in 1.0.1) and cw-utils 1.0.0 (not yet fixed: CosmWasm/cw-minus#4).

In my opinion, this is caused by scattering these packages into multiple repositories. It'd be better if they are all merged into one mono-repo and their dependencies managed centrally by the workspace Cargo.toml.

@apollo-sturdy
Copy link
Contributor

cw2 is also at 1.0.0 so it should depend on cosmwasm-std 1.1.0 as well, right? But nope, it's on 1.0.0.

Why? cw2 only needs to depend on cosmwasm-std 1.1.0 if it specifically needs something from 1.1.0.
Cargo uses caret versioning by default. From The Cargo Book: Specifying Dependencies:

The string "0.1.12" is a version requirement. Although it looks like a specific version of the time crate, it actually specifies a range of versions and allows SemVer compatible updates. An update is allowed if the new version number does not modify the left-most non-zero digit in the major, minor, patch grouping

To see this in action, you can simply create a new project with dependencies e.g:

cosmwasm-std = "1.1.3"
cw2 = "1.0.0"

and then run cargo tree, and you will see in the output:

├── cw2 v1.0.0
│   ├── cosmwasm-schema v1.1.9 (*)
│   ├── cosmwasm-std v1.1.9 (*)

showing that cw2 is indeed using the latest compatible version of cosmwasm-std, namely 1.1.9.

@larry0x
Copy link
Contributor Author

larry0x commented Dec 7, 2022

@apollo-sturdy What about cw-storage-plus? Currently my repo's Cargo.lock contains two copies of cw-storage-plus:

  • 0.16.0, which cw2 uses
  • 1.0.1, which everything else uses

My guess is this may also increase the size of wasm binaries, as they need to include two copies of cw_storage_plus::Item, one for cw2 and one for everything else.

What I wish is there only being one version of each package in Cargo.lock. As long as this is the case then I'm happy with it.

My main argument here is that mono-repo is the better approach. With the current approach of each package being their separate repo, whenever you have a major version bump of something (like cw-storage-plus 0.16 to 1.0 here) you need to go into every repo and bump them individually. With the mono-repo approach you just update one line in Cargo.toml and it's done.

@faddat
Copy link

faddat commented Dec 7, 2022

  1. Specifically, in this case, I think that we'll be better served by having a single repo with the same versioning.

  2. generally speaking, I think that Cosmos has an issue with this overall, and should try to collapse into just two repositories:

  • tendermint (which would ideally include tm-db) so that importing is a single direct import and versioning is easier

  • cosmos-sdk, which would add the following repositories to itself:

    • ledger-cosmos-go
    • cosmos-db
    • iavl
    • wasmd
    • interchain-accounts-demo
    • interchain-security
    • ics23
    • cosmos-proto
    • ibc-go

@apollo-sturdy
Copy link
Contributor

@larry0x This is expected since 0.16.0 and 1.0.1 are not compatible. Since the two versions are not compatible of course your project would need to have both versions included in Cargo.lock.

So the real question here is how easily can cw2 be upgraded to use version >=1.0.0 of cw-storage-plus and why hasn't anyone done it yet? Perhaps it is because the maintainers forgot, or perhaps there is a good reason, e.g. that upgrading would require making big changes to the package. I can see the value in your suggestion of having each of the packages in cw-plus have their dependencies specified by the workspace Cargo.toml file, but if it is the case that the dependency has not been updated on purpose (because of major breaking changes), then you would have to either make an exception to that package, or wait with upgrading all other packages until you have time to upgrade this one.

@apollo-sturdy
Copy link
Contributor

After digging a bit deeper it seems all packages in cw-plus use version 0.16.0 of cw-storage-plus as well as cw-utils.

@ethanfrey @uint Can we get some input here? Is there a good reason here or have you simply not gotten around to updating these dependencies? Would you like help in updating them and if so, would you prefer using the suggestion from @larry0x to keep versions in the workspace Cargo.toml, or would you prefer a PR that simply updates all of the dependencies and bumps each patch version?

@uint
Copy link
Contributor

uint commented Dec 14, 2022

Can we get some input here? Is there a good reason here or have you simply not gotten around to updating these dependencies?

I think that was just oversight on my part. My bad!

Would you like help in updating them

I no longer work for Confio and I imagine they haven't yet decided how to own/maintain this repo. I'm sure a PR will be appreciated.

As to the approach, I'd suggest updating the deps (cw-storage-plus, cw-multi-test, cw-utils) to just 1, or possibly even something like >=0.16.0, <2.0.0. I don't like the idea of bringing back the monorepo and setting everything to the same version since it's unnecessarily restrictive. IMO version requirements should be as loose as they can be exactly to prevent duplicate dependencies in people's projects.

To verify minimal versions work, you can always run cargo build -Zminimal-versions. You could even add a CI build like the one in storage-plus.

@ethanfrey
Copy link
Member

Yes, this was supposed to be Tom's last project. My bad not to review the code better.

I'm super busy, and holidays are coming. I will try to get someone to work on it.
Not a huge change, but needs to be done carefully across a few repos.

If someone does make a PR doing the obvious bumps in the Cargo.toml files (set version to 1 or 1.0.1) then happy to merge them all and release. But I am afraid this will be a multi-step release process.

@ueco-jb
Copy link
Contributor

ueco-jb commented Dec 15, 2022

These packages were released with updated deps:
cw-storage-plus v1.0.1 https://github.com/CosmWasm/cw-storage-plus/releases/tag/v1.0.1
cw-multi-test v0.16.2 https://github.com/CosmWasm/cw-multi-test/releases/tag/v0.16.2
cw-utils v1.0.1 https://github.com/CosmWasm/cw-utils/releases/tag/v1.0.1

I consider this issue solved.

@ueco-jb ueco-jb closed this as completed Dec 15, 2022
@webmaster128
Copy link
Member

cw-plus is also on 1.0.0. Jakub just pushed a missing git tag. It was on crates.io already before.

@apollo-sturdy
Copy link
Contributor

Thanks @ethanfrey. I've created a PR here: #853

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

No branches or pull requests

7 participants