`cargo add` - fix for adding features from repository with multiple packages.
Fixes#13121
As discussed in the issue, when using `cargo add` to add a package with features from a git repository from one of it's members, the command might fail due to improper target for querying for said features.
This PR adds a test for this edge-case where we expect it to fail with current code. It also adds a fix for this, and updates the test to expect success.
While populating available features, the current code does a `Fuzzy` search which might lead to searching for features in a wrong member package. If we change it to an `Exact` query, we get back the proper member to search within.
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.
