Auto merge of #5231 - matklad:simplify, r=alexcrichton

Simplify `build_map`

This doesn't change semantics, but helps us not to rebuild BuildScripts
twice, and makes sure that we fail with assert if somehow we get cyclic
deps.

r? @alexcrichton

Simplify `build_map`

This doesn't change semantics, but helps us not to rebuild BuildScripts
twice, and makes sure that we fail with assert if somehow we get cyclic
deps

Auto merge of #5230 - matthiaskrgr:newline, r=alexcrichton

add final linebreak to cargo default help message.

before:
patch:

I assume this is some kind of leftover from the clap transition. cc @matklad

add final linebreak to cargo default help message.

Auto merge of #5226 - jeehoonkang:doc-caret, r=alexcrichton

doc: Clarify the meaning of caret requirements

Closes #4910 (which is already closed, though 😄)

r? @alexcrichton

doc: Clarify the meaning of caret requirements

Closes #4910

Auto merge of #5225 - matthiaskrgr:revert_pr5223, r=alexcrichton

Revert "manifest: update default lto for release and bench to 'thin'."

This reverts commit e74ff4db13f2ae00245902cb5b496b3b39128653.

Turns out I was wrong, sorry for the noise.

Revert "manifest: update default lto for release and bench to 'thin'."

This reverts commit e74ff4db13f2ae00245902cb5b496b3b39128653.

Turns out I was wrong, sorry for the noise.

Auto merge of #5217 - matklad:known-crate-types, r=alexcrichton

Preprobe info for known crate type

Previously, we've calculated the set of crate types to learn about by
recursively walking the graph of units. However, to actually know
dependencies of a unit exactly, we must know target specific info, and
we don't know it at this moment (in fact, we are trying calculating it).

Note that crate-type calculation is already lazy, we don't have to calc
all crate-types upfront. So, let's just scrape this info once for
well-known crate types, and fill whatever is left lazily.

@alexcrichton would this approach work at all? I think it would, if `KNOWN_CRATE_TYPES` are all available for all target-tripples we support. Is it a valid assumption?

The larger picture is that I am trying to make unit dependency resolution eager and move it into the separate file. I even got something working, but I have to run dependency resolution three times, because it is not exactly idempotent for various reasons, including this target-info stuff :)

```
    cx.prepare()?;
    cx.build_unit_map(units.clone())?; // resolve dependencies
    cx.probe_target_info(&units)?;
    cx.build_unit_map(units.clone())?; // resolve again
    cx.build_used_in_plugin_map(&units)?;
    custom_build::build_map(&mut cx, &units)?;
    cx.build_unit_map(units.clone())?; // and resolve one final time :)
```

Auto merge of #5223 - matthiaskrgr:manifest_update, r=matklad

manifest: update default lto for release and bench to "thin"

[redacted]

manifest: update default lto for release and bench to 'thin'.

Auto merge of #5222 - AdamNiederer:patch-1, r=matklad

Add docstrings to CleanOptions

Add docstrings to CleanOptions

Auto merge of #5219 - mbrubeck:doc, r=alexcrichton

Fix usage text for cargo install

The current text was accidentally copied from `cargo init`.

Fix usage text for cargo install

The current text was accidentally copied from `cargo init`.

Auto merge of #5186 - infinity0:stricter-need-dev-deps, r=alexcrichton

Stricter need_dev_deps behaviour

The previous PR (#5012) contained an unnecessary work-around for behaviour of `--all-targets` that was misunderstood. This PR removes that work-around and adds some tests and comments to clarify the behaviour for future contributors, which may help to make easier a future fix for #5177 and #5178.

Auto merge of #5204 - lukaslueg:issue5199, r=alexcrichton

Do not allow crate-type or proc-macro for [[bin]]-targets

Fixes #5199

This simply disallows `proc-macro` and `crate-type` to be set to anything for binary targets. Is this the best way to go or does a warning about the unused setting suffice?

Preprobe info for known crate type

Previously, we've calculated the set of crate types to learn about by
recursively walking the graph of units. However, to actually know
dependencies of a unit exactly, we must know target specific info, and
we don't know it at this moment (in fact, we are trying calculating it).

Note that crate-type calculation is already lazy, we don't have to calc
all crate-types upfront. So, let's just scrape this info once for
well-known crate types, and fill whatever is left lazily.

Auto merge of #5215 - alexcrichton:update-version, r=matklad

Don't require `cargo update` when bumping versions

One historical annoyance I've always had with Cargo that I've found surprising
is that in some situations when you bump version numbers you'll have to end up
running `cargo update` later on to get everything to build. You get pretty wonky
error messages in this case as well saying a package doesn't exist when it
clearly does at a particular location!

I've had difficulty historically nailing down a test case for this but it looks
like we ironically already had one in our test suite and I also jury-rigged up
one from a case I ran into in the wild today.

Don't require `cargo update` when bumping versions

One historical annoyance I've always had with Cargo that I've found surprising
is that in some situations when you bump version numbers you'll have to end up
running `cargo update` later on to get everything to build. You get pretty wonky
error messages in this case as well saying a package doesn't exist when it
clearly does at a particular location!

I've had difficulty historically nailing down a test case for this but it looks
like we ironically already had one in our test suite and I also jury-rigged up
one from a case I ran into in the wild today.

Do not allow crate-type or proc-macro for [[bin]]-targets
in the current workspace.

Fixes #5199.

Auto merge of #5206 - lukaslueg:patch-1, r=matklad

Update CONTRIBUTING.md

Since a13a33c33b049a7b412f0cc80a7b166b0d58345c code is rejected during CI if `rustfmt` complains. Update the docs regarding that fact.

Auto merge of #5209 - matklad:no-way-we-havent-had-a-test-for-this, r=alexcrichton

Fix regression when passing arguments to subcommands

closes https://github.com/rust-lang/cargo/issues/5208

`.args(&args[1..])` was copied directly from the docopt implementation, but there, `args[0]` was the path to `cargo` and not the name of subcommand, ie, `args` were *original* arguments for Cargo as a whole.

Fix regression when passing arguments to subcommands

Auto merge of #5202 - matklad:paths, r=alexcrichton

Extract common code for dealing with path-valued arguments

Just a small refactoring to make sure all path arguments are treated in the same way.

Auto merge of #5195 - alexcrichton:resolver-docs, r=Eh2406

Add some documentation to the resolver

I spent a few hours yesterday re-teaching myself the resolver and wanted to add some comments along the way!

Auto merge of #5207 - matklad:clap-test, r=matklad

Regression tests for #5201

Better safe than sorry!

Add some documentation to the resolver

This is currently my best-effort attempt to document various portions of the
resolver with the logic that's been added recently. It at least helped me
understand a bit what was going on so I hope it can help others as well!

Regression tests for #5201

Better safe than sorry!

Extract common code for dealing with path-valued arguments

Update CONTRIBUTING.md

Since a13a33c33b049a7b412f0cc80a7b166b0d58345c code is rejected during CI if `rustfmt` complains. Update the docs regarding that fact.

Split tests, apparently `cargo clean` does not work well on windows

Auto merge of #5187 - Eh2406:faster_resolver, r=alexcrichton

Faster resolver: clean code and the `backtrack_stack`

This is a small extension to #5168 and is inspired by https://github.com/rust-lang/cargo/pull/4834#issuecomment-363518370

After #5168 these work (and don't on cargo from nightly.):
- `safe_core = "=0.22.4"`
- `safe_vault = "=0.13.2"`

But these don't work (and do on cargo from this PR.)
- `crust = "=0.24.0"`
- `elastic = "=0.3.0"`
- `elastic = "=0.4.0"`
- `elastic = "=0.5.0"`
- `safe_vault = "=0.14.0"`

It took some work to figure out why they are not working, and make a test case.

This PR remove use of `conflicting_activations` before it is extended with the conflicting from next.
https://github.com/rust-lang/cargo/pull/5187#issuecomment-373830919
However the `find_candidate(` is still needed so it now gets the conflicting from next before being called.

It often happens that the candidate whose child will fail leading to it's failure, will have older siblings that have already set up `backtrack_frame`s. The candidate knows that it's failure can not be saved by its siblings, but sometimes we activate the child anyway for the error messages. Unfortunately the child does not know that is uncles can't save it, so it backtracks to one of them. Leading to a combinatorial loop.

The solution is to clear the `backtrack_stack` if we are activating just for the error messages.

Edit original end of this message, no longer accurate.
#5168 means that when we find a permanent problem we will never **activate** its parent again. In practise there afften is a lot of work and `backtrack_frame`s between the problem and reactivating its parent. This PR removes `backtrack_frame`s where its parent and the problem are present. This means that when we find a permanent problem we will never **backtrack** into it again.

An alternative is to scan all cashed problems while backtracking, but this seemed more efficient.

When activating for the better error messages don't waste time on the other backtrack frames

When test backtracking include conflicts in `remaining_candidates`

Auto merge of #5197 - matklad:neater-tests, r=alexcrichton

Slightly improve ergonomics of writing Cargo tests

As discussed in https://github.com/rust-lang/cargo/pull/5196#discussion_r175110557

I've also employed this to some of the longer command-lines in our test suite :)

Slightly improve ergonomics of writing Cargo tests

Revert "Clean the `backtrack_stack` so we don't backtrack to a place with cashed bad activations"

remove duplicated adding to the cache

Auto merge of #5192 - matklad:hash-set-all-the-way-down, r=alexcrichton

Avoid intermediate vector

Just a microoptimization to avoid intermediate `Vec`

Avoid intermediate vector

Auto merge of #5196 - matklad:clapclapclap, r=alexcrichton

Fix a regression with parsing multivalue options

By default, clap interprets

```
cargo run --bin foo bar baz
```

as

```
cargo run --bin foo --bin bar --bin baz
```

This behavior is different from docopt and does not play nicely with
positional arguments at all. Luckily, clap has a flag to get the
behavior we want, it just not the default! It will become the default in
the next version of clap, but, until that time, we should be careful
when using the combination of `.long`, `.value_name` and
`.multiple(true)`, and don't forget to specify `.number_of_values(1)` as
well.

@alexcrichton I'd love to merge this fix before updating cargo at rust-lang/rust :)

Fix a regression with parsing multivalue options

By default, clap interprets

```
cargo run --bin foo bar baz
```

as

```
cargo run --bin foo --bin bar --bin baz
```

This behavior is different from docopt and does not play nicely with
positional arguments at all. Luckily, clap has a flag to get the
behavior we want, it just not the default! It will become the default in
the next version of clap, but, until that time, we should be careful
when using the combination of `.long`, `.value_name` and
`.multiple(true)`, and don't forget to specify `.number_of_values(1)` as
well.

Auto merge of #5193 - matklad:deny-warnings, r=alexcrichton

Deny warnings in tests

Deny warnings in tests

suggestions

Auto merge of #5190 - mbrubeck:doc, r=alexcrichton

Remove old note about pre-1.21 versions

None

Remove old note about pre-1.21 versions

Auto merge of #5188 - alexcrichton:urgh-again, r=matklad

Add a synthetic dependency on num-traits

Right now the rust-lang/rust integration is compiling Cargo twice on dist
builds, once for Cargo and once for the RLS. This is due to a dependency of
Cargo being recompiled with different features when used from the RLS or not.
For now paper over this problem with a synthetic dependency to prevent Cargo
from being compiled twice.

cargo +stable fmt

Add a synthetic dependency on num-traits

Right now the rust-lang/rust integration is compiling Cargo twice on dist
builds, once for Cargo and once for the RLS. This is due to a dependency of
Cargo being recompiled with different features when used from the RLS or not.
For now paper over this problem with a synthetic dependency to prevent Cargo
from being compiled twice.

add a test

Rename stuff for clarity

- generate_{auto => default}_target since it matches on CompileFilter::Default
- CompileFilter::{matches => target_run} to make it clear it only affects `cargo run`
- Add a comment pointing to generate_target for other subcommands

Add tests for --all-targets

Fix need_dev_deps to return false during a default `cargo build` run

- Also add a mode param to need_dev_deps to make clear the intent

Revert "Work around #5134 for now"

This reverts commit d46db71b3ff17dfc0f4be6308c8b94613d65a572.

Fix a bug in #5152 that causes rustc/rustdoc to fail unnecessarily

Auto merge of #5180 - alexcrichton:transitive-update, r=matklad

Don't abort resolution on transitive updates

This commit is directed at fixing #4127, allowing the resolver to automatically
perform transitive updates when required. A few use casese and tagged links are
hanging off #4127 itself, but the crux of the issue happens when you either add
a dependency or update a version requirement in `Cargo.toml` which conflicts
with something listed in your `Cargo.lock`. In this case Cargo would previously
provide an obscure "cannot resolve" error whereas this commit updates Cargo to
automatically perform a conservative re-resolution of the dependency graph.

It's hoped that this commit will help reduce the number of "unresolvable"
dependency graphs we've seen in the wild and otherwise make Cargo a little more
ergonomic to use as well. More details can be found in the source's comments!

Closes #4127

Clean the `backtrack_stack` so we don't backtrack to a place with cashed bad activations

Don't abort resolution on transitive updates

This commit is directed at fixing #4127, allowing the resolver to automatically
perform transitive updates when required. A few use casese and tagged links are
hanging off #4127 itself, but the crux of the issue happens when you either add
a dependency or update a version requirement in `Cargo.toml` which conflicts
with something listed in your `Cargo.lock`. In this case Cargo would previously
provide an obscure "cannot resolve" error whereas this commit updates Cargo to
automatically perform a conservative re-resolution of the dependency graph.

It's hoped that this commit will help reduce the number of "unresolvable"
dependency graphs we've seen in the wild and otherwise make Cargo a little more
ergonomic to use as well. More details can be found in the source's comments!

Closes #4127
Closes #5182

Remove `Source::for_path`

This commit removes the `Source::for_path` constructor in favor of
`Workspace::load`. This prevents re-parsing manifests multiple times as Cargo
loads up as this can sometimes be an expensive operation. Instead the
`Workspace` now retains a cache of packages that can be updated as it goes
along. Finally, this should mean that we're only parsing path dependencies at
most once rather than multiple times.

Auto merge of #5185 - matklad:printf-debugging-for-life, r=alexcrichton

Add a helper method to show stdout/err in tests

So, all this time debugging Cargo was rather annoying to me, because added `println!`s tend not to show up in the test output, because they happen in the child process. So, let's add a method which just streams the subordinate process output?

@alexcrichton I wonder how you usually debug Cargo tests? Or ... are you just not making bugs to begin with? :)

