Skip to content

Rust-lang crate ownership policy #3119

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

Merged
merged 10 commits into from
Sep 11, 2021

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented May 4, 2021

The Rust project publishes a lot of crates, and it's not always clear what their guarantees are. This aims to start an endeavor to categorize these crates and introduce a policy for consistently messaging their raison d'être and guarantees.

This policy doesn't directly affect crates published by the wider ecosystem, however the results of such categorization will likely be very useful to the ecosystem for knowing what should and shouldn't be depended on.

📝 Rendered 📝

@Manishearth Manishearth added the T-core Relevant to the core team, which will review and decide on the RFC. label May 4, 2021
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Should we pick categories for all the existing crates as part of the RFC? Or will that be done by individual teams afterwards?

Comment on lines +64 to +67
### Internal use
“Internal use” crates should contain the following text near the top of the readme/documentation:

> This crate is maintained by \[team\], primarily for use by \[rust project(s)\] and not intended for external use (except as a transitive dependency). This crate may make major changes to its APIs or be deprecated without warning.
Copy link
Member

Choose a reason for hiding this comment

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

This is the closest fit for docs.rs and crates.io I think, but the last warning doesn't apply very well to them - they're not libraries, nor will they ever be deprecated. Can the teams have flexibility to change this message a little?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those are "placeholders" -- if it's not actually a library it's a placeholder; and you should probably not publish the actual code to crates.io. (OTOH conduit is an "internal use" library for crates.io)

But yes, I think we can have flexibility.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think I misunderstood - this is a policy for crates published to crates.io, not all crates in the rust-lang org. Never mind then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct.


