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

tools/proto-compiler output does not compile on current master #1014

Closed
2 tasks
hdevalence opened this issue Oct 28, 2021 · 3 comments · Fixed by #1015
Closed
2 tasks

tools/proto-compiler output does not compile on current master #1014

hdevalence opened this issue Oct 28, 2021 · 3 comments · Fixed by #1015
Assignees
Labels
bug Something isn't working

Comments

@hdevalence
Copy link
Collaborator

Steps to reproduce

  1. Check out a clean copy of tendermint-rs
  2. Run cargo test; it succeeds
  3. cd tools/proto-compiler && cargo run && cd ../..; succeeds
  4. cargo test fails, as the output no longer compiles:
/t/tendermint-rs (master|✚4) $ cargo test
   Compiling tendermint-proto v0.23.0 (/private/tmp/tendermint-rs/proto)
error[E0433]: failed to resolve: use of undeclared type `Vec`
   --> proto/src/prost/tendermint.abci.rs:239:35
    |
239 |     #[serde(skip_serializing_if = "Vec::is_empty", with = "serde_bytes")]
    |                                   ^^^^^^^^^^^^^^^ not found in this scope
    |
help: consider importing one of these items
    |
9   | use alloc::vec::Vec;
    |
9   | use crate::Vec;
    |
9   | use serde::__private::Vec;
    |

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

My best guess is that this is a no_std-related issue but I don't actually know.

What's the definition of "done" for this issue?

  • fix proto-compiler to produce working code;
  • add some kind of check to CI that actually exercises the codepaths?
@hdevalence hdevalence added the bug Something isn't working label Oct 28, 2021
@hdevalence
Copy link
Collaborator Author

I think this can be fixed as follows:

diff --git a/proto/src/prost/tendermint.abci.rs b/proto/src/prost/tendermint.abci.rs
index c34e6d1b..4e514228 100644
--- a/proto/src/prost/tendermint.abci.rs
+++ b/proto/src/prost/tendermint.abci.rs
@@ -236,7 +236,7 @@ pub struct ResponseInfo {
     #[serde(with = "crate::serializers::from_str")]
     pub last_block_height: i64,
     #[prost(bytes="vec", tag="5")]
-    #[serde(skip_serializing_if = "Vec::is_empty", with = "serde_bytes")]
+    #[serde(skip_serializing_if = "::prost::alloc::vec::Vec::is_empty", with = "serde_bytes")]
     pub last_block_app_hash: ::prost::alloc::vec::Vec<u8>,
 }
 /// nondeterministic

@thanethomson
Copy link
Contributor

I can successfully reproduce this. Seem as though it may have something to do with this line that gets removed when I re-run proto-compiler:

use crate::prelude::*;

I'll have to look into why it's removing that line though.

@thanethomson
Copy link
Contributor

Oh it's probably because that line was added manually 😄 Damn. I'll look into fixing this.

@thanethomson thanethomson self-assigned this Oct 28, 2021
thanethomson added a commit that referenced this issue Oct 28, 2021
Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson added a commit that referenced this issue Oct 29, 2021
* Make Vec method reference absolute to fix #1014

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Regenerate protos with proto-compiler

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add experimental CI job to test that generated protos compile

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Try alternative approach to `uses` with `working-directory`

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants