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

[pull] master from rust-lang:master #65

Open
wants to merge 304 commits into
base: master
Choose a base branch
from
Open

Conversation

pull[bot]
Copy link

@pull pull bot commented Nov 15, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

epage and others added 30 commits November 13, 2024 13:32
At this point, it is assumed to always be up-to-date
`cargo remove` already will suggest other tables if appropriate.
`cargo add` auto-corrects `_` and `-`.

This is an extension of the two by suggesting existing dependencies
within the same table that are "close".

I chose to make alt tables and alt deps mutually exclusive in the
message because I didn't wantto deal with how to separate things if both
show up.
Most likely, people will only expect one or the other and if its in an
alt table, then most likely they mean that.

Related to #14814
### What does this PR try to resolve?

Did a quick audit for parts of the API and implementation that stood out
to me. So far, I've mostly focused on "input".

### How should we test and review this PR?

### Additional information

CC @CAD97
Originally it was only included for packages that have executables or
examples for `cargo install`, however this causes inconsistencies and
is kind of unexpected nowadays, e.g. with cdylib crates.

Including it always only slightly increases the crate size and allows
for all crates to know a set of dependency versions that were working,
which can make regression tracking easier.

Fixes #13447
### What does this PR try to resolve?

Originally it was only included for packages that have executables or
examples for `cargo install`, however this causes inconsistencies and is
kind of unexpected nowadays, e.g. with cdylib crates.

Including it always only slightly increases the crate size and allows
for all crates to know a set of dependency versions that were working,
which can make regression tracking easier.

Fixes #13447

### How should we test and review this PR?

The existing tests are covering this change in all kinds of various
already, and one test that previously asserted that there is *no*
Cargo.lock for library crates was changed to explicitly check for the
new behaviour.
### What does this PR try to resolve?

`cargo remove` already will suggest other tables if appropriate.
`cargo add` auto-corrects `_` and `-`.

This is an extension of the two by suggesting existing dependencies
within the same table that are "close".

Related to #14814

### How should we test and review this PR?

I chose to make alt tables and alt deps mutually exclusive in the
message because I didn't wantto deal with how to separate things if both
show up.
Most likely, people will only expect one or the other and if its in an
alt table, then most likely they mean that.

### Additional information
This was mostly done by clippy via `clippy::doc_markdown`.
I then reviewed it to fix words that shouldn't have it or where `--fix`
put the backtick in the wrong location.
This was aided by `clippy::too_long_first_doc_paragraph`
### What does this PR try to resolve?

I'm experimenting with API changes in Cargo and wanted to make it easier
to find doc comments that need updating. I didn't see a lint to help
with this but I did go ahead and apply the result of other documentation
lints.

### How should we test and review this PR?

I've not turned these lints on more generally
- `clippy::doc_markdown` has some false positives we'd need to record in
our config (`SemVer` and `SQLite`)
- `clippy::too_long_first_doc_paragraph` pointed out a doc comment that,
at a quick glance, I didn't see what summary to provide

I also didn't wanted to get this in without making a policy decision of
what lints are enabled.

### Additional information
### What does this PR try to resolve?

This is a follow up to #14639 in prep for Edition 2024

### How should we test and review this PR?

This is stacked on #14753

### Additional information
Now called `UnitHash` and `Metadata`.

The goal is to provide role-specific `UnitHash` accessors on the new `Metadata`.
I chose this method, instead of changing the accessors on
`CompilationFiles`, to have the least impact on the documentation as
what is called `Metadata` can remain the focus of the documentation,
rather than having it apply to different `CompilationFiles` accessors
which don't house the actual logic.
We have
- `-C metadata`
- `-C extra-filename`
- Uniquifying build scripts
- Uniquifying scraped documentation

The last two I'm tracking as a `unit_id`.  As we evolve the first two
hashes, we can decide which would be a better fit for backing this.
For scraping,  we could treat this as
`-C extra-filename` but then we'd have a lot of `.expect()`s.
First draft of trying to simplify the English used in the guide section
of the book, as well as normalising between 'you' for the reader and
'we' for the authors
### What does this PR try to resolve?

