-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Move rustdoc-types
crate to T-Rustdoc ownership.
#3505
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
Changes from 11 commits
4424e89
2997902
0570ccb
35485fd
c411988
6f0a689
ac9a6db
a7ee38e
5b53b84
ec35266
9c5b588
3eb2784
d54ae49
855194d
e0db5f7
4b4696d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
- Feature Name: `rustdoc_types_maintainers` | ||
- Start Date: (fill me in with today's date, YYYY-MM-DD) | ||
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
The [rustdoc-types](https://crates.io/crates/rustdoc-types) crate will go from being individually maintained to being officially maintained by the rustdoc team. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
[`rustdoc-types`](https://crates.io/crates/rustdoc-types) is a crate published on crates.io. It is used by users of the unstable [rustdoc JSON](https://github.com/rust-lang/rust/issues/76578) backend to provided a type representing the output of `rustdoc --output-format json`. It's published on crates.io to be used by out-of-tree tools that take rustdoc-json as an input. E.g: | ||
|
||
| Name | Purpose | | ||
|--|--| | ||
| [awslabs/cargo-check-external-types] | Home-rolled version of [RFC 1977] "private dependencies". Checks if any types from the private dependency are used in a crate's public API. | | ||
| [Enselic/cargo-public-api] | Compares the public API of two crates. Used to check for semver violations. | | ||
| [obi1kenobi/trustfall-rustdoc-adapter] | Higher-level database-ish API for querying Rust API's. Used by [obi1kenobi/cargo-semver-checks] | | ||
|
||
[awslabs/cargo-check-external-types]: https://github.com/awslabs/cargo-check-external-types/blob/dc15c5ee7674a495d807481402fee46fdbdbb140/Cargo.toml#L16 | ||
|
||
[Enselic/cargo-public-api]: https://github.com/Enselic/cargo-public-api/blob/19f15ce4146835691d489ec9db3518e021b638e8/public-api/Cargo.toml#L27 | ||
|
||
[obi1kenobi/trustfall-rustdoc-adapter]: https://github.com/obi1kenobi/trustfall-rustdoc-adapter/blob/92cbbf9bc6c9dfaf40bba8adfbc56c0bb7aff12f/Cargo.toml#L15 | ||
|
||
[obi1kenobi/cargo-semver-checks]: https://github.com/obi1kenobi/cargo-semver-checks | ||
|
||
[RFC 1977]: https://rust-lang.github.io/rfcs/1977-public-private-dependencies.html | ||
|
||
Currently I ([`@aDotInTheVoid`](https://github.com/aDotInTheVoid/)) maintain the `rustdoc-types` crate on [my personal GitHub](https://github.com/aDotInTheVoid/rustdoc-types/). No-one else has either GitHub or crates.io permissions. This means if I become unable (or more likely disinterested), the crate will not see updates. | ||
|
||
Additionally, if an update to `rustdoc-json-types` happens while I'm away from a computer for an extended period of time, there will be a delay in this update being published on crates.io. This happened with format_version 29, which was merged on [April 8th](https://github.com/rust-lang/rust/commit/537aab7a2e7fe9cdf50b5ff18485e0793cd8db62), | ||
but was only published to crates.io on | ||
[April 19th](https://github.com/aDotInTheVoid/rustdoc-types/commit/ad92b911488dd42681e3dc7e496f777f556a94f6), due to personal reasons. | ||
[This almost happened previously](https://github.com/aDotInTheVoid/rustdoc-types/issues/25), but was avoided due to the bors queue being quiet at the time. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
This involves: | ||
|
||
1. Moving the [github.com/aDotInTheVoid/rustdoc-types](https://github.com/aDotInTheVoid/rustdoc-types/) repo to the `rust-lang` organization, and make `rust-lang/rustdoc` maintainers/owners. | ||
2. Move ownership of `rustdoc-types` on crates.io to the rustdoc team. | ||
|
||
# Reference-level explanation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: I think we should carefully document what our versioning strategy is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the crate, or the For the crate, the current approach is "Follow semver, and don't release 1.0.0 until stabilization". The post 1.0.0 versioning strategy will be figured out in concert with designing the format for post-stabilization evolution. How much detail on versioning strategy do you think this RFC needs? From a users POV, their should be no change from this, and new releases will be published in the same fashion as before. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The crate. We should both:
It's going to be an official Rust artefact, we need to be a bit clearer to our users. |
||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
`rustdoc-types` is a republishing of the in-tree [`rustdoc-json-types`](https://github.com/rust-lang/rust/tree/b8536c1aa1973dd2438841815b1eeec129480e45/src/rustdoc-json-types) crate. `rustdoc-json-types` is a dependency of `librustdoc`, and is the canonical source of truth for the rustdoc-json output format. Changes to the format are made as a PR to `rust-lang/rust`, and will modify `src/rustdoc-json-types`, `src/librustdoc/json` and `tests/rustdoc-json`. None of this will change. | ||
|
||
Republishing `rustdoc-json-types` as `rustdoc-types` is done with [a script](https://github.com/aDotInTheVoid/rustdoc-types/blob/17cbe9f8f07de954261dbb9536c394381770de7b/update.sh) so that it is as low maintenance as possible. This also ensures that all format/documentation changes happen in the rust-lang/rust repo, and go through the normal review process there. | ||
|
||
The update/publishing process will be moved to T-rustdoc. In the medium term, I (`@aDotInTheVoid`) will still do it, but | ||
- In an official capacity | ||
- With bus factor for when I stop. | ||
|
||
We (T-rustdoc) will continue to publish a new version of the `rustdoc-types` crate | ||
every time the upstream implementation changes, and these will be versioned with | ||
normal SemVer. Changes to rustdoc-json in `rust-lang/rust` will not be accepted | ||
if they would make it not possible to publish `rustdoc-types`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little unclear about how this could ever happen. Do you mind giving an example, and/or clarifying the wording to be more precise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stuff that depends on rust-lang/rust's "weird" build environment. In paticular:
I'll add these to the RFC. |
||
|
||
## Actual Mechanics of the move | ||
|
||
### GitHub | ||
|
||
GitHub has a [list of requirements](https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository) for transferring repositories. T-infra will handle the permissions of moving the repository into the rust-lang GitHub organization. | ||
|
||
At the end of this we should have a crate in the rust-lang GitHub org with T-rustdoc as contributors, and T-infra as owners. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what this is trying to say here. "crate" is in crates.io, not GitHub typically. I also don't see what "T-infra as owners" means -- but it seems pretty surprising for T-infra to be an owner. Do we mean the rust-lang-owner account on crates.io? The other sections don't seem to elaborate on this sentence. I at least am not comfortable with infra being an owner for this crate or accompanying repository: it feels very much out of our general scope as a team, and very much "just" a rustdoc concern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Gah sorry, this is my bad writing. I mean the
Agreed. I meant this in terms of GitHub permissions, not who'se maintaining the crate/repo. If the repo admin permissions should instead go to rust-lang-owner (instead of the infra team or even the rustdoc team) I'm happy to do that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm confused, but aren't we moving that repo into the rust-lang org? If so, then permissions are granted via the team repo and should only consist of teams that need access (in this case, presumably rustdoc). Infra has access if needed through the org admins, but that is true for all repos in the org so generally doesn't need specific enumeration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In any case, I'm happy with the intent now that I understand it :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes
Ah. That makes things even simpler. |
||
|
||
### crates.io | ||
|
||
crates.io ownership is managed [via the command line](https://doc.rust-lang.org/cargo/reference/publishing.html#cargo-owner). | ||
|
||
I will run the following commands to move ownership. | ||
|
||
``` | ||
cargo owner --add github:rust-lang:rustdoc | ||
cargo owner --add rust-lang-owner | ||
cargo owner --remove aDotInTheVoid | ||
``` | ||
|
||
The `rust-lang-owner` is needed because team owners cannot add new owners. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
- Adds additional maintenance burden to rustdoc team. | ||
- One-time maintenance burden to infra team to support move. | ||
|
||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
- We could keep `rustdoc-types` as a personal project. This preserves the status quo (and is what will happen if this RFC (or something similar) isn't adopted). This is undesirable because | ||
- Bus factor: If I am unable or unwilling to maintain `rustdoc-types`, we cause a load of unnecessary churn when it becomes out of sync with the in-tree `rustdoc-json-types` | ||
- We could bundle `rustdoc-types` through rustup. This is undesirable as it means users can't depend on it in stable rust, and can't depend on multiple versions. | ||
- We could publish `rustdoc-json-types` directly from `rust-lang/rust`. However | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I am a little concerned about having a separate repo because it would mean every PR that increases FORMAT_VERSION would also necessitate a separate PR to a different repo. Is there a downside to publishing from a folder in rust-lang/rust instead (or maybe even a git subtree)? See also my comments below about merging it with rustdoc-json-types, though my main concern is requiring multiple PRs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
FWIW, this is how it's always been done, but that's defiantly not sufficient justification that it's the best way.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whats your concern with multiple PR's? Given that there's already a publish step, I don't think it cuts down on work (unless we autopublish from rust-lang/rust PR's, which I'm not fully comfortable with) We could land this now (primarily for bus-factor reasons), and then move change things separately if it becomes a problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fair enough since all that's needed to update the repo is to run a script.
Yeah, I guess that makes sense since it wouldn't even need a whole RFC for a small administrative change like that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, the impression I got from the RFC text was you wanted publishing to happen almost as soon as the format version changed. I agree that there's benefit to reviewing the changes before publishing a new crate version. |
||
- `rust-lang/rust` doesn't currently publish to crates.io. | ||
- `rustdoc-json-types` doesn't currently bump the version field in `Cargo.toml` | ||
- It may be desirable to one day use different types for rustdoc serialization vs users deserialization | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW we could use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we should probably do that either way, to replace the increasing pile of regex we currently use. |
||
|
||
Reasons for this: | ||
- It could enable performance optimizations by avoiding allocations into strings | ||
- It could help with stabilization: | ||
- Allows making structs/enums `#[non_exhaustive]` | ||
- Allows potentially supporting multiple format versions. | ||
- `rustdoc-types` is a nicer name, and what people already depend on. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems irrelevant since we could just rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fair. |
||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
- [Rust RFC 3119](https://rust-lang.github.io/rfcs/3119-rust-crate-ownership.html) establishes the Rust crate ownership policy. Under its categories, `rustdoc-types` would be an **intentional artifact** | ||
- [Some old zulip discussion about why `rustdoc-json-types` was created.](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/JSON.20Format/near/223685843) What was said then is that if T-Rustdoc wants to publish a crate, it needs to go through an RFC. This is that RFC. | ||
- the [`cargo | ||
metadata`](https://doc.rust-lang.org/cargo/commands/cargo-metadata.html) | ||
command gives JSON information about a cargo package. The | ||
[cargo-metadata](https://docs.rs/cargo_metadata/latest/cargo_metadata/) crate | ||
provides access to this. Instead of being a export of the cargo-side type declarations, | ||
it's manually written, and not officially maintained. This has [lead to compatibility issues](https://github.com/oli-obk/cargo_metadata/issues/240) | ||
in the past. Despite being stable, the exact compatibility story [isn't yet determined](https://github.com/rust-lang/cargo/issues/12377). | ||
|
||
aDotInTheVoid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
None yet | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
When the rustdoc-json feature is stabilized, we should release 1.0.0 to crates.io. How we can evolve the format post stabilization is an unanswered question. It's a blocker for stabilization, but not this RFC | ||
|
Uh oh!
There was an error while loading. Please reload this page.