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

Old cargo version with no stabilized support for edition 2021 fail to build cosmwasm-std #1204

Closed
etienne-napoleone opened this issue Feb 1, 2022 · 11 comments · Fixed by #1205

Comments

@etienne-napoleone
Copy link

uint-v0.9.2 changed to edition 2021 leading to error while trying to run rust-optimizer on a crate with deps to uint ^0.9.0. This includes cosmwasm-std.

They should not release potentially breaking changes under patch version, but it's already done.

docker run --rm -v "$(pwd)":/code   --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target   --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry cosmwasm/rust-optimizer:0.12.4
Info: RUSTC_WRAPPER=sccache
Info: sccache stats before build
Compile requests                      0
Compile requests executed             0
Cache hits                            0
Cache misses                          0
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Compilation failures                  0
Cache errors                          0
Non-cacheable compilations            0
Non-cacheable calls                   0
Non-compilation calls                 0
Unsupported compiler calls            0
Average cache write               0.000 s
Average cache read miss           0.000 s
Average cache read hit            0.000 s
Failed distributed compilations       0
Cache location                  Local disk: "/root/.cache/sccache"
Cache size                            0 bytes
Max cache size                       10 GiB
Building contract in /code ...
error: failed to download `uint v0.9.2`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/uint-0.9.2/Cargo.toml`

Caused by:
  feature `edition2021` is required

  The package requires the Cargo feature called `edition2021`, but that feature is not stabilized in this version of Cargo (1.55.0 (32da73ab1 2021-08-23)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2021 for more information about the status of this feature.

Quickfix

[dependencies]
uint = "=0.9.1"  # needed to fix uint to 2018 rust edition
cosmwasm-std = "0.16.2"

Potential fixes

  1. update cargo version to a version that have edition 2021 support
  2. change all comswams-std and other packages deps to uint = "=0.9.1".
@hashedone
Copy link
Contributor

We are already updating rust on our CIs to 1.58.1 because of that. Fixing uint version is very temporary solution and not a goto in any matter - with time there would be more and more crates failing for this reason, and it would basically disable any updates from them. Just give ma couple minutes - I will create PR on this repo.

EDIT

I just checked images - it is already updated - does it fail on the newest image from git repo? I think it is not yet released on dockerhub.

@etienne-napoleone
Copy link
Author

yep, updating rust is definitely the cleaner way :)

@etienne-napoleone
Copy link
Author

I just checked images - it is already updated - does it fail on the newest image from git repo? I think it is not yet released on dockerhub.

let me try

@webmaster128
Copy link
Member

Changing the MSRV is a patch release is not very nice of usint. cosmwasm_std has MSRV of 1.53.0 and we cannot upgrade to anything higher than 1.55.0 in wasmvm right now. I will pin uint to 0.9.1 for now.

@webmaster128
Copy link
Member

Moving this ticket to cosmwasm repo.

In CosmWasm/optimizer#74 I created a new issue to create a rust-optimizer release with the new compiler version.

@webmaster128 webmaster128 transferred this issue from CosmWasm/optimizer Feb 1, 2022
@etienne-napoleone
Copy link
Author

yes, poor versioning on their part :(

@hashedone
Copy link
Contributor

hashedone commented Feb 1, 2022

Technically due to Rust standard versioning - as long as there is not a regression in newest stable, there is no breaking change and it is ok not to bump major version. The idea of minimum supported cargo version is kind of unofficial agreement and assuming that every crate creator would consider anyone using his crate with this kind of requirement is kind of strange to me. I am aware, that it kind of became a thing in 1.56, but... 1.55 < 1.56. And even in 1.56 it does not affect dependency resolver in any way, it just fails early if it uses anything from older rust compiler, but still tries to use newest stuff if possible. Maybe if there is so crucial to have this kind of MSCV, we should always use the =version versioning, but it makes maintaining versions tedious.

In particular - I just checked uint crate - there is no mention about any kind of MSCV, also there is no such thing in the official parity repo. Assuming it would be anything else than newest stable is kind of a wish. And in 99% cases it is not an issue, as typically Rust doesn't bring too many breaking changes with versions, but we are on the edition break, and we should probably try to align to it ASAP - especially that 1.55 compiler is already kind of old. I understand there is an issue linked by @webmaster128 so we cannot upgrade it right now, but just leaving it as it is is kind of risky. We stuck in compiler which is 4 months old right now, and it is not terrible, but if we will not figure out how to fix the linkage problem, we are putting ourself in danger of being forced to use the same compiler in an year from now. And there would be some people who would like to use GATs, and const generics and all those stuff in their code and we want to be able to build our soft with modern compiler.

My point is - supporting old compiler is good. To some extend to not force ppl not to use goodies being a thing for an year, but supporting compiler being 3-6 moth olds seems reasonable. On the other hand - not supporting modern compiler being already 3 months old feels really off to me.

@webmaster128
Copy link
Member

webmaster128 commented Feb 1, 2022

On the other hand - not supporting modern compiler being already 3 months old feels really off to me.

Nobody is saying that. All code can be built with the latest Rust stable. But this crate is forcing us to reduce the range of supported compilers to an extremely small one.

@hashedone
Copy link
Contributor

hashedone commented Feb 1, 2022

It is not - unless you need to use some specific feature from the newest release. The only thing it is forcing you to do is, that if you want to support old compiler, you have to reduce range of version used. We have 3 options:

  1. Abandon MSCV idea (which is not really an option)
  2. Use precise crate versioning for everything (=version) - but it is tedious and doesn't work well with dependency resolver (in case when you updated crate in one place you need to update it everywhere with dependencies to it/you depend on - very tedious)
  3. Use precise crate versioning for crates you know that doesn't work on your MSCV (in this case we have uint). It is a bit tedious, but only because you need to update all cargos when new crate abandons support for old cargo. But since this point you are not allowed to upgrade this very crate anywhere in the project so it is acceptable probably.

Just because we want to support old cargo in out project (which we have reasons to) it not a reason to expect others to do so - it actually serious tradeoff (there are many nice small things since 1.55), and come on, 1.56 is 3 months old. If parity keeps 3 months old for their crates it is actually nice from their side. It is our issue that we cannot upgrade on our crate, I don't see a point to blame parity for them - do we want to force them staying on previous edition unless we decide to fix our linkage? And we keep tickling ppl using C++18 in their project in the same time?

@webmaster128
Copy link
Member

In #1197 / #1207 the version range of the 0.16-x series is changed to 1.55.0-latest stable. I'll do the same change for 1.0 as well to at least drop support for 1.53 and 1.54 which nobody needs AFAIK.

@webmaster128
Copy link
Member

While I closed this ticket because I'd rather copy the uint implementation than accepting the requirement for 1.56 for no reason, the bigger dilemma still exists as pointed out by @hashedone.

Reading rust-lang/api-guidelines#252 was very helpful. It's a back and forth between You can't simply break other people's projects that compiled once with compiler X on the one side and well, stability guarantess? Whatever! Give it up because there is always this one dude in the dependency tree that requires a recent Rust version for some reason if any.

I think this is pretty annoying actually. There are and there will always be good reasons to use a Rust version that is a few months old. And if that reason is it takes our lazy dev team a few months to update and test the entire tech stack with a newer compiler because they have better things to do or are really just lazy then this is still a good reason. Also in our use case increasing the required compiler version means a lot of unnecessary support requests because contract devs are often not very familiar with Rust and get cryptic error messages if their compiler is outdated. This is fine if outdated means 2 years old. But here suddenly outdated means started a project, worked on it for a bit, got distracted, holiday break, got back to it, compile error. In Node.js land there is a culture that dropping compiler/interpreter/runtime support for a given code branch requires a major version bump. This is the mentality I was used to from C++ as well.

Seems like we have to accept the pain, deal with it somehow and provide a somewhat consident experience for our callers.

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 a pull request may close this issue.

3 participants