- **Intentional artifacts**: These are crates which are intentionally released by some team (usually libs), are actively maintained, are intended to be used by external users, and intentionally have an air of officialness. Example: [libc](https://crates.io/crates/libc)
- **Internal use**: These are crates which are used by some “internal client”, like rustc, crates.io, docs.rs, etc. Their primary purpose is not to be used by external users, though the teams that maintain them (typically the teams of their internal client) may wish for the crate to have wider adoption. The line can be blurry between these and “intentional artifacts” and ultimately depends on the goals of the team. Example: [conduit](https://crates.io/crates/conduit), [measureme](https://crates.io/crates/measureme). There are two subcategories based on whether they are intended to ever show up as a transitive dependency:
- **Transitively intentional**: These are dependencies of intentional artifact libraries, and will show up in users' dependency trees, even if they are not intended to be _directly_ used. The Rust Project still needs to handle security issues in these crates _as if_ they are "intentional artifacts".
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- **Transitively intentional**: These are dependencies of intentional artifact libraries, and will show up in users' dependency trees, even if they are not intended to be _directly_ used. The Rust Project still needs to handle security issues in these crates _as if_ they are "intentional artifacts".
- **Transitively intentional**: These are dependencies of intentional artifact libraries, and will show up in users' dependency trees, even if they are not intended to be _directly_ used. The Rust Project still needs to handle security issues in these crates _as if_ they are "intentional artifacts". Example: [compiler_builtins](https://github.com/rust-lang/compiler-builtins).

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I think that crate is not transitively intentional -- it will never show up in deptrees

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, what would you consider a transitively intentional crate then? compiler_builtins is a dependency of core, and will show up if you build core from source with -Zbuild-std.

Copy link
Member Author

Choose a reason for hiding this comment

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

If a security issue in one of these crates affects a published binary (or crates.io, etc), that will still need to be handled as a bug in the binary or website.

I should mention the stdlib as well.

A transitively intentional crate would be an internal dependency of libc, etc. Basically, it has to show up in your Cargo dependency tree (and as such you need to worry about keeping it up to date).

Copy link
Member Author

Choose a reason for hiding this comment

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

(I suspect your confusion is stemming from the same place as the other one -- the stdlib is not a published crate, so it is external to this ontology)

Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Awesome! I love the idea of more intentionally communicating a crate's policy/commitments.

@ehuss
Copy link
Contributor

ehuss commented May 4, 2021

This looks great! I'm wondering what the intent is around crates that have no team or are ambiguous? For example, mdbook does not have a formal team, just a set of maintainers. git2 is used heavily by Cargo, and maybe should be under the cargo team, but that is not clear.

What happens if a team dissolves or fades away? (Presumably some sort of transfer to core or another team and possibly deprecated or expatriated? Would core be responsible for that?)

It might be helpful to have some information on the process of creating and publishing a new crate. The details don't need to be in the RFC, but perhaps the RFC could say that this information will be published on the forge? Right now, the process is roughly: for GitHub: ask infra to create a new repo or accept a transfer of an existing one. For crates.io: publish under a personal account, then invite rust-lang-owner, and hope someone behind that alias accepts the invite (or infra sets up the publishing). This process can be a little ad-hoc and difficult, so it might be nice to make it clearer and easier.

I prefer to keep publishing rights to an absolute minimum instead of allowing the entire team. Is infra willing to create additional GitHub sub-teams for that purpose? How does that work?

@Manishearth
Copy link
Member Author

I'm wondering what the intent is around crates that have no team or are ambiguous? For example, mdbook does not have a formal team, just a set of maintainers. git2 is used heavily by Cargo, and maybe should be under the cargo team, but that is not clear.

I think an effort can be made to create a team/project group. E.g. I imagine mdbook fitting under devtools.

git2 can just be the cargo team if it's an "internal use", if it's an "intentional artifact" the cargo team can figure it out with the libs team. This RFC does not prescribe a particular choice, it asks the teams to figure it out -- I expect if this lands we will have a slow process of working through the crates and figuring a lot of this out.

The details don't need to be in the RFC, but perhaps the RFC could say that this information will be published on the forge?

Done, it's mostly a matter of process set by the infra team so it's not worth figuring out the details here

I prefer to keep publishing rights to an absolute minimum instead of allowing the entire team. Is infra willing to create additional GitHub sub-teams for that purpose? How does that work?

I think creating a foo-publish team is totally fine. This can again be included in the actual forge page, and doesn't need to be figured out here.

@Shnatsel
Copy link
Member

Shnatsel commented May 4, 2021

@HeroicKatora and myself been working on a tool that might be of use. It provides visibility into crate ownership for a given crate and all of its dependencies. https://github.com/rust-secure-code/cargo-supply-chain

I've added structured output support (currently in git), so you can create logic for custom policies on top of it. I'd be happy to hear if it works or doesn't work for you. Feature requests are also welcome.


## Transitions and new crates

Teams should feel free to create new crates in any of these categories; however "Intentional Artifact" crates should ideally be accompanied with an RFC. As we move towards having team charters, this will also involve a charter change. Teams should notify [email protected] when they've created such crates so that the core team may track these crates and ensure this policy is applied.
Copy link

Choose a reason for hiding this comment

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

It'd be great to have some guidelines around intentional crates. To me, they seem to occupy a weird niche—important enough for a team to maintain and publish, but not shipped as part of the toolchain itself. When might this be a sensible solution? Should these crates have a long-term plan to become part of core or std?

The thrust of this RFC seems to be to impose some structure upon what was a very ad-hoc process of creating semi-official crates in the rust-lang org. Given that, the other categories make sense to me, but intentional crates still feel odd. What message are they intended to convey to Rust users? "You can trust and rely on these crates, because they are supported by a Rust team just like std"?

If I'm being really honest, crates like libc look like missing bits of core and std. It's pretty hard to write correct FFI without libc, and interop via FFI is a huge selling point of Rust. I'd argue that perhaps we should not be encouraging the creation of new crates like this, but acknowledge the ones that currently exist and are important to the Rust ecosystem while determining a path for them to become part of the distributed toolchain.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm actually kinda surprised at this question: the distinction, to me, is pretty straightforward and (I thought) well known: things go in std if we're comfortable permastabilizing them, intentional artifact crates outside of std are ones where we expect to use semver to make changes. Rust tends to default to the latter since permastabilizing an API is a pretty large commitment.

@shepmaster
Copy link
Member

I suggest retitling this to make it more obvious. Something like

Add ownership policy for crates published by the Rust project

The current title sounds like it is talking about every crate.

@Manishearth Manishearth changed the title Add crate ownership policy Rust-lang crate ownership policy May 4, 2021
@Manishearth
Copy link
Member Author

Oops, my bad, it picked up the title from the commit


Every crate in the organization must be owned by at least one team on crates.io. Teams should use `rust-lang/foo` teams for this. Non-expatriated crates may not have personal accounts as owners; if a crate needs additional owners that are not part of teams; the team should create a project group. Note that this does not forbid non-team (or project group) users from having maintainer access to the repository; it simply forbids them from _publishing_.

Currently it is not possible for a crate to be owned by _only_ a team; the `rust-lang-owner` account (or a similar account to be decided by the infra team) can be used as a stopgap in such cases, but we should try to phase this account out as much as possible. For crates being auto-published, a `rust-lang/publish-bots` team (or individual bot accounts) can be used to allow bot accounts to publish crates.
Copy link
Member

Choose a reason for hiding this comment

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

For crates being auto-published

Is the intent to have this cover the rustc ap crates as well (e.g. https://crates.io/crates/rustc-ap_rustc_ast)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes!

@Manishearth Manishearth added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label May 19, 2021
@Manishearth
Copy link
Member Author

cc @rust-lang/libs

@yaahc
Copy link
Member

yaahc commented Jun 4, 2021

Looks great ^_^

Copy link
Contributor

@inquisitivecrystal inquisitivecrystal left a comment

Choose a reason for hiding this comment

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

I left some comments on things I think could be spelled out more or clarified. The substantive policy here looks good, in my (inexperienced) opinion!

# Summary
[summary]: #summary

Have a more intentional policy around crates published by the Rust project, to be applied to existing and future crates published by us.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Have a more intentional policy around crates published by the Rust project, to be applied to existing and future crates published by us.
Have a more intentional policy around crates published by the Rust project **on crates.io**, to be applied to existing and future crates published by us.

There seems to be a lot of confusion in the other comments about the fact that this only applies to crates published on crates.io. To reduce confusion, I would suggest that portion of the policy be very clearly and explicitly stated in the summary section. I's stated below in the motivation section, but it's sufficiently important to the policy that putting it in the summary section would be a good idea IMO.

We propose the following categories of published crates:


- **Intentional artifacts**: These are crates which are intentionally released by some team (usually libs), are actively maintained, are intended to be used by external users, and intentionally have an air of officialness. Example: [libc](https://crates.io/crates/libc)
Copy link
Contributor

Choose a reason for hiding this comment

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

As a reader, the meaning of "intentional" is a needlessly confusing, and "transitively intentional" below even more so. Maybe something like "artifacts for external use"? If you really want this to be the term of art though, it's probably not a huge problem. Just makes the categories a bit more obscure than they need to be.

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 "for external use" doesn't flow as well for a term of art, which is why I picked "intentional"

@Manishearth Manishearth removed the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jul 2, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Aug 18, 2021

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Aug 18, 2021

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Aug 18, 2021
@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Sep 1, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 1, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Sep 11, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 11, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Manishearth Manishearth merged commit a76ef3a into rust-lang:master Sep 11, 2021
@Manishearth Manishearth deleted the crate-ownership branch September 11, 2021 18:42
@Manishearth
Copy link
Member Author

Tracking issue at rust-lang/rust#88867

@notriddle
Copy link
Contributor

@Manishearth The file 0000-rust-crate-ownership.md should be renamed, and fixed to link to this PR, right?

@Manishearth
Copy link
Member Author

Huh, I thought I did that

@Manishearth
Copy link
Member Author

oh shoot

@Manishearth
Copy link
Member Author

Fixed

mwkmwkmwk pushed a commit to mwkmwkmwk/rfcs that referenced this pull request Sep 23, 2021
It appears that some of the changes that were meant to go to RFC 3119
ended up in the RFC template instead.
nikomatsakis added a commit that referenced this pull request Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-core Relevant to the core team, which will review and decide on the RFC. T-libs Relevant to the library team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.