Change verification order during packaging.
Once we support packaging workspaces with dependencies, dependency packages need to be built before anything is verified. In addition to a little refactoring, this commit reorders the console messages so that package metadata (archive size, etc.) is reported before verification results.
As [suggested](ecba15ca99 (r1640182916)) on #13947 this, splits out the first commit (which has a lot of test output churn) as a separate PR.
Once we support packaging workspaces with dependencies, dependency
packages need to be built before anything is verified. In addition to a
little refactoring, this commit reorders the console messages so that
package metadata (archive size, etc.) is reported before verification
results.
Co-Authored-By: Tor Hovland <55164+torhovland@users.noreply.github.com>
I got a CI failure because the following line showed up:
```
[WARNING] build failed, waiting for other jobs to finish...
```
I'm assumin this is a race condition in the test for whether the
successful target completed before the error or not.
Before snapbox, we used a `contains` check which didn't have this
problem. I'm replacing this with a `...` multi-line (0+) glob.
Migrate a few test files to snapbox
This migrates the following files to `snapbox`
- `artifact_dep`
- Has a few `does_not_contain`
- `artifact_dir`
- `bad_config`
- `bad_manifest_path`
- Does not use `str!` for all tests
- `bench`
Note: This also adds auto-redactions for:
- `[HOST_TARGET]`
- `[ALT_TARGET]`
- Only added if cross-compilation is allowed for the target
- `[AVG_ELAPSED]`
- For `bench` output
- `[JITTER]`
- For `bench` output
Part of #14039
test: migrate features_are_quoted to snapbox
### What does this PR try to resolve?
Part of https://github.com/rust-lang/cargo/issues/14039.
Migrate `tests/testsuite/shell_quoting.rs` to snapbox.
### How should we test and review this PR?
N/A
### Additional information
N/A
Add assert redactions
This was split out from #14048 so that the test changes in that PR do not block the redactions.
This adds auto-redactions for:
- A new `[HASH]` for `/<file>-<16 char hash>`
- `[HOST_TARGET]`
- `[ALT_TARGET]`
- Only added if cross-compilation is allowed for the target
- `[AVG_ELAPSED]`
- For `bench` output
- `[JITTER]`
- For `bench` output
This also moves all common redactions to a function that `assert_e2e` and `assert_ui` call to reduce the amount of duplicate code and makes it so we only compile regex redactions once.
Add local registry overlays
This PR adds (private to cargo internals) support for local registry overlays, in which you can locally pretend to add packages to remote registries; the local packages will have the same source ids as the remote registry that you're overlaying.
There are two ways to set up these overlays: programmatically using `GlobalContext::local_overlays` and through the `__CARGO_TEST_PACKAGE_CONFUSION_VULNERABILITY_DO_NOT_USE_THIS` environment variable. You can't set up these overlays with `.cargo/config`.
The motivation for this is [packaging workspaces](https://github.com/rust-lang/cargo/issues/10948). When we're packing a workspace, we'd like to be able to pretend (for lockfile generation and verification) that some workspace packages are already published even though they aren't.
tests: Migrate alt_registry to snapbox
### What does this PR try to resolve?
The overall goal is to enable the use of snapshot testing on as many cargo tests as possible to reduce the burden when having to touch a lot of tests. Towards that aim, this PR
- Adds snapshot testing to `cargo_test_support::Execs`
- Migrates `alt_registry` tests over as an example (and to vet it)
I've taken the approach of deprecating all other output assertions on `Execs` with `#[allow(deprecated)]` in every test file. This let's us easily identity what files haven't been migrated, what in a file needs migration, and helps prevent a file from regressing. This should make it easier to do a gradual migration that (as many people as they want) can chip in. It comes at the cost of losing visibility into deprecated items we use. Hopefully we won't be in this intermediate state for too long.
To reduce manual touch ups of snapshots, I've added some regex redactions. My main concern with the `FILE_SIZE` redaction was when we test for specific sizes. That shouldn't be a problem because we don't use `Execs::with_stderr` to test those but we capture the output and have a custom asserter for it.
### How should we test and review this PR?
Yes, this puts us in an intermediate state which isn't ideal but much better than one person trying to do all of this in a single branch / PR.
The main risk is that we'll hit a snag with snapbox being able to support our needs. We got away with a lot because everything was custom, down to the diffing algorithm. This is why I at least started with `alt_registry` to get a feel for what problems we might have. There will likely be some we uncover. I'm fairly confident that we can resolve them in some way,
### Additional information
This is a continuation of the work done in #13980.
While this is noisy and hides other deprecations, I figured deprecations would
make it easier for people to discover what tasks remain and allow us to
divide and conquer this work rather than doing a heroic PR.
In theory, this will be short lived and we'll go back to seeing
deprecations in our tests.
Previously when checking if a dependency is a proc-macro,
the v2 feature resolve resolver v2 looks for `proc-macro = true`
for every target of the dependency.
However, it's impossible to depend on a non-lib target as a proc-macro.
This fix switches to only check if `proc-macro = true` for `[lib]`
target for a dependency.
fix(toml): remove `lib.plugin` key support and make it warning
### What does this PR try to resolve?
Remove `lib.plugin` key, making it an "unused key" warning.
Remove some of the tests, which should look useless (I hope I'm understanding this
- [x] Remove key, and related tests.
- [x] Adjust the documentation about the plugin.
- [ ] Some of the comments and function names have not yet finished being modified.
part of #13629Closes#13246
fix(toml): Convert warnings that `licence` and `readme` files do not exist into errors
### What does this PR try to resolve?
In this PR:
- Changed the warning to a hard error and modified the associated test function;
- Removed what should have been a redundant test function：`publish::publish_with_missing_readme`；
- Since `cargo publish` is preceded by the execution of `cargo package`, the error message in the test `function bad_license_file` needs to be modified.
issue: https://github.com/rust-lang/cargo/issues/13629#license-file-and-readme-pointing-to-a-non-existent-file.
### Additional information
It seems that this is not enough, the current situation is that `cargo package` warns if `package.readme` is an empty string or the wrong file location, but if I cancel `package.readme`, no warning is generated.
I'm wondering if I should judge `package.readme&licence` when executing `cargo package` and return an error if it doesn't exist?
As this has not been done before, your advice is sought.
test(lints): Ensure unused optional dep fires for shadowed dep
This is a way to have an unused optional dependency before 2024 edition.
In prior editions, it is currently a hard error.
I'm tempted to switch that hard error to this lint in prior editions but at minimum, we need to make sure we don't make changes that cause this to revert back to a hard error on 2024+
This is a way to have an unused optional dependency before 2024
edition.
In prior editions, it is currently a hard error.
I'm tempted to switch that hard error to this lint in prior editions
but at minimum, we need to make sure we don't make changes that cause
this to revert back to a hard error on 2024+
Per discussion in https://github.com/rust-lang/cargo/issues/6790. The
--out-dir CLI option and out-dir config option are often confused with
the OUT_DIR environment variable, when the two serve very different
purposes (the former tells Cargo where to copy build artifacts to,
whereas the OUT_DIR environment variable is set *by* Cargo to tell
build scripts where to place their generated intermediate artifacts).
Renaming the option to something less confusing is a prerequisite to
stabilizing it.
Allows the default git/gitoxide configuration to be obtained from the ENV and config
### What does this PR try to resolve?
The default git/gitoxide feautures config can be obtained througt `-Zgit` and `-Zgitoxide`.
However, it cannot be obtained from environment variables or configurations.
It's not very ergonomic.
### How should we test and review this PR?
The previous commit explained the staus quo, the next commit addressed the problem.
### Additional information
Inspired by https://github.com/rust-lang/cargo/issues/11813#issuecomment-1817517629
See also https://github.com/rust-lang/cargo/issues/13285#issuecomment-2016875459
### Change of usage
1. Mirror Zgit/Zgitoxide when they parsed as string
Specify the feature you like
```
CARGO_UNSABLE_GIT='shallow-deps' cargo fetch
cargo fetch --config "unstable.git='shallow-deps'"
cargo fetch -Zgit="shallow-deps"
```
2. Specify partial fields when parsed as table
```
CARGO_UNSTABLE_GITOXIDE_FETCH=true cargo fetch
```
The rest fields will use Rust default value. That said, checkout and internal_use_git2 will be false.
Besides, you can pass true to the whole feature to specify the pre-defined features.
```
CARGO_UNSTABLE_GITOXIDE=true cargo fetch
```
- pass true to enable all pre-definded git/gitoxide features
- support parse git/gitoxide as table in Config, if the field is tagged with #[serde(default)], then it can be skipped
refactor: Transition direct assertions from cargo-test-support to snapbox
### What does this PR try to resolve?
Cargo has a bespoke testing framework for functional tests
- Extra stuff for us to maintain
- Don't leverage benefits from contributions related to other projects
- Less incentive to be thoroughly documented
UI tests are written using snapbox. The latest release of snapbox (#13963) was geared at supporting cargo's needs in the hope that we can consolidate on testing frameworks.
Besides having a single set of semantics, benefits we'd gain include
- Updating of test snapshots
- Fancier redacting of test output (e.g. #13973)
This is the first incremental step in this direction. This replaces direct assertions with snapbox assertions. This still leaves all of the CLI output assertions. These will be done incrementally.
### How should we test and review this PR?
### Additional information
Fix: Skip deserialization of unrelated fields with overlapping name
### What does this PR try to resolve?
Split from https://github.com/rust-lang/cargo/pull/13687#discussion_r1622694446
This fixes the overlap of environment variable names:
> For example, when env_key is UNSTABLE_GITOXIDE_FETCH
and field_key is UNSTABLE_GIT, the field shouldn't be
added because `unstable.gitoxide.fetch` doesn't
belong to `unstable.git` struct.
### How should we test and review this PR?
Updates of test cases `struct_with_overlapping_inner_struct_and_defaults` can be used to compare changes before and after changes
### Additional information
r? weihanglo and very appreciate your more optimized code
This reverts commit f525e1f383.
This removes color control from warnings for unstable features.
For some reason this removed color support from `cargo -Zhelp` in the
tests but I can't reproduce it locally.
The most important thing was getting the config fix in.
There are two follow ups
- Can we have the config working *and* color?
- Why did this fail for this field and not the others we already had
tests for?
I ran out my immediate time box for looking into these.
Fixes#13991
feat: stabilize `cargo update --precise <yanked>`
### What does this PR try to resolve?
Stabilize `cargo update --precise <yanked>`.
The cargo team has discussed the stabilization in the meeting today.
The interface of this is quite small and not as controversial as `--precise <pre-release>`,
since there is no version requirement operators involved.
We'd like to move forward and stabilize this part.
Note that `--precise <yanked>` allows using yanked version only for the specified package,
We leave the cascading allowing yanked versions (e.g. <https://github.com/rust-lang/cargo/issues/4225#issuecomment-1930353693>) as a future extension.
### How should we test and review this PR?
Check if any adjustment needed for warnings and CLI help text.
### Additional information
cc <https://github.com/rust-lang/cargo/issues/4225>.
Include `lints.rust.unexpected_cfgs.check-cfg` in the fingerprint
### What does this PR try to resolve?
When changing the `--check-cfg` args in the `lints.rust.unexpected_cfgs.check-cfg` lint config, the build should be restarted as the arguments we pass to the compiler change, and they can change the diagnostics output by generating new or removing some `unexpected_cfgs` warning(s).
### How should we test and review this PR?
Look at the before and after test (separate commit).
### Additional information
Similar to https://github.com/rust-lang/cargo/pull/13012 which did that for the declared features.
Contrary to that PR, I didn't add a new `DirtyReason` variant, since even the `[lints]` table didn't get one.
r? `@epage`
chore: Update to snapbox 0.6
### What does this PR try to resolve?
This unblocks regex redactions which will help with `Finished in 3.45s` messages as well as maybe switching away from `compare.rs` generally.
### How should we test and review this PR?
### Additional information
Improve error description when deserializing partial field struct
### What does this PR try to resolve?
Fixes#13688
### How should we test and review this PR?
one commit add test, one commit fixed and update the test.
### Additional information
`usize` was renamed from `uint` in order to convey that it's not the
"default integer type". Guide new users to use integers with specific
bit width instead of `usize`.
Fix warning output in build_with_symlink_to_path_dependency_with_build_script_in_git
The test `build_with_symlink_to_path_dependency_with_build_script_in_git` was emitting a large warning block (in my case, about init.defaultBranch) because it was running `git` without filtering its output. It's not clear to me why this test was shelling out to `git` instead of using the built-in test support functions. From what I can tell, this should be exactly equivalent, and silences the warning output.
Those new tests tries to be as exhaustive as possible while being
reasonable in the number of them. To do so we try to check for
check/doc/test/build-script/features with a the `check-cfg` config.
Many of those tests are very similar to their non-config counterpart.
This permits things like this to be recognized and passed to
rustc/rustdoc.
```rust
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(foo)"] }
```
Refactor cargo lint tests
In #13621, it was brought up that [the lints tests are nested more deeply than other UI tests](https://github.com/rust-lang/cargo/pull/13621#discussion_r1538065181). This got me wondering if there was a better way to structure all the lint tests.
What I came up with was:
- Lints should not have UI tests, only parts of the diagnostic system, i.e., how warnings, errors, notes, etc., look
- This is because UI tests should focus on parts of the system that make up each lint's output
- We can always add UI tests for each lint if desired
- All tests related to the linting system should live in `tests/testsuite/lints/`
- Tests related to `[lints.cargo]` should stay in `lints_table.rs` as it is for the whole lints table
- Each lint will get a file in `lints/` for all of its tests
- This makes `lints/mod.rs` smaller and targeted only at tests for the linting system itself
- It makes it much easier to know where to place a test
Old syntax suggestion
Fixes#13868.
The build error in the issue will now include a suggestion:
```
Compiling zerocopy v0.8.0-alpha.9
error: the `cargo::` syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of `zerocopy v0.8.0-alpha.9` is 1.56.0.
Consider using the old `cargo:` syntax in front of `rustc-check-cfg=`.
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.
```
The suggestion is only included for reserved prefixes.
A test has been added.
Fix: Build only the specified artifact library when multiple types are available
### What does this PR try to resolve?
Fixes#12109.
#### TL;DR:
A crate `bar` exposes it's library as both a `staticlib` and a `cdylib`. A second crate `foo` specifies an artifact dependency of type `staticlib` on `bar`, meaning cargo should build `bar` as a `staticlib` and expose certain environment variables (e.g. `CARGO_STATICLIB_FILE_BAR`). However, due to a bug, cargo ends up building (and exposing the associated environment variables) for both the `staticlib` and `cdylib`.
The same happens if `foo` specifies an artifact dependency of type `cdylib`; both artifact types are built.
### How should we test and review this PR?
The first commit introduces a test which reproduces the issue, the second commit introduces the fix. This setup was recommended by `@ehuss.`
### Additional information
Artifact dependencies: https://rust-lang.github.io/rfcs/3028-cargo-binary-dependencies.html
#### TL;DR
If a crate `foo` requests an artifact dependency of kind <artifact_kind> from a crate `bar` then the following happens:
- `bar` is built with crate-type <artifact_kind> and a directory is created at `target/<profile>/deps/artifact/bar-<build_hash_or_something>/<artifact_kind>`. The binary artifact is placed in this directory.
- Cargo exposes certain environment variables, the most important for this PR is `CARGO_<artifact_kind>_FILE_BAR`, which points to the binary artifact that was specified. This environment variable is available at compile time for normal dependencies and at runtime for build-dependencies.
If multiple artifact-kinds are requested cargo will create a unit for each, and so they will all be built separately.
fix(toml): Remove unstable rejrected frontmatter syntax for cargo script
### What does this PR try to resolve?
With rust-lang/rfcs#3503 approved, we no longer need to allow easy, high fidelity experiments with alternative cargo script syntax.
### How should we test and review this PR?
### Additional information
We still need to improve the experience for users writing bad syntax but that can come later.
Fix global_cache_tracker::max_download_size test flakiness
This (hopefully) fixes an issue where the `global_cache_tracker::max_download_size` test was sporadically failing on CI. My theory is that the `populate_cache` function was inconsistently saving entries with either the same timestamp or timestamps that differed by 1 second. The SQL query in `get_registry_items_to_clean_size_both` sorts the results based on `(timestamp,name)`. Thus if the timestamps were the same, it was sorting on name. If they differed, then the timestamp would dominate. The solution is to force the tests to use the same basis for the starting time so that a function call like `days_ago(1)` returns consistent results.
I don't have a particularly good way to reproduce the issue. Adding a sleep into `populate_cache` causes 100% errors. Running on a slowed down system, or perhaps GitHub Actions might also reproduce, but I did not try.
Turns out, we allow these fields, just in limited ways, so we need to be
consistent.
I limited when this applies to reduce noise from the user solving there
problem because they are unlikely to keep the field and switch it to the
opposite value
fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting
### What does this PR try to resolve?
This is part of rust-lang/rust#123754
This is a follow up to #11409 which tweaked how we do inheritance of default-features, including warning when `default-features = false` is ignored. This turns those warnings into an error.
### How should we test and review this PR?
### Additional information
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.
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.
