Revert "Clarify when cargo detects changes"
Reverts rust-lang/cargo#11092
We had not fully confirmed that this language change matches the new behavior.
docs(ref): Clarify workspace settings
### What does this PR try to resolve?
In reviewing the status of #10625, I was reminded
- that we are having growing pains with the workspace documentation
- that `workspace.resolver` isn't documented
So I re-organized the workspace docs to have a high level intro / behavior description and then to focus on being a field reference, much like `manifest.md`. I could see splitting it specifically into tutorial/reference like the overriding dependencies document does it.
When adding `workspace.resolver`, I remembered in the nested workspace discussion there were other workspace related sections that are not present. We now link out to `profile`, `patch`, and `replace`. In doing this, I realized that `patch` and `replace` do not specify their workspace behavior, so I do that.
### How should we test and review this PR?
Look at it commit by commit to get more digestible chunks. Unfortunately, the first commit didn't split up so easily.
### Additional information
Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
<!-- homu-ignore:end -->
[master] Run `reach_max_unpack_size` test only on debug build
`cargo test --release` fails on test `reach_max_unpack_size` as the binary to exercise is optimized. The alternative approach is removing `cfg!(debug_assertions)` from this line.
<9ef926dafc/src/cargo/sources/registry/mod.rs (L842)>
#11089
Clarify when cargo detects changes
### What does this PR try to resolve?
Its not clear if "any file" means "any rust file" or "any file, including non rust files". I think the confusion stemmed from the focus on it belonging to the rust package. So it was unclear if any file in the directory counts as belonging to the package, or if the package only contains rust source files, and therefore a `.proto` or `.fbs` file in the same directory as the Cargo.toml would not trigger a rebuild.
I amended the wording to focus on directories rather than packages.
### How should we test and review this PR?
Please let me know which change it should be, and I will update my PR accordingly. Also, please let me know if my edit is accurate, or if it is misleading!
[master] Fix for CVE-2022-36113 and CVE-2022-36114
This PR includes the fixes for CVE-2022-36113 and CVE-2022-36114 targeting the master branch. See [the advisory](https://blog.rust-lang.org/2022/09/14/cargo-cves.html) for more information about the vulnerabilities.
This gives users of custom registries the same protections, using the
same size limit that crates.io uses.
`LimitErrorReader` code copied from crates.io.
Expose cargo add internals as edit API
Move the manifest editing utilities from cargo add to a new `cargo::util::edit` module as part of prep work for `cargo remove` (#10520). No other substantive changes have been made, as this PR is intended only to reduce the refactoring surface area of the implementation of the feature itself.
## Background
In cargo edit, there are a number of top-level modules which enable editing of Cargo manifest files (including `src/dependency.rs` and `src/manifest.rs`). In #10472, these files were added instead as a submodule of the cargo add command, with the stated intention of breaking them out later for subsequent `cargo-edit` subcommands. This PR follows through on that expectation.
## Decisions
Concerns raised in #10472 regarding this change:
- Where should the editing API should live?
- Proposal: `cargo::ops::edit`
- Justification: precedent has been set by `cargo::ops::resolve` and others to have utils shared by multiple ops live in `cargo::ops`. This is also serves to be a rather conservative API change.
- Concerns: the name `edit` could be overly general for those unfamiliar with the cargo edit project (see alternatives)
- Alternatives:
- `cargo::edit`: this seems to me to be too top level, and would confuse users trying to discover the cargo API
- `cargo::util::edit`: if we want to expose this at a higher level, perhaps renaming to act as a counterpart to `crate::util::toml`
- For each of these, replace `edit` with `toml_edit`, `toml_mut`, `manifest_edit`, `manifest_mut`, `edit_toml`, `edit_manifest` etc. for a more specific module name
- Any more specific naming of types reduce clashes (e.g. `Dependency` or `Manifest` being fairly generic)
- Currently the only thing distinguishing these similarly named types is their path, which the `edit` module makes more clear
- Alternatives: rename to `EditDependency`/`EditManifest`, `TomlDependency`/`TomlManifest`, etc.
Notable changes:
- Add/improve module documentation.
- Add missing item documentation.
- Add "editable" to `Dependency` and `Manifest` to further distinguish them from
similarly named items in the library.
Stylistic changes:
- Terminate doc comments with a period.
- Wrap doc comments.
Take priority into account within the pending queue
This is the PR for the work discussed in [this zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/pending.20queue.20scheduling.20experiments) and whose detailed description and some results are available [here](https://github.com/lqd/rustc-benchmarking-data/tree/main/experiments/cargo-schedules/pending-queue-sorted) with graphs, summaries and raw data -- much of which was shown in the thread as well.
Units of works have a computed priority that is used in the dependency queue, so that higher priorities are dequeued sooner, as documented [here](996a6363ce/src/cargo/util/dependency_queue.rs (L34-L35)).
This PR further applies that principle to the next step before being executed: if multiple pieces of work are waiting in the pending queue, we can sort that according to their priorities. Here as well, higher priorities should be scheduled sooner.
They are more often than not wider than pure chains of dependencies, and this should create more parallelism opportunities earlier in the pipeline: a high priority piece of work represents more future pieces of work down the line, and try to sustain CPU utilization longer (at the potential cost of this parallelism being distributed differently than today, between cargo invoking rustc and rustc's own codegen threads -- when applicable).
This is a scheduling tradeoff that behaves differently for each project, machine configuration, amount of available parallelism at a given point in time, etc, but seems to help more often than hinders: at low-core counts and with enough units of work to be done, so that there is jobserver token contention where choosing a "better" piece of work to work on next may be possible.
There's of course a bit of noise in the results linked above and 800 or so of the most popular crates.io crates is still a limited sample, but they're mostly meant to show a hopefully positive trend: while there are improvements and regressions, that trend looks to be more positive than negative, with the wins being more numerous and with higher amplitudes than the corresponding losses.
(A report on another scheduling experiment -- a superset of this PR, where I also simulate users manually giving a higher priority to `syn`, `quote`, `serde_derive` -- [is available here](https://github.com/lqd/rustc-benchmarking-data/tree/main/experiments/cargo-schedules/pending-queue-prioritized) and also improves this PR's results: the regressions are decreased in number and amplitude, whereas the improvements are bigger and more numerous. So that could be further work to iterate upon this one)
Since this mostly applies to clean builds, for low core counts, and with a sufficient number of dependencies to have some items in the pending queue, I feel this also applies well to CI use-cases (esp. on the free tiers).
Somewhat reassuring as well, and discussed in the thread but not in the report: I've also tried to make sure cargo and bootstrapping rustc are not negatively affected. cargo saw some improvements, and bootstrap stayed within today's variance of +/- 2 to 3%. Similarly, since 3y old versions of some tokio crates (`0.2.0-alpha.1`) were the most negatively affected, I've also checked that recent tokio versions (`1.19`) are not disproportionately impacted: their simple readme example, the more idiomatic `mini-redis` sample, and some of my friends' tokio projects were either unaffected or saw some interesting improvements.
And here's a `cargo check -j2` graph to liven up this wall of text:
---
I'm not a cargo expert so I'm not sure whether it would be preferable to integrate priorities deeper than just the dependency queue, and e.g. have `Unit`s contain a dedicated field or similar. So in the meantime I've done the simplest thing: just sort the pending queue and ask the units' priorities to the dep queue.
We could just as well have the priority recorded as part of the pending queue tuples themselves, or have that be some kind of priority queue/max heap instead of a Vec.
Let me know which you prefer, but it's in essence a very simple change as-is.
fix(add): Clarify which version the features are added for
### What does this PR try to resolve?
This gives a hint to users that we might not be showing the feature list
for the latest version but the earliest version.
Also when using a workspace dependency, this is a good reminder of what the version requirement is that was selected. That could also be useful for reused dependencies but didn't want to bother with the relevant plumbing for that.
ie we are going from
```console
$ cargo add chrono@0.4
Updating crates.io index
Adding chrono v0.4 to dependencies.
Features:
- rustc-serialize
- serde
```
to
```console
$ cargo add chrono@0.4
Updating crates.io index
Adding chrono v0.4 to dependencies.
Features as of v0.4.2:
- rustc-serialize
- serde
```
### How should we test and review this PR?
I'd recommend looking at this commit-by-commit. This is broken up into several refactors leading up the main change. The refactors are focused on pulling UI logic out of dependency editing so we can more easily evolve the UI without breaking the editing API. I then tweaked the behavior in the final commit to be less redundant / noisy.
The existing tests are used to demonstrate this is working.
### Additional information
I'm also mixed on whether the meta version should show up.
Fixes#11073
The workspace behavior doesn't seem to be documented at all, so a blurb
was brought in that is like the profile blurb. The workspace docs then
link out to it so users can be aware of this special workspace behavior.
In a workspace, only the workspace's profile is respected. This is
already documented in the profile documentation but we should raise
visibility of it within the workspace documentation.
In looking over #10625, I remembered we've been having growing pains
with the workspace documentation. It was originally written when there
were a lot fewer workspace features. As more workspace features have
been added, they've been tacked on to the documentation.
This re-thinks the documentation by focusing on the schema, much like
`manifest.md` does.
Do not add home bin path to PATH if it's already there
This is to allow users to control the order via PATH if they so desire.
Tested by preparing two different `cargo-foo` scripts in `$HOME/.cargo/bin` and `$HOME/bin`:
```
> env PATH="/usr/bin/:$HOME/bin:$HOME/.cargo/bin" ./target/debug/cargo foo
Inside ~/bin/
> env PATH="$HOME/.cargo/bin:/usr/bin/:$HOME/bin" ./target/debug/cargo foo
Inside ~/.cargo/bin/
> env PATH="/usr/bin/:$HOME/bin" ./target/debug/cargo foo
Inside ~/.cargo/bin/
```
and trailing slash:
```
> env PATH="$HOME/.cargo/bin/:/usr/bin/:$HOME/bin" ./target/debug/cargo foo
Inside ~/.cargo/bin/
> env PATH="/usr/bin/:$HOME/bin:$HOME/.cargo/bin/" ./target/debug/cargo foo
Inside ~/bin/
```
Fix https://github.com/rust-lang/cargo/issues/11020
Don't use `for` loop on an `Option`
This PR removes a single `for` loop over `Option`, replacing it with an `if let` to improve code clarity. This currently blocks https://github.com/rust-lang/rust/pull/99696 that adds a lint against this pattern.
