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

aptos-core is stuck on serde < 1.0.53 due to a change in 1.0.53 that breaks testsuite/generate-format #10424

Closed
banool opened this issue Oct 6, 2023 · 2 comments · Fixed by aptos-labs/serde-reflection#3 or #11137
Assignees
Labels
bug Something isn't working move move-deps stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@banool
Copy link
Contributor

banool commented Oct 6, 2023

Repro

git clone git@github.com:aptos-labs/aptos-core.git
# Change serde version in Cargo.toml to 1.0.153
cd testsuite/generate-format
cargo run -- --corpus aptos

You will see an error like this:

thread 'main' panicked at 'Incomplete tracing detected inside container: StructTag:
A final registry was requested but some formats are still unknown within the container StructTag. This can
happen if `tracer.trace_value` was called on a value `foo` which does not reveal some of the underlying
formats. E.g. if a field `x` of struct `Foo` has type `Option<String>` and `foo` is value of type
`Foo` such that `foo.x == None`, then tracing the value `foo` may result in a format `Option<Unknown>`
for the field `x`. The same applies to empty vectors and empty maps.

To fix this, avoid `trace_value` and prefer `trace_type` when possible, or make sure to trace at
least one value `foo` such that `foo.x` is not empty.
', testsuite/generate-format/src/lib.rs:50:17

Summary

This issue arises from this change: serde-rs/serde#2387.

I tried using the latest version of serde + serde_yaml and it doesn't fix the issue. I also tried to fork serde and fix it but then we get a bunch of other errors, see https://github.com/banool/serde/tree/banool/undo-2387 and https://github.com/aptos-labs/aptos-core/tree/banool/upgrade-serde.

I'm not sure right now how we could handle this issue in our codebase. I believe changing the format (I'm not 100% sure what format that is, btw) is out of the question so we'd have to do something on our side to undo whatever change serde-rs/serde#2387 introduces.

See more discussion here: https://aptos-org.slack.com/archives/C03N83P7QUC/p1691767293160939. I suspect this is relevant too: zefchain/serde-reflection#35 (meaning switching to this version of serde-reflection might not fix our issues).

Impact

This stops us from using crates that depend on a particular minimum version of serde higher than this version. To date this has happened twice:

As time goes on this is going to happen more and more frequently as our version of serde gets increasingly out of date.

@banool banool added the bug Something isn't working label Oct 6, 2023
@banool banool changed the title [Bug] aptos-core is stuck on serde < 1.0.53 due to breaking change in 1.0.53 [Bug] aptos-core is stuck on serde < 1.0.53 due to a change in 1.0.53 that breaks testsuite/generate-format Oct 6, 2023
@banool banool changed the title [Bug] aptos-core is stuck on serde < 1.0.53 due to a change in 1.0.53 that breaks testsuite/generate-format aptos-core is stuck on serde < 1.0.53 due to a change in 1.0.53 that breaks testsuite/generate-format Oct 6, 2023
@banool banool self-assigned this Oct 6, 2023
@banool banool removed their assignment Oct 6, 2023
@banool
Copy link
Contributor Author

banool commented Oct 11, 2023

I opened an issue in serde: serde-rs/serde#2627.

@banool
Copy link
Contributor Author

banool commented Oct 11, 2023

I tried updating to the latest zefchain and that didn't fix it, presumably because of this issue: zefchain/serde-reflection#35.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working move move-deps stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: Done
3 participants