-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add --print target-spec-json-schema #144498
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
With this macro we only need to enumerate every variant once. This saves a lot of duplication already between the definition, the `FromStr` impl and the `ToJson` impl. It also enables us to do further things with it like JSON schema generation.
547cba8
to
d6acb28
Compare
This comment has been minimized.
This comment has been minimized.
d6acb28
to
8f1abd3
Compare
This comment has been minimized.
This comment has been minimized.
8f1abd3
to
a07cfd1
Compare
This comment has been minimized.
This comment has been minimized.
This schema is helpful for people writing custom target spec JSON. It can provide autocomplete in the editor, and also serves as documentation when there are documentation comments on the structs, as `schemars` will put them in the schema.
a07cfd1
to
d4383c1
Compare
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
Some changes occurred in compiler/rustc_codegen_ssa The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. These commits modify the If this was unintentional then you should revert the changes before this PR is merged. These commits modify compiler targets. This PR modifies cc @jieyouxu There are changes to the cc @jieyouxu |
The MCP isn't accepted yet, but that doesn't need to stop anyone from reviewing. |
|
I can probably review it after the MCP passes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me overall, a few questions
fn json_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { | ||
schemars::json_schema! ({ | ||
"type": "string", | ||
"enum": ["false", "true", "wasm", "musl", "mingw"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: for some options, it seems like there are no target_spec_enum
-like help where the possible variants are sync'd. Not that I think this is a big problem, just some extra potential for desync.
schemars::json_schema! ({ | ||
"type": "string", | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: is this because it's annoying if not impossible to express an array of:
- "Simple strings" like
none
ordefault-for-arch
, vs - "Key-value strings" like
llvm-module-flag=xxx
orllvm-arg=xxx
, right?
@@ -361,6 +362,8 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ | |||
"rand_xorshift", // dependency for doc-tests in rustc_thread_pool | |||
"rand_xoshiro", | |||
"redox_syscall", | |||
"ref-cast", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark: at least this uses rustc version detection, not just nightly detection
None of them are blocking, so feel free to r=me once the MCP is accepted. |
This schema is helpful for people writing custom target spec JSON. It can provide autocomplete in the editor, and also serves as documentation when there are documentation comments on the structs, as
schemars
will put them in the schema.I was motivated to do this because I saw someone write their own version of this schema by hand, so demand for this clearly exists. It's not a lot of effort to implement, so I thought it would make sense.
MCP: rust-lang/compiler-team#905
I think it would also be useful to put this in the sysroot in
etc
so people can link it directly in their editors.I would have loved to add a test that validates the JSON schema against the spec JSON of every builtin target, but I don't want to do it as the JSON schema validation crates have incredible amounts of dependencies because JSON schema supports a ton of random features. I don't want to add that, even as a dev dependency.