Auto merge of #5184 - matklad:delegate-debug, r=Eh2406

Delegate debug implementation for InternedString

Let's make `InternedString` debug implementation the same as for `String` / `str`? It's more concise, which helps when you debug printing other stuff, like `eprintln!("id = {:?}", a_package_id)`.

Add a helper method to show stdout/err in tests

Delegate debug implementation for InternedString

Auto merge of #5176 - alexcrichton:rustfmt, r=alexcrichton

Add `cargo fmt` to CI and delete `rustfmt.toml`

This commit adds CI to run `cargo fmt` over Cargo itself as well as the internal
`crates-io` crate. This should switch Cargo to the "default style" (aka whatever
rustfmt spits out) and ensure that we keep it that way via CI. Hopefully this
won't be too much of a bother to keep up and running in CI as it should just be
a `cargo fmt` away!

Pin to an older nightly temporarily

Needs a workaround until rust-lang/rust#49033 is fixed

Prettify rustfmted single-line strings

cargo fmt

Add `cargo fmt` to CI and delete `rustfmt.toml`

This commit adds CI to run `cargo fmt` over Cargo itself as well as the internal
`crates-io` crate. This should switch Cargo to the "default style" (aka whatever
rustfmt spits out) and ensure that we keep it that way via CI. Hopefully this
won't be too much of a bother to keep up and running in CI as it should just be
a `cargo fmt` away!

Auto merge of #5181 - alexcrichton:selectively-write-dep-info, r=matklad

Don't rewrite dep-info files if they don't change

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes #5172

Auto merge of #5179 - matklad:uplink-pdb, r=alexcrichton

Copy `.pdb` files to `target` directory

`.pdb` files are for windows debug info (unlike on linux, debug info is
in a separate file). Windows executable actually hard-code paths to
`.pdb` files, so debugging mvsc rust programs works even without this
patch. However, if you want to distribute the executable to other
machines, you'd better distribute both `foo.exe` and `foo.pdb`, because
absolute paths won't work on another machine. Having same-named .pdb
file alongside the binary would work though.

closes #4960

Don't check for filenames in message-format tests

Depending on the OS, there might be an additional artifacts for
debuginfo (dSYM folder for macs, .pbd file for windows). Given that we
can't disable `.pdb` for windows[1], let's just ignore all filenames!

[1]: https://github.com/rust-lang/rust/pull/28505)

Don't rewrite dep-info files if they don't change

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes #5172

Copy `.pdb` files to `target` directory

`.pdb` files are for windows debug info (unlike on linux, debug info is
in a separate file). Windows executable actually hard-code paths to
`.pdb` files, so debugging mvsc rust programs works even without this
patch. However, if you want to distribute the executable to other
machines, you'd better distribute both `foo.exe` and `foo.pdb`, because
absolute paths won't work on another machine. Having same-named .pdb
file alongside the binary would work though.

closes #4960

Auto merge of #5175 - matklad:conv, r=alexcrichton

Use conventions to specify the integration tests

Auto merge of #5171 - rtsuk:gn-compatible-depfiles, r=alexcrichton

Gn compatible depfiles

Fixes https://github.com/rust-lang/cargo/issues/5162.

Auto merge of #5168 - Eh2406:faster_resolver, r=alexcrichton

Faster resolver: Cache past conflicting_activations, prevent doing the same work repeatedly.

This work is inspired by @alexcrichton's [comment](https://github.com/rust-lang/cargo/issues/4066#issuecomment-303912744) that a slow resolver can be caused by all versions of a dependency being yanked. Witch stuck in my brain as I did not understand why it would happen. If a dependency has no candidates then it will be the most constrained and will trigger backtracking in the next tick. Eventually I found a reproducible test case. If the bad dependency is deep in the tree of dependencies then we activate and backtrack `O(versions^depth)`  times. Even tho it is fast to identify the problem that is a lot of work.

**The set up:**
1. Every time we backtrack cache the (dep, `conflicting_activations`).
2. Build on the work in #5000, Fail to activate if any of its dependencies will just backtrack to this frame. I.E. for each dependency check if any of its cached `conflicting_activations` are already all activated. If so we can just skip to the next candidate. We also add that bad `conflicting_activations` to our set of  `conflicting_activations`, so that we can...

**The pay off:**
 If we fail to find any candidates that we can activate in lite of 2, then we cannot be activated in this context, add our (dep, `conflicting_activations`) to the cache so that next time our parent will not bother trying us.

I hear you saying "but the error messages, what about the error messages?" So if we are at the end `!has_another` then we disable this optimization. After we mark our dep as being not activatable then we activate anyway. It won't resolve but it will have the same error message as before this PR. If we have been activated for the error messages then skip straight to the last candidate, as that is the only backtrack that will end with the user.

I added a test in the vain of #4834. With the old code the time to run was `O(BRANCHING_FACTOR ^ DEPTH)` and took ~3min with DEPTH = 10; BRANCHING_FACTOR = 5; with the new code it runs almost instantly with 200 and 100.

Use get_path() instead of get_string()

Use conventions to specify the integration tests

more comments

Add documentation

Put this config in the build section

Return errors instead of ignoring them

More idiomatic

Only activate for the better error messages if we will bactrack to the user

test that cashing works with constraints correctly

Auto merge of #5152 - matklad:clap, r=alexcrichton

Clap

Reopening of #5129

So, looks like all tests are :green_apple: on my machine!

I definitely want to refactor it some more, and also manually checked that we haven't regressed any help messages, but all the major parts are in place already.

Optionally write dependencies targets relative

If dep-info-basedir is present in the config, write out dependency
targets as relative to that directory.

make sure cashing works whether there is an answer or not

only jump to the end if our frame was made for the error message

File per command

if a frame will fail make it top priority

Use `avoid_dev_deps` for all commands

Fix spelling of quiet

Auto merge of #5167 - xftroxgpx:nosubmodules, r=matklad

Closes #5165

#5165

if we are just here_for the error_messages then only try the last candidate

cache past conflicting_activations use them to prevent doing the same work repeatedly

reduce the mut args to find_candidate

this test is slow because it runs the same searches repeatedly