Eric Huss [Tue, 2 Aug 2022 19:54:16 +0000 (12:54 -0700)]
Re-enable version_works_without_rustc on windows.
This test was ignored in https://github.com/rust-lang/cargo/pull/3189
without much discussion of explaining why.
AFAICT, this test works fine on Windows on both MSVC and GNU.
Empty paths do the expected behavior (preventing cargo from running
rustc). There are some special rules on Windows about discovering the
process to run (such as searching the app's launch directory), but
I do not think that is relevant here. Confirmed by trying to run
`cargo check` in this test fails to find `rustc`.
bors [Tue, 2 Aug 2022 14:41:51 +0000 (14:41 +0000)]
Auto merge of #10918 - ehuss:fix-formats_source, r=Eh2406
Fix formats_source test requiring rustfmt.
The requirements added in #9892 that `rustfmt` must be present doesn't work in the `rust-lang/rust` environment. There are two issues:
* Cargo is run without using rustup. If you also have rustup installed, the test will fail because the `rustfmt` binary found in `PATH` will fail to choose a toolchain because HOME points to the sandbox home which does not have a rustup configuration.
* rust-lang/rust CI uninstalls rustup, and does not have rustfmt in PATH at all. It is not practical to make rustfmt available there.
The solution here is to just revert the behavior back to where it was where it checks if it can run `rustfmt` in the sandbox. This should work for anyone who has a normal rustup installation (including Cargo's CI). If running the testsuite without rustup, then the test will be skipped.
This also includes a small enhancement to provide better error information when rustfmt fails.
The test `scrape_examples_complex_reverse_dependencies` no longer works on the latest nightly. It fails with the error:
```
error[E0433]: failed to resolve: could not resolve path `a::f`
--> examples/ex.rs:1:13
|
1 | fn main() { a::f(); }
| ^^^^ could not resolve path `a::f`
|
= note: this error was originally ignored because you are running `rustdoc`
= note: try running again with `rustc` or `cargo check` and you may get a more detailed error
error: Compilation failed, aborting rustdoc
For more information about this error, try `rustc --explain E0433`.
error: could not document `foo`
```
It is not clear to me what this test was trying to exercise, so I'm not sure how to fix it. It has an example that is trying to call a function from a proc-macro, but proc-macros do not export functions.
bors [Mon, 1 Aug 2022 00:51:01 +0000 (00:51 +0000)]
Auto merge of #9892 - ehuss:cargo_test-ignore-reason, r=weihanglo
Add requirements to cargo_test.
This extends the `#[cargo_test]` attribute to support some additional requirements to control whether or not a test can run. The motivation for this is:
* Can more clearly see when a test is disabled when it prints "ignored"
* Can more easily scan for disabled tests when I do version bumps to see which ones should be enabled on stable (to pass on CI).
The form is a comma separated list of requirements, and if they don't pass the test is ignored. The requirements can be:
* `nightly` — The test only runs on nightly.
* `>=1.55` — The test only runs on rustc with the given version or newer.
* `requires_git` — Requires the given command to be installed. Can be things like `requires_rustfmt` or `requires_hg`, etc.
This also enforces that the author must write a reason why it is ignored (for some of the requirements) so that when I look for tests to update, I know why it is disabled.
This also removes the `CARGO_TEST_DISABLE_GIT_CLI` option, which appears to no longer be necessary since we have migrated to GitHub Actions.
Auto merge of #10914 - ehuss:contrib-new-release, r=epage
Contrib: Document new-release process
This adds some contributor documentation on the process I use to bump the version and update the changelog. In case I am unavailable to create the changelog, it may perhaps be useful to someone.
Of course nobody is required to use this process, but I find it works fairly smoothly. However, the tool I use is a hacked together script, and may have some hard-coded things, so it may need some work to be useful to others.
Auto merge of #10911 - Nemo157:override-to-resolver-1, r=epage
Override to resolver=1 in published package
As discussed in #10112 this avoids inconsistent dependency resolution in development and packaged builds when an edition 2021 crate is published from a workspace using the default resolver=1.
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API
A downside is we update the git dependencies a second time and any sources we didn't initially update.
Unlike the proposal in the attached issue, this does not roll back on error.
- For some errors, the user might want to debug what went wrong. Losing the intermediate state makes that difficult
- Rollback adds its own complications and risks, including since its
non-atomic
We've already tried to address most potential errors during `cargo add`s processing. To meet this desire, we now error if `--locked` is passed in and we would change the manifest. See that individual commit's message for more details.
Ed Page [Mon, 25 Jul 2022 15:11:50 +0000 (10:11 -0500)]
fix(add): Update the lock file
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API
A downside is we update the git dependencies a second time.
We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
non-atomic
Ed Page [Mon, 25 Jul 2022 14:51:58 +0000 (09:51 -0500)]
fix(add): Respect --locked
This is prep for #10901 to avoid the most common failure case for the
lock file. We are assuming if the users action caused a change in the
manifes, then it will cause a change in the lock file. This isn't
entirely true but close enough and I think these semantics can make
sense.
Auto merge of #10899 - ehuss:empty-wrapper-test, r=weihanglo
Make the empty rustc-wrapper test more explicit.
This changes the test for an empty RUSTC_WRAPPER environment variable to make it explicit that it doesn't just ignore the environment variable, but that it also essentially unsets any config-loaded value. It's not clear if this implication was known at the time it was added in #5985, but I don't think we can change it, and it can be useful.
refactor(source): Open query API for adding more types of queries
### What does this PR try to resolve?
This refactors the Source/Registry traits from accepting a `fuzzy: bool` to accepting an enum so we can add alternative query styles in the future, as discussed in the Cargo team meeting for fixing #10729
The expected fix for `cargo add` at this point would be
- Add `QueryKind::Normalized`
- Initially, we can make it like Exact for path sources and like Fuzzy for registry sources
- Switch cargo-add to using that kind everywhere (both where `Exact` and `Fuzzy` are used)
- A test to ensure this fixed it
- Rename `Fuzzy` to `Alternatives` or something to clarify its actual intent
### How should we test and review this PR?
The refactor is broken down into multiple commits, so ideally review a commit at a time to more easily see how it evolved.
### Additional information
Future possibilities
- Supporting normalized search on all sources
- Doing version / source matching for normalized results (probably not needed for cargo-add but will make the API less surprising for future users)
Auto merge of #10868 - Muscraft:add-reason-for-nightly-tests, r=ehuss
add a reason to `masquerade_as_nightly_cargo` so it is searchable
When I was working on the stabilization for workspace inheritance, it was very tedious to find all of the places to remove `.masquerade_as_nightly_cargo()`. I [suggested](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/problems.20finding.20.60.2Emasquerade_as_nightly_cargo.28.29.60) to add a reason to `.masquerade_as_nightly_cargo()` so that it would be easier to find all of the places to remove it. By adding the reason it makes it easy to search for all places with the features name. This PR adds the reason(s) to all of the places `.masquerade_as_nightly_cargo()` is called, as well as updates the documentation so it talks about adding a reason when making the call.
Auto merge of #10867 - Muscraft:remove-unneeded-nightly-from-tests, r=ehuss
remove`.masquerade_as_nightly_cargo()` from various tests the no longer need it
When looking at making [`.masquerade_as_nightly_cargo()` take in a list of reasons](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/problems.20finding.20.60.2Emasquerade_as_nightly_cargo.28.29.60), I found various tests that no longer needed `.masquerade_as_nightly_cargo()`. This removes it from all of the offending tests.
I tried my best to find all tests that no longer need it but I could have missed one or two.
Auto merge of #10866 - Muscraft:update-extra-link-tests, r=ehuss
remove `.masquerade_as_nightly_cargo()` from build_script_extra_link_arg.rs
When looking at making [`.masquerade_as_nightly_cargo()` take in a list of reasons](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/problems.20finding.20.60.2Emasquerade_as_nightly_cargo.28.29.60), I found that [`build_script_extra_link_arg.rs`](https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/tests/testsuite/build_script_extra_link_arg.rs) still was using it. `extra-link-arg` was [stabilized in 1.56.0](https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/src/doc/src/reference/unstable.md#extra-link-arg), so this PR removes `.masquerade_as_nightly_cargo()` from all the tests in this file.
Auto merge of #10862 - Muscraft:workspace-selection-test, r=epage
Add a test for regressions in selecting the correct workspace root
This adds a test to check for regressions in selecting the correct workspace when there are nested workspaces.
#10846 solved a problem with nested workspace resolution that was caused by #10776. `@ehuss` [suggested](https://github.com/rust-lang/cargo/pull/10846#issuecomment-1183754728) that a test should be added to ensure that this issue does not pop up again.
I ensured that this worked by testing against commit before #10846. Sporadically I would get an error that was the same as described in #10846.
```
error: package `{path}/cargo/target/tmp/cit/t0/foo/sub/foo/Cargo.toml` is a member of the wrong workspace
expected: {path}/cargo/target/tmp/cit/t0/foo/sub/Cargo.toml
actual: {path}/cargo/target/tmp/cit/t0/foo/Cargo.toml
```
I then tested it on the commit with the fix and the test passed every time.
---
While this does add a test to catch any regression I am worried that it will not catch it every time. It was noted in #10846 that this error would sometimes happen but not every time, in my testing I found this to be true as well. Since this is caused by the `HashMap` order changing each run, switching to something ordered like `BTreeMap` **_should_** catch any regressions every run (if the implementation were to ever change). I'm not sure if this is necessary so I figured I would note the concern here.
Auto merge of #10861 - RalfJung:cargo-install-debug, r=ehuss
clarify profile used for 'cargo install --debug'
TIL that the profile used by `cargo build` is called `dev`, not `debug`. That made me wonder, is that profile also used by `cargo install --debug` (despite the name of the flag being `--debug`, not `--dev`)? Turns out the answer is yes, but the first 2 places where I looked for help did not say that. So this PR changes those two places to be explicit about this.
A comment on killercup/cargo-edit#15 had me worried that `cargo add` was
deleting comments now. It appears that isn't the case for inline
tables.
Standard tables however do delete comments. The work to make sure they
don't conflicts with another need. When changing the source, we delete
the old source fields and append the new which can cause some formatting
to be carried over unnecessarily.
For example, what would normally look like
```toml
cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" }
```
When fixed to preserve comments with my naive solution looks like
```toml
cargo-list-test-fixture-dependency = { optional = true , path = "../dependency", version = "0.0.0" }
```
Note that `optional = true` used to be last, so space separating it and
`}` was kept, now separating it and `,`.
More work will be needed to get this into an ideal state but we can at
least have confidence with inline tables for now.
Ed Page [Mon, 11 Jul 2022 21:14:34 +0000 (16:14 -0500)]
test(add): Ensure comments are preserved
A comment on killercup/cargo-edit#15 had me worried that `cargo add` was
deleting comments now. It appears that isn't the case for inline
tables.
Standard tables however do delete comments. The work to make sure they
don't conflicts with another need. When changing the source, we delete
the old source fields and append the new which can cause some formatting
to be carried over unnecessarily.
For example, what would normally look like
```toml
cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" }
```
When fixed to preserve comments with my naive solution looks like
```toml
cargo-list-test-fixture-dependency = { optional = true , path = "../dependency", version = "0.0.0" }
```
Note that `optional = true` used to be last, so space separating it and
`}` was kept, now separating it and `,`.
More work will be needed to get this into an ideal state but we can at
least have confidence with inline tables for now.
Auto merge of #10846 - EmbarkStudios:fix-workspace-resolution, r=epage
Fix nested workspace resolution
This fixes a bug that was introduced in #10776 with nested workspaces.
As an example, say we have two workspaces:
`/code/example/Cargo.toml` and `/code/example/sub/Cargo.toml`, and a crate within the `sub` workspace `/code/example/sub/test-crate/Cargo.toml`.
Since the `ws_roots` is a HashMap with randomized ordering, this code will _sometimes_ cause the workspace at `/code/example/Cargo.toml` to be discovered and used _before_ `/code/example/sub/Cargo.toml`,
This change just makes it so that the input manifest path is walked up to find the (presumably) most appropriate workspace so that the ordering of the `HashMap` doesn't matter.
If you encounter this bug by running cargo nightly, you can workaround it by adding the crate(s) to the `excluded` field in the workspace they don't belong to.
Auto merge of #10836 - ehuss:patch-config-docs, r=weihanglo
Mention `[patch]` config in "Overriding Dependencies"
The "Overriding Dependencies" chapter is the primary documentation for the `[patch]` table, but it does not mention that `[patch]` also works in config files. This adds a note to mention this option.
cc https://github.com/rust-lang/cargo/issues/10832#issuecomment-1177647089
Auto merge of #10833 - ehuss:terminal-width-new-flag, r=Eh2406
Update terminal-width flag.
The rustc flag `-Zterminal-width` has been stabilized as `--terminal-width` in https://github.com/rust-lang/rust/pull/95635. This updates cargo to use the new flag so that tests will pass.
Tests won't pass until the next nightly is published in about 10 hours from now. I just wanted to post this to get ahead of the breaking change.
This doesn't stabilize in cargo because that will take more time, and this is needed to prevent CI from failing. Will continue the stabilization discussion at https://github.com/rust-lang/rust/issues/84673.
Auto merge of #10831 - arlosi:sparse-publish-fix, r=Eh2406
Fix publishing to crates.io with -Z sparse-registry
Attempting to publish a crate to crates.io with `-Z sparse-registry` failed with the following error:
```
error: failed to publish to registry at https://crates.io
Caused by:
the remote server responded with an error: Dependency `serde` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.
```
The check in `registry.rs` `dep_registry_id != registry_id` caused the `publish` operation include the crates.io index url in the HTTP request because the id was replaced. The crates.io API seems to require that the `registry` field is not present.
This change fixes the issue by making the `registry` function return the non-replaced crates.io `source_id` only for this case. Other replacement indices of crates.io continue to include the registry URL when publishing.
Tested manually by publishing `arlosi-cargo-test` to crates.io with `-Z sparse-registry`
Auto merge of #10830 - arlosi:parallel_yank, r=Eh2406
Make `is_yanked` return `Poll<>`
The `is_yanked` check performed by `cargo install` and `cargo package` was running sequentially (calling `block_until_ready` after every check).
This change makes `is_yanked` return `Poll<>` and runs the check in parallel, which gives better performance for `cargo install --locked` and `cargo package` when using a sparse registry.
projects / cargo.git / log