jj-vcs
FR: check `config-schema` against the actual settings "get" requests
Is your feature request related to a problem? Please describe. I wrote this PR and I bet there's a bunch of other settings undocumented in the schema: https://github.com/jj-vcs/jj/pull/6796
We should have a more principled way of validating this.
Describe the solution you'd like
There's some ideas I have but they all generally boil down to figuring out how to intercept the settings "get" calls in the test suite and then validating that the schema knows about them.
This seems maybe a little bit hard because the ideal situation with this is that it is possible to run the validator against the entire test suite and ideally not add pointless dependencies to the jj executable if possible.
My possible ideas are:
- Log all accesses?
- Add a way to check accesses with a static function that's only set while running tests?
The challenge with these is that you want to only affect tests of jj itself and not anything depending on jj_lib, and it's hard to write global initialization code for tests in Rust because of libtest being unusually silly among languages.
I would like for this schema validation test to be in one place, yet it depends on the runtime behaviour of the entire rest of the test suite, which also makes it tricky.
Describe alternatives you've considered
Status quo: the schema is incomplete :(
Additional context Add any other context or screenshots about the feature request here.
I think a better option is to define a set of config types in the code, and use them for both deserialization of values (at runtime) and schema generation. IIRC there are multiple issues/discussions related to this topic, but I couldn't find.
At this point, I believe that all built-in TOML configs are validated against the schema. Until Yuya's suggestion becomes a reality, I'd suggest moving the default values for various configs into TOML built-in config (and have jj's Rust code panic or report an error if it's missing), and to try to prefer that kind of setup over defining the default inside the Rust code.
I also hope that Yuya's suggestion does not have to be an all-or-nothing affair. We can migrate piece-wise once we establish a structure where some pieces of the schema are automatically generated. I believe a schema can refer to other schema files, which should make this simpler, though I'm less sure about whether relative URLs are supported in these references. At worst, we could assemble a single schema json out of pieces using some sort of templating.
Getting the schema from serializeable types would be most useful (IMO) for the config sections where there is a finite number of allowed keys that the users often misspell. So, we could start with those.
Copying my thought from Discord, I think it'd also be nice to allow jj-related tools like jjui to store their config in jj's config.toml (and extract it with jj config get commands). Of course, we couldn't verify such configs against our config schema. In other words, I'm mentioning this because this is trivial to do now but would become harder as we make our config stricter (which we should).
I think it would be nice to settle on a convention for where to put arbitrary settings for arbitrary tools. My first idea would be to use a namespace, e.g. we could allow jjui to put its options into the [external.jjui] table (or any sub-tables, e.g. [external.jjui.ui]) and to allow it to put its own colors in the [colors.external.jjui] table. It'd be nice if we could agree on a name (e.g. "external" was my first thought).
Another option would be to reserve any table starting with x- for this, e.g. [x-jjui].
The larger question here is whether we agree that doing something to support storing arbintrary tools' configs in jj's config.toml is desirable. The main advantage is that this immediately allows repo-specific (and maybe workspace-specific) config for any such tool. It also decreases the number of config files.
An example in the wild is that tig can store its config inside .gitconfig's [tig] key, which I happily use. To be fair, tig also allows using ~/.config/tigrc if the user prefers, so perhaps some users prefer that. Also, git clearly did not establish any convention for top-level config keys that external programs should use.
Update: We could also pull out this discussion into a new child issue, if we prefer.
Regarding third party tools in jj.toml, that ship has already sailed and i don't think that feature is removable: for example, it is possible to read config variables in templates today. So having external tools be able to use the config is probably strictly a good thing for everything cohesively fitting together.
One possible way to be strict is if the config forbade unknown settings in namespaces owned by jj, but I am unsure about that even; git definitely doesn't check this and having strict ownership doesn't make sense for stuff like the templates section.
Either way, it's not super duper important for this issue, since jj's own config handling code should definitely not be allowed to read anything that's not in the schema without a good reason.
One possible way to be strict is if the config forbade unknown settings in namespaces owned by jj,
The config schema and the plan in https://github.com/jj-vcs/jj/issues/6797#issuecomment-2993230849 are both, in my mind, closely connected to forbidding (or warning on) unknown settings in namespaces owned by jj. For example, the present-day fact that the config schema only includes finitely many keys is already a way to warn the user on unknown settings, though this way is limited in that it only works for users whose IDE verifies their jj config against the schema.
So, my comment was mostly about defining what the "namespaces owned by jj" should be.
Either way, it's not super duper important for this issue,
As I mentioned above and in the previous comment, I think the two issues are related. Still, it's worth stating explicitly that I agree that the issue I was talking about is not identical to the problem that you originally raised (that the config schema is incomplete).
The config schema and the plan in #6797 (comment) are both, in my mind, closely connected to forbidding (or warning on) unknown settings in namespaces owned by jj.
I wouldn't expect we'll ever implement strict schema validation that rejects any unknown keys. Maybe we can add jj config check command that warns unknown keys, but they shouldn't be an error.