This was allowed when switching from `toml` v0.5 to `toml_edit` which
started allowing empty keys when parsing TOML.
This mirrors the change
we made for disallowing empty feature names in #12928.
fix(toml)!: Disallow `[lints]` in virtual workspaces
This was missed with the initial `[lints]` implementation.
While this is a breaking change, this is aligned with ones we've done in the past. A lot of times, we warn first. My hope is that isn't needed this time because
- It only exists virtual workspaces so they aren't published
- It is a nop to have this which is likely to be caught
- This is so new that the number of people using it, and likely running into this case, is quite low.
This was missed with the initial `[lints]` implementation.
While this is a breaking change, this is aligned with ones we've done in
the past. A lot of times, we warn first. My hope is that isn't needed
this time because
- It only exists virtual workspaces so they aren't published
- It is a nop to have this which is likely to be caught
- This is so new that the number of people using it, and likely running
into this case, is quite low.
Limit exported-private-dependencies lints to libraries
### What does this PR try to resolve?
Completed https://github.com/rust-lang/cargo/issues/13039.
This PR limit `exported-private-dependencies` lint in libraray `Target`.
### How should we test and review this PR?
Your can checkout out 2348ac2a20edf772495349d7911938251e343bf1 and run test, it will failed and then it will be passed in the commit 2348ac2a20edf772495349d7911938251e343bf1
### Additional information
In https://github.com/rust-lang/rust/blob/87e1447aa/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs#L856
when the remap result of `work_dir` is an empty string,
LLVM won't generate some symbols for root debuginfo node.
For example, `N_SO` and `N_OSO` on macOS,
or `DW_AT_comp_dir` on Linux when debuginfo is splitted.
Precisely, it is observed that when the `DIFile` of compile unit was
provied with an empty compilation `Directory` string,
LLVM would not emit those symbols for the root DI node.
This behavior is not desired,
resulting in corrupted debuginfo and degrading debugging experience.
This is might not be a bug of `--remap-path-prefix` in rustc,
since `-fdebug-prefix-map` in clang 16 could have the same result
(`DW_AT_comp_dir` is gone when `work_dir` is remapped to an empty string).
However, in gcc 12 `fdebug-prefix-map` will return an absolute work_dir
when an empty string occurs.
To not bother whether this needs to be fixed in rustc or not,
let's fix it by always appending an explicit `.`
when `--remap-path-prefix` remaps to relative workspace root
a.k.a. where rustc is invoking.
For more on gcc/clang remap options, see
https://reproducible-builds.org/docs/build-path/
With `gix-config` now being fixed, it will properly respect `GIT_CONFIG_NOSYSTEM`
both for system-wide configuration as well as for the git installation configuration.
That way, credential helpers provided by the git installation won't be called anymore,
which prevents them from 'somehow' emitting information to stderr.
The latter was previously disabled for credential helpers, and despite
everything^1 looking like it should work, it simply didn't.
[1]: https://github.com/rust-lang/cargo/pull/13117#issuecomment-1844881287
fix(toml): Disallow inheriting of dependency public status
### What does this PR try to resolve?
This is a step towards rust-lang/rust#44663. When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance. We can always add it later. imo the principle of what should
be inherited is what is truely common among dependencies. For example,
we don't allow removing features. Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.
### How should we test and review this PR?
### Additional information
This reverts parts of #12817
re-enable previously disabled tests with Windows-specific fix
Related to #11821 for which this is a fix.
However, it's probably not yet the optimal solution, depending on how `stderr` of subprocesses should be handled.
### Tasks
* [x] try to fix the issue with an env var.
- Failure, as one warning remains that seems to originate from a C# HTTP client
* [x] figure out if `stderr` should be on or off by default - on by default like before, but now one can control it.
* [x] create a new `gix` release and use it here
### Review Notes
* Personally, I think `cargo` should keep `stderr` to be inherited so users can see potentially relevant warnings or errors provided by credential helpers. Thus this is still the default, but the tests that need it explicitly disabled `stderr` of credential helpers.
----
On Windows, `gix` will call the `git-credential-manager, but with
`stderr` set to `inherit` which makes any errors visible to the user,
just like `git` does.
```
1 1 Updating git repository `https://foo.bar/foo/bar`
2 +warning: auto-detection of host provider took too long (>2000ms)
3 +warning: see https://aka.ms/gcm/autodetect for more information.
4 +fatal: A task was canceled.
5 +warning: auto-detection of host provider took too long (>2000ms)
6 +warning: see https://aka.ms/gcm/autodetect for more information.`
````
This, however, isn't what's desirable in tests sometimes, nor may
it be desirable in Cargo.
For now, it seems easiest to disable this particular feature that
issues the warning messages, even though a future `gix` update
should allow to control what to do with `stderr`.
On Windows, `gix` will call the `git-credential-manager, but with
`stderr` set to `inherit` it makes any errors visible to the user,
just like `git` does.
```
1 1 Updating git repository `https://foo.bar/foo/bar`
2 +warning: auto-detection of host provider took too long (>2000ms)
3 +warning: see https://aka.ms/gcm/autodetect for more information.
4 +fatal: A task was canceled.
5 +warning: auto-detection of host provider took too long (>2000ms)
6 +warning: see https://aka.ms/gcm/autodetect for more information.`
````
This, however, isn't what's desirable in tests sometimes, nor may
it be desirable in Cargo.
In the latest version of `gix`, it's possible to control `stderr`
which is now set on a per-test basis.
Also note that for `cargo` as a whole the default didn't change,
and stderr of spawned helper programs will remain visible in the
enclosing terminal.
feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs
### What does this PR try to resolve?
This tries to add just enough information to Package ID Specs that we can be sure they are unambiguous. On its own thats important but this will unblock #12914 so we can have a user-facing ID that can be used with cargo's CLI.
More specifically, this adds
- `git+`, etc prefixes to the scheme
- These were previously stabilized for `cargo metadata`s source id urls
- Like with `SourceID`, this will allow `PackageIDSpec` to generate `directory+` and `local-registry+` prefixes but not parse them. I'm assuming that the layers where those are dealt with they won't appear here but I don't have enough experience with them
- git ref query strings for URLs
Things from `SourceID` that this does not include
- precise: this seems less related to matching (e.g. ignored for `impl Ord for SourceId`)
- canonical URL for git kinds: this could be nice for users but users aren't the target audience and we can switch to these later
Fixes#10256
### How should we test and review this PR?
Per-commit
### Additional information
This is a step towards #44663. When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance. We can always add it later. imo the principle of what should
be inherited is what is truely common among dependencies. For example,
we don't allow removing features. Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.
This reverts parts of #12817
refactor(schemas): Pull out mod for proposed schemas package
Originally for #12801 we talked about a `cargo-util-manifest-schema` package
- `util` in the name to not clash with plugins
- manifest specific to keep the scope down
The problem is we have types that aren't manifest specific, like
- `PartialVersion` (currently slated for `cargo-util-semverext`)
- `RustVersion`
- `PackageIdSpec`
- `SourceKind` (soon)
Things get messy if we try to break things down into common packages. Instead, I think it'd be useful to have a schemas package that has mods for each type of schema, re-exporting what is needed.
Normally, componentizing your package by the layer in the stack is a recipe for pain.
I don't think that'll apply here because these are meant to be so low level.
The other big concern could be compile times. My hope is it won't be too bad.
So this moves the `util/toml` types into the module and we can add more in the future.
This is more robust than `thread::sleep`, ensuring
* foo is running before the first `cargo uninstall` call
* foo is stopped before the second `cargo uninstall` call
Include declared list of features in fingerprint for `-Zcheck-cfg`
This PR include the declared list of features in fingerprint for `-Zcheck-cfg` (#10554)
`--check-cfg` verifies that `#[cfg()]` attributes are valid in Rust code, which includes `--cfg features foo` definitions that came from `[features]` tables in `Cargo.toml`. For us to correctly re-check cfgs on feature changes,we need to include them in the fingerprint.
For example, if a user deletes an entry from `[features]`, they should then get warnings about any `#[cfg()]` attributes left in the code. Without this change, that won't happen. With this change, it now does.
Otherwise changing (add, removing, ...) features from the `[features]`
table would not cause rustc to be called with the new `--check-cfg`,
showing potentially outdated warnings.
Originally for #12801 we talked about a `cargo-util-manifest-schema` package
- `util` in the name to not clash with plugins
- manifest specific to keep the scope down
The problem is we have types that aren't manifest specific, like
- `PartialVersion` (currently slated for `cargo-util-semverext`)
- `RustVersion`
- `PackageIdSpec`
- `SourceKind` (soon)
Things get messy if we try to break things down into common packages.
Instead, I think it'd be useful to have a schemas package that has mods
for each type of schema, re-exporting what is needed.
Normally, componentizing your package by the layer in the stack is a
recipe for pain.
I don't think that'll apply here because these are meant to be so low
level.
The other big concern could be compile times. My hope is it won't be
too bad.
So this moves the `util/toml` types into the module and we can add more
in the future.
Have cargo add --optional <dep> create a <dep> = "dep:<dep> feature
### What does this PR try to resolve?
`cargo add --optional <dep>` would create a `<dep> = "dep:<dep>` feature iff
- `rust-version` is unset or is new enough for the syntax
- `dep:<dep>` doesn't already exist
Fixes https://github.com/rust-lang/cargo/issues/11010
### How should we test and review this PR?
As the `dep:` syntax is only available starting with Rust 1.60. this pr maintains the previous usage convention in the earlier version.
run
```shell
cargo add --optional <dep>
```
with different rust-version to verify.
Add `--public` for `cargo add`
## What does this PR try to resolve?
Complete https://github.com/rust-lang/cargo/issues/13037
This PR want to add `--public/--no public` flag for `cargo add`
Note: this assumes we'll remove workspace inheritance support for `public` as it sounds like we'll be reverting it https://github.com/rust-lang/rust/issues/44663#issuecomment-1826474620. If we decide to keep workspace inheritance, we'll need to come back and update this.
## How should we test and review this PR?
Most of Code were reference `cargo add --optional`, So can reviewed the new code based on the part of `optional` code.
The new testcases were origin from the `cargo add --optional` part.
- `public` testcase：there is no dependencies and will be add `public` dependencies.
- `no_public` testcase: there is no dependencies and will be add `no_public` dependencies.
- `overwrite_public` testcase: the dependencies already exists but will be overwrite with `public`.
- `overwrite_no_public` testcase: the dependencies already exists but will be overwrite with `no_public`.
- `overwrite_public_with_no_public` testcase: the dependencies already marked as `no_public` and will be overwrite with `public`.
- `overwrite_no_public_with_public` testcase: the dependencies already marked as `public` and will be overwrite with `no_public`.
Fixed uninstall a running binary failed on Windows
### What does this PR try to resolve?
Fixes https://github.com/rust-lang/cargo/issues/3364
The problem reproduce when you try to uninstall a running binary and it will failed on Windows, this is because that cargo had already remove the installed binary tracker information in disk, and next to remove the running binary but it is not allowed in Windows. And to the worst, you can not uninstall the binary already and only to reinstall it by the `--force` flag.
### How to solve the issue?
This PR try to make the uninstall a binary more reasonable that only after remove the binary sucessfully then remove the tracker information on binary and make the remove binary one by one.
### How should we test and review this PR?
Add testcase 0fd4fd357b
- install the `foo`
- run `foo` in another thread.
- try to uninstall running `foo` and only failed in Windows.
- wait the `foo` finish, uninstall `foo`
- install the `foo`
### Additional information
Don't filter on workspace members when scraping doc examples
Fixes https://github.com/rust-lang/cargo/issues/13074.
I confirmed locally that it fixed the issue in docs.rs.
cc `@willcrichton`
r? `@epage`
Fixes error count display is different when there's only one error left
### What does this PR try to resolve?
When there's only 1 error left, the number 1 appears in the output so that it scans the same as the output when there's more than 1 error, so:
```
error: could not compile `crate` (lib test) due to 1 previous error
```
instead of the current:
```
error: could not compile `crate` (lib test) due to a previous error
```
Fixes#12390
rustc related PR [#114759](https://github.com/rust-lang/rust/pull/114759)
fix: reorder `--remap-path-prefix` flags for `-Zbuild-std`
### What does this PR try to resolve?
Order of `--remap-path-prefix` flags is important for `-Zbuild-std`.
We want to show `/rustc/<hash>/library/std` instead of `std-0.0.0`.
Fixesrust-lang/rust#117839
### How should we test and review this PR?
Follow the steps in rust-lang/rust#117839, or run
```
CARGO_RUN_BUILD_STD_TESTS=true cargo +nightly t --test build-std
```
to verify.
Handle $message_type in JSON diagnostics
### What does this PR try to resolve?
Unblocks https://github.com/rust-lang/rust/pull/115691.
Without this change, Cargo's testsuite fails in `doc::doc_message_format` and `metabuild::metabuild_failed_build_json`.
### How should we test and review this PR?
Tested with and without https://github.com/rust-lang/rust/pull/115691.
In Cargo repo: `cargo test --test testsuite`
In Rust repo: `x.py test src/tools/cargo` (separately on master and $message_type PR)
This is an alternative to #12158's `CARGO_WORKSPACE_DIR` that was
implementing the solution to #3946 that previously discussed in the
cargo team meeting.
`CARGO_WORKSPACE_DIR` is a bit awkward to document / describe because
its the effective workspace directory of the thing being built.
If the thing being built doesn't have a workspace, it falls back to
`CARGO_MANIFEST_DIR`.
It would also be hard to take into account what the
`CARGO_WORKSPACE_DIR` would be for path dependencies into foreign
workspaces *and* it wouldn't solve the problem the user is having.
What the user really wants is the CWD of rustc when it is invoked.
This is much simpler to describe and is accurate when using a path
dependency to a foreign package.
Because the CWD is a much simpler mechanism to talk about, I figured we
could diverge from our prior consensus and make it always present,
rather than limiting it to tests.
Remaining work for #3946: get this stabilized
Exited with hard error when custom build file no existence or not in package
## What does this PR try to resolve?
Fixed https://github.com/rust-lang/cargo/issues/11383
## How should we test and review this PR?
Add test `build_script_outside_pkg_root`, this will check `custom_build.rs` existence and whether in the package root, if not then exited with a hard error
## Additional information
The code just handle the `custom build` target that i know how to test it. Other target type is skipped.
