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

Protobuf version check does not play well with semver #1363

Closed
cheme opened this issue Jan 1, 2020 · 5 comments · Fixed by #1368
Closed

Protobuf version check does not play well with semver #1363

cheme opened this issue Jan 1, 2020 · 5 comments · Fixed by #1368
Labels

Comments

@cheme
Copy link
Contributor

cheme commented Jan 1, 2020

This line does not work well with semver cargo expectation:

const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_8_1;
result in https://gitlab.parity.io/parity/substrate/-/jobs/341902
It would either require to have a fix versioning in cargo.toml or check over every version that match semver requirement. Not sure what is better.

@bltavares
Copy link

I've started a test project and found this issue. I've partially solved it by pinning the version of protobuf crate on my Cargo.toml

[dependencies]
libp2p = "0.13.1"
protobuf = "=2.8.1"

@tomaka
Copy link
Member

tomaka commented Jan 2, 2020

#1353 fixed it for the stable-futures branch.
Should we release a version 0.13.2 with protobuf pinned down (cc #1365), or is it fine to wait for 0.14?

@cheme
Copy link
Contributor Author

cheme commented Jan 2, 2020

It seems to break substrate ci so I would +1 in favor of 0.13.2 with protobuf pinned down.

@tomaka
Copy link
Member

tomaka commented Jan 2, 2020

Theoretically Substrate should only break if someone updates the Cargo.lock to use a new protobuf version.

@cheme
Copy link
Contributor Author

cheme commented Jan 2, 2020

Yes, that is the 'try_build' check that breaks: https://gitlab.parity.io/parity/substrate/-/jobs/341902 (it seems to use its own lock internally, I am not sure of the purpose of this test but it may be to detect such issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants