I've wanted something like this myself. I dislike using `--open`
because I tend to move up to re-run my `cargo doc` run but then have to
edit it to remove `--open`.
Also makes it annoying when opening docs when `cargo doc` is wrapped by
a tool like `make`.
This was previously attempted in #5592:
- Unlike the request in #5562, this aligns with #5592 in always printing
rather than using a flag as this seems generally useful
- Unlike #5592, this prints as an alternative to "Opening" to keep
things light
- Unlike #5592, this prints afterwards as the link is only valid then
Fixes#5562
fix(remove): Preserve feature comments
### What does this PR try to resolve?
We've been having a hard time balancing leaving the feature list in a good looking start and preserving formatting. With our new formatting policy (#12836), we can just choose to preserve formatting instead.
Fixes#11743
### How should we test and review this PR?
The first commit copies an existing test. The second is where the fun begins, customizing the test for some weird cases. The follow up commits do the slow walk for improving it.
We ended up preserving some line-trailing comments because they come after the comma and toml_edit treats that as part of the prefix of the next item. Tracking removal of that was going to require us to determine if the newline existed in the suffix or in the next item's prefix and edit accordingly and I decided to skip that to keep this initial implementation simpler.
### Additional information
fix(add): Preserve more comments
### What does this PR try to resolve?
This was fixed in killercup/cargo-edit#725 and I forgot to carry it over to here.
I kept in some auto-formatting because there is little to preserve until the TOML 1.1 spec is out which will allow mult-line inline-tables which means there might be comments interspersed. We'll see which comes first, `cargo fmt` support for `Cargo.toml` or the 1.1 spec.
Fixes#10850
### How should we test and review this PR?
First commit adds a test demonstrating the problem. The second makes a lot of noise that the third cleans up, so its a mixed bag as to which commit to look at.
### Additional information
docs: remove review capacity notice
The Cargo project is in a better state than it was when maintainers added this notice. I believe it's time to take it down, as maintainers have got more time on it, and more people are willing to contribute to the project constantly.
Note that this doesn't mean Cargo start accepting arbitrary pull requests or features. The reviewers are still a small handful of people. The guideline never change — issue, discuss, then pull request.
fix(help):Clarify install's positional
### What does this PR try to resolve?
- That a version is accepted
- That you are selecting from the source a package which led to part of
the confusion in #4830
I wonder if we should rename our `CRATE` value names to `PKG`/`PACKAGE`
While doing this, I decided to fix the inconsistency in how we handle value names in help.
### How should we test and review this PR?
### Additional information
Adjust `-Zcheck-cfg` for new rustc syntax and behavior
https://github.com/rust-lang/rust/pull/111072 introduced a new syntax for `rustc` `--check-cfg` argument. This PR adjust cargo `-Zcheck-cfg` for new that new syntax and behavior.
This PR removes all the `-Zcheck-cfg` options (`features`, `names`, `values`, `output`), as they don't make much sense now since with the new `rustc` behavior: `features`, `names` and `values` are all combine together and the `output` option was only here because the other were.
Now the new behavior from cargo is to always pass one `--check-cfg` argument to rustc for the `feature`s which implicitly enables well known names and values.
fix(replace): Partial-version spec support
### What does this PR try to resolve?
#12614 changed package ID specs to allow fields in the version number to be optional. This earliest branch with this change is `rust-1.74.0` (beta). While `@Eh2406` was investigating version metadata issues in #12772, problems with the partial version change were found
- `replace`s that specify version metadata were ignored **(fixed with this PR)**
- This also extends out to any other place a PackageIDSpec may show up, like `cargo check -p <name>`@<spec>``
- We explicitly kept the same semantics of version requirements that pre-releases require opt-in. If nothing else, this gives us more room to change semantics in the future if we ever fix the semantics for pre-release.
- `replace`s that don't specify version metadata when the `Cargo.lock` contained a version metadata, it would previously be ignored (with a warning) but now match **(unchanged with this PR)**
- When the version metadata in `Cargo.lock` differed from the overriding `Cargo.toml`, cargo would panic **(now an error in this PR)**
With this PR, we are acknowledging that we changed behavior in taking ignored replaces (because of differences with version metadata) and applying them. Seeing as version metadata is relatively rare, replaces are relatively rare, and differences in it for registries is unsupported, the impact seems very small.
The questions before us are
- Do we revert #12614 in `master` and `rust-1.74.0` or merge this PR into `master`
- If we merge this PR into `master`, do we cherry-pick this into `rust-1.74.0` or revert #12614, giving ourselves more time to find problems
### How should we test and review this PR?
The initial commit adds tests that pass as of #12614. Prior to #12614, these tests would have warned that the `replace` was unused and failed because `bar::bar` didn't exist. Each commit then changes the behavior (or not) and updates the corresponding test.
### Additional information
This commit removes all the -Zcheck-cfg options (features, names,
values, output), as they don't make much sense now since features, names
and values are all combine together for upstream rustc and the output
option was only here because the other were.
Now the new behavior from cargo is to always pass one `--check-cfg`
argument to rustc for the `feature`s which implicitly enables well known
names and values.
Print environment variables for build script executions with `-vv`
### What does this PR try to resolve?
When debugging complicated builds (I was trying to figure out how `cargo-miri` cross-compiles compiler_builtins without needing a C cross compiler), it's useful to see all the environment variables passed to the build script.
This is also consistent with other commands.
### How should we test and review this PR?
I tested it locally by creating a small crate with an empty `build.rs` and building it. Additionally, a test is included.
fix(cli): Suggest cargo-search on bad commands
This is a low-tech solution alternative to the options proposed in #4682
- Search `[[bin]]`s within all packages in the registry (which aren't tracked atm), suggesting to `cargo install` what is found
- Check if `cargo-<cmd>` is in the registry, suggesting `cargo install if it is
By suggesting `cargo search`, we are giving them a tool so they can verify if the package is what they want (a `cargo info` would help as a next step).
Is is needed?
- New users might not know of `cargo search` but they can search on crates.io
- New users might not be aware of the `cargo-<cmd>` naming pattern
Seems like this can still offer some benefit.
Fixes#4682
- That a version is accepted
- That you are selecting from the source a package which led to part of
the confusion in #4830
I wonder if we should rename our `CRATE` value names to `PKG`/`PACKAGE`
This is a low-tech solution alternative to the options proposed in #4682
- Search `[[bin]]`s within all packages in the registry (which aren't
tracked atm), suggesting to `cargo install` what is found
- Check if `cargo-<cmd>` is in the registry, suggesting `cargo install
if it is
By suggesting `cargo search`, we are giving them a tool so they can
verify if the package is what they want (a `cargo info` would help as a
next step).
Is is needed?
- New users might not know of `cargo search` but they can search on
crates.io
- New users might not be aware of the `cargo-<cmd>` naming pattern
Seems like this can still offer some benefit
Fixes#4682
ci/contrib: use separate concurrency group
Run the contrib workflow in its own concurrency group to avoid multiple runs causing conflicting git pushes.
The contrib workflow pushes to the gh-pages branch, and thus is not safe to be run multiple times in parallel. Given that github-actions can stall workflow runs for quite some time, multiple executions can overtake each other. By using concurrency groups, all workflow runs will wait until previous runs completed.
Given that the workflow uses orphan-branches and forced pushes, conflicting workflows will not fail, but might produce outdated data if they overtake each other.
We explicitly disable cancellation to get better diagnostics and more easily find the first possible run that failed. Since these workflow runs are rather fast, and only run on master, cancellation seems not necessary.
ci/contrib: do not fail on missing gh-pages
The current contrib deploy-hook fails if there is no `gh-pages` branch. Change the CI order to disregard the old `gh-pages` branch first.
The `contrib` deploy-hook always creates a fresh `gh-pages` commit and pushes it out. However, currently it relies on the old `gh-pages` branch to exist, since it does not ignore errors when pruning it. Fortunately, the code always creates a new orphan branch, since it does not want to keep history for deployments. Therefore, we can simply use:
`git worktree --orphan -B <branch> <path>`
This will ensure to always create an orphan branch named `<branch>`, and override an existing branch if it exists (see `-b` vs `-B`). Hence, there is no need for us to prune the old branch, anymore.
Since we will recreate the branch on every push, we have to explicitly specify the remote to push to. We no longer set up branch tracking.
Note that running github-actions in a private fork is quite useful to avoid constantly pushing changes to a PR and triggering notifications. Unfortunately, private forks on github do not automatically clone the gh-pages branch. This PR fixes annoying CI failures when updating the master branch on private forks, since github does not automatically clone the gh-pages branch.
Run the contrib workflow in its own concurrency group to avoid multiple
runs causing conflicting git pushes.
The contrib workflow pushes to the gh-pages branch, and thus is not safe
to be run multiple times in parallel. Given that github-actions can
stall workflow runs for quite some time, multiple executions can
overtake each other. By using concurrency groups, all workflow runs will
wait until previous runs completed.
We explicitly disable cancellation to get better diagnostics and more
easily find the first possible run that failed. Since these workflow
runs are rather fast, and only run on master, cancellation seems not
necessary.
Clarify flag behavior in `cargo remove --help`
### What does this PR try to resolve?
I noticed what I believe are typos in `cargo rm --help`:
```
Section:
--dev Remove as development dependency
--build Remove as build dependency
--target <TARGET> Remove as dependency from the given target platform
```
This change updates that section with a more appropriate description of those flags.
### How should we test and review this PR?
I've updated the relevant test for that help output.
### Additional information
Sorry for not opening an issue about this. I believe it's easy enough to approve it if my assumption is correct.
The current contrib deploy-hook fails if there is no `gh-pages` branch.
Change the CI order to disregard the old `gh-pages` branch first.
The `contrib` deploy-hook always creates a fresh `gh-pages` commit and
pushes it out. However, currently it relies on the old `gh-pages` branch
to exist, since it does not ignore errors when pruning it. Fortunately,
the code always creates a new orphan branch, since it does not want to
keep history for deployments. Therefore, we can simply use:
`git worktree --orphan -B <branch> <path>`
This will ensure to always create an orphan branch named `<branch>`, and
override an existing branch if it exists (see `-b` vs `-B`). Hence,
there is no need for us to prune the old branch, anymore.
Since we will recreate the branch on every push, we have to explicitly
specify the remote to push to. We no longer set up branch tracking.