Unhelpful error messages with env-vars that don't set all fields of a config struct

[Nemo157]

Problem

I have been using the gitoxide integration for a while, via setting env-vars so it doesn't impact running the stable compiler:

CARGO_UNSTABLE_GITOXIDE_CHECKOUT=true
CARGO_UNSTABLE_GITOXIDE_FETCH=true
CARGO_UNSTABLE_GITOXIDE_INTERNAL_USE_GIT2=false
CARGO_UNSTABLE_GITOXIDE_SHALLOW_DEPS=true
CARGO_UNSTABLE_GITOXIDE_SHALLOW_INDEX=true

After updating to a version that includes #13592 any usage of cargo starts failing with

> cargo test -- --ignored zero_byte
error: missing field `list_files`

which makes no mention of where this field is missing

Steps

> CARGO_UNSTABLE_GITOXIDE_FETCH=true cargo build
error: missing field `list_files`

Possible Solution(s)

No response

Notes

#13687 will likely fix this specific issue by making the fields of this struct default, but the general issue of the error message not having enough context will remain.

Version

cargo 1.79.0-nightly (a59aba136 2024-03-28)
release: 1.79.0-nightly
commit-hash: a59aba136aab5510c16b0750a36cbd9916f91796
commit-date: 2024-03-28
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: NixOS 23.11.0 [64-bit]

[weihanglo]

Thanks for the report. I guess this is not a general issue but -Zgit/-Zgitoxide specific one?

[Nemo157]

Those do appear to be the only structs in the unstable config. I'm not sure where the general config-env-var parsing happens so I'm not certain if they're the only ones that are affected.

[Nemo157]

Found a way to trigger it with a stable config too:

> CARGO_TERM_PROGRESS_WIDTH=2 cargo check
error: missing field `when`

[linyihai]

FYI, I have searched the code base, and have not found the place that triggers missing field, guess that this is related to the parsing process of the third-party library serde?

[Nemo157]

Yeah, the env-vars are read as part of a deserializer that merges the config files, env-vars and cli flags into one, then the error is produced by the Deserialize impl from ProgressConfig, you also get the same unhelpful error using the cli flag or config file:

> cargo check --config 'term.progress.width=2'
error: missing field `when`
> mkdir -p .cargo && echo 'term.progress.width=2' > .cargo/config.toml
> cargo check
error: missing field `when`

What would be most useful is more context on the error: what's the full config key that's being parsed and failed, and where are the other fields coming from (they can even come from multiple locations:

> CARGO_TERM_PROGRESS_WIDTH=2 cargo check --config 'term.progress.when="always"'
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s

[epage]

Whats interesting about the config case is that its term.progress.when. Serde only reports when. We only know that the caller looked up term. I'm unsure if we can connect the dots to report term.progress.when without specializing the error reporting to each individual struct which is just playing whack-a-mole (like in #13687).

[epage]

See #13694 to see what that looks like

[Nemo157]

I'm unsure if we can connect the dots to report term.progress.when

It looks like this is being tracked in the Deserializer as it enters into parsing each field of the structs:

cargo/src/cargo/util/context/de.rs

Line 315 in 18181c6

self.de.key.push(field);

it feels like it should be possible to attach this context somewhere at this level when the error first passes back to the deserializer.

[epage]

Good point. I do something similar in toml_edit, see https://github.com/toml-rs/toml/blob/48f968b449d64a3b7bcefab37c3f08438ba857ec/crates/toml_edit/src/de/table.rs#L171

However, I hit my time box I set aside for this as I need to be wrapping up some existing tasks and putting most of my focus to my 2024 Edition work.