onelson / estuary

33 stars 8 forks source link

Cargo sends `version_req`, not `req` for dependencies. #12

Closed onelson closed 3 years ago

onelson commented 3 years ago

Early tests for package uploads were done with cargo projects generated by cargo new which adds an empty [dependencies] field to Cargo.toml.

This field isn't required by the manifest format, and when it's missing it will cause the crate publish to fail since the deserialization target currently considers the field as required.

We should either:

Of the fields currently defined for PartialPackageVersion, name and vers are absolutely required, but we should give the "optional treatment" to both deps and features.

So, it looks as though the payload cargo sends doesn't exactly match the payload the package index maintains. Add a publish test with a fleshed out Cargo.toml to smoke out any field mismatches, and fix the version_req vs req problem.

onelson commented 3 years ago

I think I misread the initial report of this. Sounds like the thing that failed to deserialize was a req field on a Dependency.

https://github.com/onelson/estuary/blob/0b2128313d6436c7ab2ca4fc050798960fdebbd6/src/package_index.rs#L79

Technically if the req field isn't there, the crate publish should not be accepted, so I think we need to do a little more digging to see what led to this.

The fix might be to provide better feedback in the error message.

onelson commented 3 years ago

Looks like req/version_req is the only field different between what cargo sends and what the index format expects.

This can be fixed with https://serde.rs/field-attrs.html#alias