Found this when working on another PR

### How should we test and review this PR?

### Additional information
### What does this PR try to resolve?

We removed rejected syntax in #13861 haven't update frontmatter parsing
to what [RFC
3503](https://rust-lang.github.io/rfcs/3503-frontmatter.html#reference-level-explanation)
says.

This adds tests and fixes various cases to ensure we correctly detect
the frontmatter and the infostring.

This is part of #12207

### How should we test and review this PR?

### Additional information
epage and others added 30 commits December 11, 2024 19:29
### What does this PR try to resolve?

This helps `-Ztrim-paths` build a stable cross-platform path for the
registry and git sources. Sources files then can be found from the same
path when debugging.

It also helps cache registry index all at once for all platforms,
for example the use case in
#14795
(despite they should use `cargo vendor` instead IMO).

Some caveats:

* Newer cargo will need to re-download files for global caches
  (index files, git/registry sources).
  The old cache is still kept and used when running with older cargoes.
* Absolute paths on windows iarenot really covered by the
"cross-platform" hash,
  because path prefix components like `C:` are always there.
  That means hashes of some sources kind,
  like local registry and local path,
  are not going to be real cross-platform stable.

#### Security concern

There might be hash collisions if you have two registries under the same
domain. This won't happen to crates.io, as the infra would have to
intentionally put another registry on index.crates.io to collide.
We don't consider this is an actual threat model, so we are not going to
use any cryptographically secure hash algorithm like BLAKE3.

At least, the current unstable SipHash isn't in a better situation.
We might switch to a cryptographic secure one when needed.

See also
<#13171 (comment)>

### How should we test and review this PR?

We have an FCP in
<#14795 (comment)>.

This PR implements the proposal,
The path-length concern in
<#14795 (comment)>
is automatically addressed
because we don't need cryptographically secure hash for now.

### Additional information

See more information and benchmark results in
<#14116>.
This has been around since #12224 and isn't in the RFC, so its being
removed.

This is part of #12207.
This has been around since #12224 and isn't in the RFC, so its being
removed.

This is part of #12207.

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
I'm writing a compatibility layer between the cargo and meson build
systems.
This change allows me to plumb that through better :)

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
…14923)

### What does this PR try to resolve?

The user likely intended what they said and we should tell them why it
didn't work.
Rejected versions also imply alt versions below them.

Fixes #4260

In changing the errors from alt-versions to rejected-versions, I found
part of the old error message was better and changed the message
introduced in #14897 from matching alt-names to alt-versions.

This includes the test for #14921. The "fix" was done before this
because as it was part of the refactors in prep for this.

### How should we test and review this PR?

### Additional information

Next steps after this are to still produce an
`IndexSummary::Unsupported` even if there are deserialization errors, so
long as we know the name and version which should take care of #10623
and #14894.
While #14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers": "<semver>"}`.

As a side effect, the index cache will include more "invalid" index
entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to #6880 when the index cache was
introduced.

Fixes #10623
Fixes #14894
…playDepth`

refactor(tree): replace previous `depth: u32` opts with new type `DisplayDepth`
### What does this PR try to resolve?

While #14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers":
"<semver>"}`.

Fixes #10623
Fixes #14894

### How should we test and review this PR?

My biggest paranoia is some bad interaction with the index cache
including more "invalid" index entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to #6880 when the index cache was
introduced.

### Additional information
I found a bug in the manifest parser and figured this would help make it
more obvious.

Since I was already changing the order, I figure I'm make things a
little more logical (user-facing first, implementtion details later)
### What does this PR try to resolve?

This bug has been there since #14360

Found this when looking to change the normalization code for cargo
script. As a result, I also changed `cargo-util-schemas` in the hope
that grouping the fields would make the intent clearer. Since I was
already changing field order, I also re-ordered for my personal taste
(user facing features first)

### How should we test and review this PR?

### Additional information
It checks rustup existence even when test is skipped for other reasons
### What does this PR try to resolve?

It checks rustup existence even when test is skipped for other reasons.

### How should we test and review this PR?

See
<rust-lang/rust#134278 (comment)>.

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.