fix(lint): Warn not Error on unsupported lint tool
In a recent Cargo Team meeting, it was decided to lessen the error on an unsupported tool in `[lints]` to a warning. This PR implements that change.
refactor: Move diagnostic printing to Shell
As I have been working on cargo's diagnostic system, I have disliked copying around the code for a new `Renderer`, and then emitting the diagnostic:
```rust
let renderer = Renderer::styled().term_width(
gctx.shell()
.err_width()
.diagnostic_terminal_width()
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
);
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
```
Moving to a method on `Shell` helps mitigate the risk of updating duplicate code and missing one. It should also make it nearly impossible to get into scenarios where `gctx.shell()` is borrowed twice, leading to panics. This problem was one I ran into early on as I tried to write messages to `stderr` like so:
```rust
writeln!(
gctx.shell().err(),
"{}",
Renderer::styled()
.term_width(
gctx.shell()
.err_width()
.diagnostic_terminal_width()
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
)
.render(message)
)?;
```
Populate git information when building Cargo from Rust's source tarball
### What does this PR try to resolve?
Right now Cargo doesn't populate the commit hash or date in its version output when it's built from Rust's plain source tarball (like `rustc-1.77.2-src.tar.xz`). That's because Cargo's build script only looks for information in the `.git` directory, which is missing from that tarball.
I opened https://github.com/rust-lang/rust/pull/124553 to have bootstrap inject a `git-commit-info` file in `src/tools/cargo` when building the plain source tarball, containing the correct git information. This is the approach also used by the compiler.
This PR updates the build script to read the information from that file if there is no `.git` and the file is present.
### How should we test and review this PR?
To test the PR you need to move the `.git` directory somewhere else and create a `git-commit-info` file like this:
```
25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04
25ef9e3d8
2024-04-09
```
Then clearing the build cache and running `cargo run -- -vV` should show the git information in the `git-commit-info` file.
### Additional information
This PR can be merged independently from https://github.com/rust-lang/rust/pull/124553
Starting with this commit we deduplicate calls to rm_rf_prefix_list by crate name and not by directory; this can lead to more calls to rm_rf_prefix_list (especially in presence of multiple -p arguments),
but it is also more transparent in terms of progress reporting (we're just storing away whether a given directory + glob pair has already been removed)
fix(toml): Improve granularity of traces
### What does this PR try to resolve?
Once we move resolving out of `to_targets`, we won't have tracing visibility. As we don't have a top-level function for resolving, I put it on each. I mirrored this for "to_" for consistency and it seems useful when looking at the output.
### How should we test and review this PR?
### Additional information
fix(toml): Warn, rather than fail publish, if a target is excluded
### What does this PR try to resolve?
We have a couple of problems with publishing
- Inconsistent errors: if a target that `package` doesn't verify is missing `path`, it will error, while one with `path` won't, see #13456
- Users may want to exclude targets and their choices are
- Go ahead and include them. I originally excluded my examples before doc-scraping was a think. The problem was if I had to set `required-features`, I then could no longer exclude them
- Muck with `Cargo.toml` during publish and pass `--allow-dirty`
This fixes both by auto-stripping targets on publish. We will warn the user that we did so.
This is a mostly-one-way door on behavior because we are turning an error case into a warning.
For the most part, I think this is the right thing to do.
My biggest regret is that the warning is only during `package`/`publish` as it will be too late to act on it and people who want to know will want to know when the problem is introduced.
The error is also very late in the process but at least its before a non-reversible action has been taken.
Dry-run and `yank` help.
Fixes#13456Fixes#5806
### How should we test and review this PR?
Tests are added in the first commit and you can then follow the commits to see how the test output evolved.
The biggest risk factors for this change are
- If the target-stripping logic mis-identifies a path as excluded because of innocuous path differences (e.g. case)
- Setting a minimum MSRV for published packages: `auto*` were added in 1.27 (#5335) but were insta-stable. `autobins = false` did nothing until 1.32 (#6329). I have not checked to see how this behaves pre-1.32 or pre-1.27. Since my memory of that error is vague, I believe it will either do a redundant discovery *or* it will implicitly skip discovery
Resolved risks
- #13729 ensured our generated target paths don't have `\` in them
- #13729 ensures the paths are normalize so the list of packaged paths
For case-insensitive filesystems, I added tests to show the original behavior (works locally but will fail when depended on from a case-sensitive filesystem) and tracked how that changed with this PR (on publish warn that those targets are stripped). We could try to normalize the case but it will also follow symlinks and is likely indicative of larger casing problems that the user had. Weighing how broken things are now , it didn't seem changing behavior on this would be too big of a deal.
We should do a Call for Testing when this hits nightly to have people to `cargo package` and look for targets exclusion warnings that don't make sense.
### Additional information
This builds on #13701 and the work before it.
By enumerating all targets in `Cargo.toml`, it makes it so rust-lang/crates.io#5882 and rust-lang/crates.io#814 can be implemented without any other filesystem interactions.
A follow up PR is need to make much of a difference in performance because we unconditionally walk the file system just in case `autodiscover != Some(false)` or a target is missing a `path`.
We cannot turn off auto-discovery of libs, so that will always be done for bin-only packages.
This could offer performance gains when parsing a published
manifest since the targets don't need to be discovered.
To see this, we'd first need to stop discovering potential targets even when it isn't
needed.
fix(toml)!: Remove support for inheriting badges
### What does this PR try to resolve?
We allowed `[badges]` to inherit from `[workspace.package.badges]` which was a bug:
- This was not specified in the RFC
- We did not document this
- Even if someone were to try to guess to use this, it is inconsistent with how inheritance works because this should inherit from `workspace.badges` instead of `workspace.package.badges`
While keeping in mind that `[badges]` is effectively deprecated.
In that context, I think its safe to break support for this without a transition period.
Fixes#13643
### How should we test and review this PR?
### Additional information
chore(ci): Don't check `cargo` against beta channel
We already only partially check it and it has been a source of false positives.
While there is trust in the job, contributors and maintainers go into the job assuming there is a problem and it takes time to break that assumption.
If we lose trust in the job, we then won't trust it when it fails for other reasons.
This also increases the risk of us not seeing other problems `bump-checks` is meant to find if the steps in the job get re-arranged to make this one of the early ones.
We already only partially check it and it has been a source of false
positives.
While there is trust in the job, contributors and maintainers go into
the job assuming there is a problem and it takes time to break that
assumption.
If we lose trust in the job, we then won't trust it when it fails for
other reasons.
This also increases the risk of us not seeing other problems
`bump-checks` is meant to find if the steps in the job get re-arranged
to make this one of the early ones.
