Native options discrepancy warning

[bolmstedt]

Describe the bug When extending a list, items are added twice in the native value, but not in the legacy value.

This can be reproduced by creating the following files:

# pants.toml
[GLOBAL]
pants_version = "2.23.0.dev1"
backend_packages = ["pants.backend.docker"]

# .pants.rc
[GLOBAL]
backend_packages = "+['pants.backend.docker.lint.hadolint']"

And then running pants version to produce:

15:31:12.47 [WARN] Value mismatch for the option `backend_packages` in scope []:
Legacy value: ['pants.backend.docker', 'pants.backend.docker.lint.hadolint'] of type <class 'list'>, from source CONFIG
Native value: ['pants.backend.docker', 'pants.backend.docker.lint.hadolint', 'pants.backend.docker.lint.hadolint'] of type <class 'list'>, from source CONFIG
...

Pants version 2.23.0.dev1

OS MacOS

[lilatomic]

confirming reproduction. Digging into it, we process all items in .pants.rc twice. Need to do more digging to find out why. (with replace actions (everything except for add or remove) this doesn't have an effect.)

[lilatomic]

In OptionParser.new we skip config discovery if we've been passed some discovered configs https://github.com/pantsbuild/pants/blob/dc0aa0a6444618435c60f8807ab89997885f5910/src/rust/engine/options/src/lib.rs#L326-L328

But we don't skip reading from the .pants.rc file https://github.com/pantsbuild/pants/blob/dc0aa0a6444618435c60f8807ab89997885f5910/src/rust/engine/options/src/lib.rs#L382

When starting up initially, we don't pass any discovered configs. But on the Python side, when we're starting up the rust OptionParser to compare, we do pass in the already discovered configs https://github.com/pantsbuild/pants/blob/a3eaf7007fcb2d878edd9f08d922799289567c25/src/python/pants/option/options.py#L166

I tried just jamming that to None to get the rust parser to discover configs. It didn't explode, but I'm not sure if that's the right decision lol

[benjyw]

Hmm yes, the Rust parser does its own config discovery, so we should pass it no configs I think!

[lilatomic]

Ah, seems passing in the configs is necessary for integration tests. They do some stuff to set the OptionsBootstrapper. That makes sense. I think then that the solution is to not load the rc if we've been passed configs, and then only pass configs for the OptionsBootstrapper (so tests work). I imagine that we can simplify that once we're using the rust parser only.

[benjyw]

The simplification promised above happens in https://github.com/pantsbuild/pants/pull/21784