Skip to content

Fix lint-rust action #151

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

Merged
merged 6 commits into from
Aug 24, 2022
Merged

Fix lint-rust action #151

merged 6 commits into from
Aug 24, 2022

Conversation

matheus23
Copy link
Contributor

@matheus23 matheus23 commented Aug 22, 2022

Work on #150

This fixes some issues introduced by compiling with rust 1.63 instead of 1.62.

I can't quite figure out the no_std problems though. It seems like there's some dependency that re-enables the std feature on serde, but I don't know which and how to debug it. Here's the output I get from building for a no_std target:

$ cargo build --no-default-features --target thumbv6m-none-eabi --manifest-path core/Cargo.toml
   Compiling data-encoding v2.3.2
   Compiling serde v1.0.144
   Compiling anyhow v1.0.62
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv6m-none-eabi` target may not support the standard library
  = note: `std` is required by `serde` because it does not declare `#![no_std]`

@vmx
Copy link
Member

vmx commented Aug 22, 2022

but I don't know which and how to debug it.

cargo tree --edges features might help.

@matheus23
Copy link
Contributor Author

This seems to be caused by the serde_test devDependency: It seems like std can't be turned off for serde_test.
According to cargo tree --no-default-features -i serde --edges features, serde_bytes and serde_json also cause this, but I can turn off default features for them.

I'm really no expert when it comes to rust's build systems. Any idea on how to resolve this? It seems similar to this issue I found: serde-rs/serde#1535

@vmx
Copy link
Member

vmx commented Aug 23, 2022

It also fails on CI, but I can't reproduce it locally :-/. What I would've tried would've been upgrading the crate to Rust edition 2021 (https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html#transitioning-an-existing-project-to-a-new-edition).

@matheus23
Copy link
Contributor Author

I think when I ran this locally I accidentally changed some build.rs generated files which of course aren't checked in.
They were generated by prost-build, so I upgraded that an it should now compile successfully.

@vmx
Copy link
Member

vmx commented Aug 24, 2022

prost v0.11 doesn't bundle protoc anymore. This means that if we upgrade, the CI would need to be adapted. I'd be OK with not upgrading.

@matheus23
Copy link
Contributor Author

Alright. I

  • upgraded the edition and fixed resulting errors in the proc macros,
  • downgraded prost-build to 0.10 (and upgraded prost to 0.10 just to be sure)
  • added an #![allow(clippy...)], otherwise clippy would complain about generated code (:man_facepalming:)

The CI looks green on my fork, I'm pretty confident it'll be green here too @vmx

@matheus23
Copy link
Contributor Author

🟢 🎉
Would love for this to get released soon :)

@vmx vmx merged commit 81cb2db into ipld:master Aug 24, 2022
@vmx
Copy link
Member

vmx commented Aug 24, 2022

Thanks a lot @matheus23 for working through all this and getting CI back to green!

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.

2 participants