Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 16 commits into from
Closed

Move rustdoc-types crate to T-Rustdoc ownership. #3505

Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4424e89
rustdoc-types-maintainers: Initial Version
aDotInTheVoid Oct 3, 2023
2997902
rustdoc-types-maintainers: Fix Typo
aDotInTheVoid Oct 5, 2023
0570ccb
Typo Fixes and Rearangement
aDotInTheVoid Oct 5, 2023
35485fd
rustdoc-types-maintainers: More typo fixes
aDotInTheVoid Jan 5, 2024
c411988
Update text because it's been so long.
aDotInTheVoid May 8, 2024
6f0a689
Fix copy&paste messup.
aDotInTheVoid May 8, 2024
ac9a6db
Updates
aDotInTheVoid Jul 6, 2024
a7ee38e
typo fixes
aDotInTheVoid Jul 6, 2024
5b53b84
Clairify versioning gaurentees
aDotInTheVoid Jul 6, 2024
ec35266
Apply suggestions from code review
fmease Jul 6, 2024
9c5b588
typo-fix a typo-fix
aDotInTheVoid Jul 6, 2024
3eb2784
Clairify github move/ownership.
aDotInTheVoid Jul 6, 2024
d54ae49
use team repo for gh perms
aDotInTheVoid Jul 6, 2024
855194d
rustdoc-types-maintainers.md: typo fix
aDotInTheVoid Sep 22, 2024
e0db5f7
Clairify what being publishable means in practice.
aDotInTheVoid Sep 22, 2024
4b4696d
Fill in details
aDotInTheVoid Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions text/0000-rustdoc-types-maintainers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
- Feature Name: `rustdoc_types_maintainers`
- Start Date: 2023-10-3
Comment on lines +1 to +2
Copy link
Member

@camelid camelid Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits: (no need for a feature name if there's no actual feature)

Suggested change
- Feature Name: `rustdoc_types_maintainers`
- Start Date: 2023-10-3
- Start Date: 2023-10-03

- RFC PR: [rust-lang/rfcs#3505](https://github.com/rust-lang/rfcs/pull/3505)
- 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 provide 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I think we should carefully document what our versioning strategy is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the crate, or the FORMAT_VERSION constant?

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crate. We should both:

  • Document what we plan to do in the near future
  • Make a clear commitment; do we plan to do that for the forseeable future, or do we want flexibility to change how we do it. Are we going to make a release every time it changes? Do we guarantee that we will always pair rustdoc JSON changes with rustdoc-types changes? Or is it acceptable if we change rustdoc JSON in a way that breaks rustdoc-types but not immediately publish a new version?

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` (eg: using `rustc_*`
crates, or nightly features).

## 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 moved the [`aDotInTheVoid/rustdoc-types`]
repo into the rust-lang GitHub org. T-rustdoc will have `maintain` permissions
(via the [team repo](https://github.com/rust-lang/team/)).

### 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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though my main concern is requiring multiple PRs.

FWIW, this is how it's always been done, but that's defiantly not sufficient justification that it's the best way.

Is there a downside to publishing from a folder in rust-lang/rust instead (or maybe even a git subtree)?

  1. Can't tag the git repo with creates.io version
    • I'm not sure how compelling this is
  2. No way to diff view diff between releases
  3. Having the crate be a small repo makes it easier to depend on it via cargo's git feature (rather than having cargo clone rl/r). update.sh: Make user, repo, and branch easy to change rustdoc-types#14
  4. Requires re-engineering release procedures, and it's unclear how that would work in rust-lang/rust
    • Are we now going to cd into a local clone of rust-lang/rust and cargo publish? This is involved to
    • Alternatively, is rust-lang/rust CI going to autopublish? This is a whole can of worms, and it'd be much easier to set up auto-publishing on it's own repo.
    • Changelog generation get's tricky if it needs to be done in the PR that implements it.
      • Can't know the date it will be published to crates.io before merging, due to unpredictable bors delay
        • But maybe we don't need this in the changelog, it's not super intuitive how this is ties to crates.io (not rustup dates)
  5. I kinda like that the publish happens later, and gets another pass on it before going out to users:

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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)

Fair enough since all that's needed to update the repo is to run a script.

We could land this now (primarily for bus-factor reasons), and then move change things separately if it becomes a problem.

Yeah, I guess that makes sense since it wouldn't even need a whole RFC for a small administrative change like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda like that the publish happens later, and gets another pass on it before going out to users:

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we could use cfgs for this (like feature = "unstable_internal_rustdoc" for the internal rustdoc version). I don't think we'd want to maintain two completely separate versions of the API anyway, since it would be confusing to keep in sync.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems irrelevant since we could just rename rustdoc-json-types to rustdoc-types.

Copy link
Member Author

Choose a reason for hiding this comment

The 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).

# 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