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

bump dependency versions #4

Merged
merged 5 commits into from
Dec 12, 2022
Merged

bump dependency versions #4

merged 5 commits into from
Dec 12, 2022

Conversation

larry0x
Copy link
Contributor

@larry0x larry0x commented Dec 7, 2022

Closes: #3

Including k256 as a dependency leads to a compilation error:

$ cargo build --target wasm32-unknown-unknown
Compiling subtle v2.4.1
   Compiling cfg-if v1.0.0
   Compiling zeroize v1.5.7
   Compiling const-oid v0.9.0
   Compiling itoa v1.0.4
   Compiling base64ct v1.5.3
   Compiling typenum v1.15.0
   Compiling serde v1.0.145
   Compiling serde_json v1.0.86
   Compiling ryu v1.0.11
   Compiling generic-array v0.14.6
   Compiling schemars v0.8.11
   Compiling crunchy v0.2.2
   Compiling dyn-clone v1.0.9
   Compiling thiserror v1.0.37
   Compiling hex v0.4.3
   Compiling getrandom v0.2.7
error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> /Users/larry/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/getrandom-0.2.7/src/lib.rs:235:9
    |
235 | /         compile_error!("the wasm32-unknown-unknown target is not supported by \
236 | |                         default, you may need to enable the \"js\" feature. \
237 | |                         For more information see: \
238 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |________________________________________________________________________^

   Compiling base16ct v0.1.1
error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /Users/larry/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/getrandom-0.2.7/src/lib.rs:262:5
    |
262 |     imp::getrandom_inner(dest)
    |     ^^^ use of undeclared crate or module `imp`

For more information about this error, try `rustc --explain E0433`.
error: could not compile `getrandom` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

This PR removes k256 and bump all dependencies to latest.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Could you add those Wasm build check to the CI? This will prevent such problems in the future: https://github.com/CosmWasm/cw-storage-plus/pull/23/files

@larry0x
Copy link
Contributor Author

larry0x commented Dec 7, 2022

@webmaster128 Removed anyhow and serde_json, updated CI

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

I wonder if there are breaking change in the prost cw2 or cw-storage-plus upgrades that affect this create. I'm not familiar with the codebase. The rest LGTM.

@webmaster128
Copy link
Member

Turned on "Build forked pull requests" now

@ueco-jb ueco-jb mentioned this pull request Dec 12, 2022
@webmaster128 webmaster128 requested a review from ueco-jb December 12, 2022 12:00
Copy link
Contributor

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

Just suggestions, since it's the same.

# Once we bump `cosmwasm-*` deps to a version after `1.1.5`, we can remove these.
serde_json = "1.0.40"
cw-storage-plus = "1.0.1"
prost = "0.11.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
prost = "0.11.0"
prost = "0.11"

Copy link
Member

@webmaster128 webmaster128 Dec 12, 2022

Choose a reason for hiding this comment

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

The problem with those changes it it makes the patch release invisible. This is often the cause for setting the min version of a dependency too low. If there is .0 you see that you either want .0 or that you want something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it doesn't make them invisible - it literally means the same. I would go even further, some users who don't understand how Cargo dep versioning works might be confused.
I'm good either way. 👍

cw2 = "0.16"
cosmwasm-schema = "1.1.9"
cosmwasm-std = "1.1.9"
cw2 = "1.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cw2 = "1.0.0"
cw2 = "1.0"

@webmaster128 webmaster128 merged commit 83298e8 into CosmWasm:main Dec 12, 2022
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.

Bump cosmwasm-std to ^1.1.8
3 participants