Skip to content
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

docs: Declare support level for each crate in our Charter / docs #14600

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 26, 2024

What does this PR try to resolve?

This is to bring us into conformance with the Rust crate ownership policy.

Items of note

  • cargo does not have a status specified on it. cargo install cargo shouldn't be done and cargo add cargo is "internal" but putting in our README that Cargo is "internal" feels weird from a messaging perspective.
  • cargo-credential-1password is declared as Experimental as it is intended for the community but I was unsure if we wanted to commit to full support for it. In my mind, the ideal thing to do would be to expatriate this to 1password.
  • home is declared as Internal despite its wide use within the ecosystem.
  • cargo-credential is declared as Intentional as its an API intended for the wider ecosystem and I didn't see a reason to declare it experimental.
  • cargo-platform, cargo-util-schemas, and crates-io are declared as Intentional as they are both used internally and intended for others to use for logic that integrates with cargo/registries. I wondered about these being Experimental or Internal instead.

How should we test and review this PR?

I was mixed on what to do with unpublished crates (and didn't fully check what all is unpublished).
I ended up only skipping xtasks.

Additional information

@epage epage added the T-cargo Team: Cargo label Sep 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 26, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-cfg-expr Area: Platform cfg expressions A-cli-help Area: built-in command-line help A-credential-provider Area: credential provider for storing and retreiving credentials A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-environment-variables Area: environment variables A-home Area: the `home` crate A-interacts-with-crates.io Area: interaction with registries A-testing-cargo-itself Area: cargo's tests Command-fix S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2024
@epage
Copy link
Contributor Author

epage commented Sep 26, 2024

While the policy asks for an RFC for making things Intentional and #14599 isn't approved yet, I'm going to at least start this attempting to codify where things are at, rather than set new policy.

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 26, 2024

Team member @epage 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 An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Sep 26, 2024
This is to bring us into conformance with the [Rust crate ownership
policy](https://forge.rust-lang.org/policies/crate-ownership.html).

Items of note
- `cargo-credential-1password` is declared as Experimental as it is
  intended for the community but I was unsure if we wanted to commit to
  full support for it.  In my mind, the ideal thing to do would be to
  expatriate this to 1password.
- `home` is declared as Internal despite its wide use within the
  ecosystem.
- `cargo-credential` is declared as Intentional as its an API intended
  for the wider ecosystem and I didn't see a reason to declare it
  experimental.
- `cargo-platform`, `cargo-util-schemas`, and `crates-io` are declared
  as Intentional as they are both used internally and intended for
  others to use for logic that integrates with cargo/registries.
  I wondered about these being Experimental or Internal instead.
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together, it looks great!

@@ -9,6 +9,10 @@ This is a low-level library. You pass it the JSON output from `rustc`, and you c

If you are looking for the [`cargo fix`] implementation, the core of it is located in [`cargo::ops::fix`].

> This crate is maintained by the Cargo team, primarily for use by Cargo
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, this one is a bit tricky. It is used by compiletest and ui_test. I wonder about making this a little more broad, such as "Cargo and the Rust compiler test suite" or something like that?

Comment on lines +1 to +2
> This crate is maintained by the Cargo team for use by the wider
> ecosystem. This crate follows semver compatibility for its APIs.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm on the fence here. My intention when I created it was to be more "internal". I was thinking that if others found it useful, we could expand that to a more "experimental" status. I didn't know if anyone would actually have a use for it, and it wasn't really something I wanted to put effort into.

Also, as evident by the lack of a README, this isn't really up to my standards for an intentional artifact that we intend to support.

It looks like it has a few dependents now, though: https://crates.io/crates/cargo-platform/reverse_dependencies.

I'm fine with elevating this to be "intentional" if ya'll agree. It has required almost no maintenance effort, and I don't expect us to delete it, and we already have semver checks. I just wanted to make clear what I was originally thinking.

(We probably should eventually add a real README, though 🤣)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification!

I think its reasonable for the Rust Project to provide an API for this micro-DSL. My main concern is what shape any of that should take and thats where I feel like the commitment of "Intentional" is strong. Granted, the definition of "Intentional" is strong enough I'm hesitant to mark anything intentional but anything else would also be too weak / scare people away.

@ehuss
Copy link
Contributor

ehuss commented Sep 28, 2024

cargo does not have a status specified on it.

This is probably should say something. I'm personally fine with putting it in the root README. Can you say more about your concern about the messaging? Is it related to the README having different audiences?

@epage
Copy link
Contributor Author

epage commented Sep 29, 2024

This is probably should say something. I'm personally fine with putting it in the root README. Can you say more about your concern about the messaging? Is it related to the README having different audiences?

I think thats a way of summing it up. If someone goes to the Cargo repo to learn about Cargo and sees a big banner saying that Cargo is experimental or internal, that will send the wrong message. However, we do want to send a message like that to anyone who builds the binary directly or tries to depend on it as a library.

@ehuss
Copy link
Contributor

ehuss commented Oct 1, 2024

I think it should be fine to include the notice in the root README with the right wording to make it clear what it means. The README already has some low-level documentation for building cargo, which is for a very limited audience.

@epage
Copy link
Contributor Author

epage commented Oct 1, 2024

I've done a pass at wording for cargo in both README.md and src/cargo/lib.rs

@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Oct 1, 2024
@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Oct 1, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 1, 2024

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

@@ -9,6 +9,13 @@ Cargo downloads your Rust project’s dependencies and compiles your project.
[The Cargo Book]: https://doc.rust-lang.org/cargo/
[Cargo Contributor Guide]: https://rust-lang.github.io/cargo/contrib/

> The Cargo binary distributed through with Rust is maintained by the Cargo
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 already mentioned in the "Installing Cargo" section.

Should we move "For all other uses of this crate..." to under "Compiling from Source"? Something like

## Compiling from Source

While [the `cargo` crate](https://crates.io/crates/cargo) is avaiable via crates.io,
compiling it from source is not intended for external use,
regardless for using either as a binary or library.
This crate may make major changes to its APIs.

### Requirements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That section heavily implies "building the binary from source" and doesn't really cover the "depending on it as a library" case.

I feel like a lot of this should be re-worked, mostly by moving in to the contributor guide. I recommend we remove the install instructions and defer re-working the compilation instructions.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Since Eric's concern has been addressed. I am going to merge this.

If there are other concerns, we can address later.

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 2, 2024

📌 Commit 5c87c14 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2024
@bors
Copy link
Collaborator

bors commented Oct 2, 2024

⌛ Testing commit 5c87c14 with merge 2652990...

@bors
Copy link
Collaborator

bors commented Oct 2, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2652990 to master...

@bors bors merged commit 2652990 into rust-lang:master Oct 2, 2024
24 checks passed
@epage epage deleted the charter-crates branch October 2, 2024 20:34
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 5, 2024
Update cargo

17 commits in 80d82ca22abbee5fb7b51fa1abeb1ae34e99e88a..ad074abe3a18ce8444c06f962ceecfd056acfc73
2024-09-27 17:56:01 +0000 to 2024-10-04 18:18:15 +0000
- test: Remove the last of our custom json assertions (rust-lang/cargo#14576)
- docs(ref): Expand on MSRV (rust-lang/cargo#14636)
- docs: Minor re-grouping of pages (rust-lang/cargo#14620)
- docs(ref): Highleft whats left for msrv-policy (rust-lang/cargo#14638)
- Fix `cargo:version_number` - has only one `:` (rust-lang/cargo#14637)
- docs: Declare support level for each crate in our Charter / docs (rust-lang/cargo#14600)
- chore(deps): update tar to 0.4.42 (rust-lang/cargo#14632)
- docs(charter): Declare new Intentional Artifacts as 'small' changes (rust-lang/cargo#14599)
- fix: Remove implicit feature removal (rust-lang/cargo#14630)
- docs(config): make `--config <PATH>` more prominent (rust-lang/cargo#14631)
- chore(deps): update rust crate unicode-width to 0.2.0 (rust-lang/cargo#14624)
- chore(deps): update embarkstudios/cargo-deny-action action to v2 (rust-lang/cargo#14628)
- docs(ref): Clean up language for `package.rust-version` (rust-lang/cargo#14619)
- docs: clarify `target.'cfg(...)'`  doesnt respect cfg from build script (rust-lang/cargo#14312)
- test: relax compiler panic assertions (rust-lang/cargo#14618)
- refactor(compiler): zero-copy deserialization when possible (rust-lang/cargo#14608)
- test: add support for features in the sat resolver (rust-lang/cargo#14583)
@rustbot rustbot added this to the 1.83.0 milestone Oct 5, 2024
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cfg-expr Area: Platform cfg expressions A-cli-help Area: built-in command-line help A-credential-provider Area: credential provider for storing and retreiving credentials A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-environment-variables Area: environment variables A-home Area: the `home` crate A-interacts-with-crates.io Area: interaction with registries A-testing-cargo-itself Area: cargo's tests Command-fix disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants