This was written for `cargo_add.rs`, based on `snapbox`. This allows
creating a test from a known reproduction case or easily debugging on an
existing test case.
In #10472, cargo-add was merged with support for an inline version
syntax of `foo@version`. That also served as the change proposal for
extending that syntax to `cargo install` for convinience and consistency.
While both commands are specifying a version-req, `cargo-install` has an
implicit-but-required `=` operand while `cargo-add` allows any operand.
This doesn't use the full `pkgid` syntax because that allows syntax that
is unsupported here.
This doesn't use `cargo-add`s parser because that is for version reqs.
I held off on reusing the parser from `cargo-yank` because they had
different type system needs and the level of duplication didn't seem
worth it (see Rule of Three).
Mark .cargo/git and .cargo/registry as cache dirs
Fixes#10457
### What does this PR try to resolve?
As we previously discussed in #10457 this marks `~/.cargo/git` and `~/.cargo/registry` as not to be included in backups, and not subject to content indexing. These directories can be fairly large and frequently changed, and should only contain content that can be re-downloaded if necessary.
### How should we test and review this PR?
I did two manual tests:
1. Using the binary built from this tree, run `cargo update` in a source tree that has a git dependency, and observe that afterwards, there is a `CACHEDIR.TAG` in `~/.cargo/git`.
2. Similarly, run `cargo update` and observe that there's a `CACHEDIR.TAG` in `~/.cargo/registry`.
(I ran this on Linux. This code should also trigger OS-specific behavior on macOS and Windows that's the same as is currently applied to `target/`.)
I added some test assertions.
fix(yank): Use '--version' like install
### What does this PR try to resolve?
During the design conversations on cargo-add, we noticed:
- `cargo-install` has a public flag `--version` and an invisible alias `--vers`
- `cargo-yank` has a public flag `--vers`
This switches `cargo-yank` to publicly use `--version` and have an invisible alias
`--vers`, making it consistent with `cargo-install`.
### How should we test and review this PR?
This updated all tests to use the "recommended" flag.
In #10472, cargo-add was merged with support for an inline version
syntax of `foo@version`. That also served as the change proposal for
extending that syntax to `cargo yank` for convinience and consistency.
The major difference is that `cargo-add` is specifying a version-req
while `cargo-yank` is specifying a version.
This doesn't use the full `pkgid` syntax because that allows syntax that
is unsupported here.
This doesn't use `cargo-add`s parser because that is for version reqs.
During the design conversations on cargo-add, we noticed that
`cargo-install` has a public flag `--version` and an invisible alias
`--vers` while `cargo-yank` has a public flag `--vers`. This switches
`cargo-yank` to publicly use `--version` and have an invisible alias
`--vers`, making them consistent.
Completions are a best guess.
In addition to `foo:1.2.3`, we now support `foo@1.2.3` for pkgids. We
are also making it the default way of rendering pkgid's for the user.
With cargo-add in #10472, we've decided to only use `@` in it and to add
it as an alternative to `:` in the rest of cargo. `cargo-add`
originally used `@`. When preparing it for merge, I switched to `:` to
be consistent with pkgids. When discussing this, it was felt `@` has
precedence in too many tools to switch to `:` but that we should instead
switch pkgid's to use `@`, in a backwards compatible way.
See also https://internals.rust-lang.org/t/feedback-on-cargo-add-before-its-merged/16024/26?u=epage
feat: Import cargo-add into cargo
### Motivation
The reasons I'm aware of are:
- Large interest, see #5586
- Make it easier to add a dependency when you don't care about the version (instead of having to find it or just using the major version if thats all you remember)
- Provide a guided experience, including
- Catch or prevent errors earlier in the process
- Bring the Manifest format documentation into the terminal via `cargo add --help`
- Using `version` and `path` for `dependencies` but `path` only for `dev-dependencies` (see crate-ci/cargo-release#288 which led to killercup/cargo-edit#480)
### Drawbacks
1. This is another area of consideration for new RFCs, like rust-lang/rfcs#3143 (this PR supports it) or rust-lang/rfcs#2906 (implementing it will require updating `cargo-add`)
2. This is a high UX feature that will draw a lot of attention (ie Issue influx)
e.g.
- killercup/cargo-edit#521
- killercup/cargo-edit#126
- killercup/cargo-edit#217
We've tried to reduce the UX influx by focusing the scope to preserving semantic information (custom sort order, comments, etc) but being opinionated on syntax (style of strings, etc)
### Behavior
Help output
<details>
```console
$ cargo run -- add --help
cargo-add [4/4594]
Add dependencies to a Cargo.toml manifest file
USAGE:
cargo add [OPTIONS] <DEP>[`@<VERSION>]` ...
cargo add [OPTIONS] --path <PATH> ...
cargo add [OPTIONS] --git <URL> ...
ARGS:
<DEP_ID>... Reference to a package to add as a dependency
OPTIONS:
--no-default-features Disable the default features
--default-features Re-enable the default features
-F, --features <FEATURES> Space-separated list of features to add
--optional Mark the dependency as optional
-v, --verbose Use verbose output (-vv very verbose/build.rs output)
--no-optional Mark the dependency as required
--color <WHEN> Coloring: auto, always, never
--rename <NAME> Rename the dependency
--frozen Require Cargo.lock and cache are up to date
--manifest-path <PATH> Path to Cargo.toml
--locked Require Cargo.lock is up to date
-p, --package <SPEC> Package to modify
--offline Run without accessing the network
--config <KEY=VALUE> Override a configuration value (unstable)
-q, --quiet Do not print cargo log messages
--dry-run Don't actually write the manifest
-Z <FLAG> Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for
details
-h, --help Print help information
SOURCE:
--path <PATH> Filesystem path to local crate to add
--git <URI> Git repository location
--branch <BRANCH> Git branch to download the crate from
--tag <TAG> Git tag to download the crate from
--rev <REV> Git reference to download the crate from
--registry <NAME> Package registry for this dependency
SECTION:
--dev Add as development dependency
--build Add as build dependency
--target <TARGET> Add as dependency to the given target platform
EXAMPLES:
$ cargo add regex --build
$ cargo add trycmd --dev
$ cargo add --path ./crate/parser/
$ cargo add serde serde_json -F serde/derive
```
</details>
Example commands
```rust
cargo add regex
cargo add regex serde
cargo add regex@1
cargo add regex@~1.0
cargo add --path ../dependency
```
For an exhaustive set of examples, see [tests](https://github.com/killercup/cargo-edit/blob/merge-add/crates/cargo-add/tests/testsuite/cargo_add.rs) and associated snapshots
Particular points
- Effectively there are two modes
- Fill in any relevant field for one package
- Add multiple packages, erroring for fields that are package-specific (`--rename`)
- Note that `--git` and `--path` only accept multiple packages from that one source
- We infer if the `dependencies` table is sorted and preserve that sorting when adding a new dependency
- Adding a workspace dependency
- dev-dependencies always use path
- all other dependencies use version + path
- Behavior is idempotent, allowing you to run `cargo add serde serde_json -F serde/derive` safely if you already had a dependency on `serde` but without `serde_json`
- When a registry dependency's version req is unspecified, we'll first reuse the version req from another dependency section in the manifest. If that doesn't exist, we'll use the latest version in the registry as the version req
### Additional decisions
Accepting the proposed `cargo-add` as-is assumes the acceptance of the following:
- Add the `-F` short-hand for `--features` to all relevant cargo commands
- Support ``@`` in pkgids in other commands where we accept `:`
- Add support for `<name>`@<version>`` in more commands, like `cargo yank` and `cargo install`
### Alternatives
- Use `:` instead of ``@`` for versions
- Flags like `--features`, `--optional`, `--no-default-features` would be position-sensitive, ie they would only apply to the crate immediate preceding them
- This removes the dual-mode nature of the command and remove the need for the `+feature` syntax (`cargo add serde -F derive serde_json`)
- There was concern over the rarity of position-sensitive flags in CLIs for adopting it here
- Support a `--sort` flag to sort the dependencies (existed previously)
- To keep the scope small, we didn't want general manifest editing capabilities
- `--upgrade <POLICY>` flag to choose constraint (existed previously)
- The flag was confusing as-is and we feel we should instead encourage people towards `^`
- `--allow-prerelease` so a `cargo add clap` can choose among pre-releases as well
- We felt the pre-release story is too weak in cargo-generally atm for making it first class in `cargo-add`
- Offer `cargo add serde +derive serde_json` as a shorthand
- Infer path from a positional argument
### Prior Art
- *(Python)* [poetry add](https://python-poetry.org/docs/cli/#add)
- `git+` is needed for inferring git dependencies, no separate `--git` flags
- git branch is specified via a URL fragment, instead of a `--branch`
- *(Javascript)* [yarn add](https://yarnpkg.com/cli/add)
- `name@data` where data can be version, git (with fragment for branch), etc
- `-E` / `--exact`, `-T` / `--tilde`, `-C` / `--caret` to control version requirement operator instead of `--upgrade <policy>` (also controlled through `defaultSemverRangePrefix` in config)
- `--cached` for using the lock file (killercup/cargo-edit#41)
- In addition to `--dev`, it has `--prefer-dev` which will only add the dependency if it doesn't already exist in `dependencies` as well as `dev-dependencies`
- `--mode update-lockfile` will ensure the lock file gets updated as well
- *(Javascript)* [pnpm-add](https://pnpm.io/cli/add)
- *(Javascript)* npm doesn't have a native solution
- Specify version with ``@<version>``
- Also overloads `<name>[`@<version>]`` with path and repo
- Supports a git host-specific protocol for shorthand, like `github:user/repo`
- Uses fragment for git ref, seems to have some kind of special semver syntax for tags?
- Only supports `--save-exact` / `-E` for operators outside of the default
- *(Go)* [go get](https://go.dev/ref/mod#go-get)
- Specify version with ``@<version>``
- Remove dependency with ``@none``
- *(Haskell)* stack doesn't seem to have a native solution
- *(Julia)* [pkg Add](https://docs.julialang.org/en/v1/stdlib/Pkg/)
- *(Ruby)* [bundle add](https://bundler.io/v2.2/man/bundle-add.1.html)
- Uses `--version` / `-v` instead of `--vers` (we use `--vers` because of `--version` / `-V`)
- `--source` instead of `path` (`path` correlates to manifest field)
- Uses `--git` / `--branch` like `cargo-add`
- *(Dart)* [pub add](https://dart.dev/tools/pub/cmd/pub-add)
- Uses `--git-url` instead of `--git`
- Uses `--git-ref` instead of `--branch`, `--tag`, `--rev`
### Future Possibilities
- Update lock file accordingly
- Exploring the idea of a [`--local` flag](https://github.com/killercup/cargo-edit/issues/590)
- Take the MSRV into account when automatically creating version req (https://github.com/killercup/cargo-edit/issues/587)
- Integrate rustsec to report advisories on new dependencies (https://github.com/killercup/cargo-edit/issues/512)
- Integrate with licensing to report license, block add, etc (e.g. https://github.com/killercup/cargo-edit/issues/386)
- Pull version from lock file (https://github.com/killercup/cargo-edit/issues/41)
- Exploring if any vendoring integration would be beneficial (currently errors)
- Upstream `cargo-rm` (#10520), `cargo-upgrade` (#10498), and `cargo-set-version` (in that order of priority)
- Update crates.io with `cargo add` snippets in addition to or replacing the manifest snippets
For more, see https://github.com/killercup/cargo-edit/issues?q=is%3Aissue+is%3Aopen+label%3Acargo-add
### How should we test and review this PR?
This is intentionally broken up into several commits to help reviewing
1. Import of production code from cargo-edit's `merge-add` branch, with only changes made to let it compile (e.g. fixing up of `use` statements).
2. Import of test code / snapshots. The only changes outside of the import were to add the `snapbox` dev-dependency and to `mod cargo_add` into the testsuite
3. This extends the work in #10425 so I could add back in the color highlighting I had to remove as part of switching `cargo-add` from direct termcolor calls to calling into `Shell`
Structure-wise, this is similar to other commands
- `bin` only defines a CLI and adapts it to an `AddOptions`
- `ops` contains a focused API with everything buried under it
The "op" contains a directory, instead of just a file, because of the amount of content. Currently, all editing code is contained in there. Most of this will be broken out and reused when other `cargo-edit` commands are added but holding off on that for now to separate out the editing API discussions from just getting the command in.
Within the github UI, I'd recommend looking at individual commits (and the `merge-add` branch if interested), skipping commit 2. Commit 2 would be easier to browse locally.
`cargo-add` is mostly covered by end-to-end tests written using `snapbox`, including error cases.
There is additional cleanup that would ideally happen that was excluded intentionally from this PR to keep it better scoped, including
- Consolidating environment variables for end-to-end tests of `cargo`
- Pulling out the editing API, as previously mentioned
- Where the editing API should live (`cargo::edit`?)
- Any more specific naming of types to reduce clashes (e.g. `Dependency` or `Manifest` being fairly generic).
- Possibly sharing `SourceId` creation between `cargo install` and `cargo edit`
- Explore using `snapbox` in more of cargo's tests
Implementation justifications:
- `dunce` and `pathdiff` dependencies: needed for taking paths relative to the user and make them relative to the manifest being edited
- `indexmap` dependency (already a transitive dependency): Useful for preserving uniqueness while preserving order, like with feature values
- `snapbox` dev-dependency: Originally it was used to make it easy to update tests as the UX changed in prep for merging but it had the added benefit of making some UX bugs easier to notice so they got fixed. Overall, I'd like to see it become the cargo-agnostic version of `cargo-test-support` so there is a larger impact when improvements are made
- `parse_feature` function: `CliFeatures` forces items through a `BTreeSet`, losing the users specified order which we wanted to preserve.
### Additional Information
See also [the internals thread](https://internals.rust-lang.org/t/feedback-on-cargo-add-before-its-merged/16024).
Fixes#5586
Add support for rustc --check-cfg well known names and values
This pull-request add support for `rustc` `--check-cfg` well known names and values.
### What does this PR try to resolve?
This pull-request add support for `rustc` `--check-cfg` well known names and values:
- `-Z check-cfg-well-known-names`: `--check-cfg names()` well known names
- `-Z check-cfg-well-known-values`: `--check-cfg values()` well known values
### How should we test and review this PR?
#### Testing
`cargo check -Z check-cfg-well-known-names` and
`cargo check -Z check-cfg-well-known-values`
#### Review
This PR contains one commit that split `features_args` into `check_cfg_args`, add the well known support in it, adds call to that new function and add documentation and test for those new flags.
### Additional information
This was implemented as two new additional flags because it's most likely that these flags will not be stabilize at the same time and also because some of these flags may become the default.
RFC: https://github.com/rust-lang/rfcs/pull/3013
`-Z check-cfg-features`: https://github.com/rust-lang/cargo/pull/10408
`cargo doc` support: https://github.com/rust-lang/cargo/pull/10428
Reserve filename `Cargo.toml.orig` in `cargo package`
### What does this PR try to resolve?
_([previously discussed on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Reserving.20.2FCargo.2Etoml.2Eorig))_
Currently, `cargo package` will detect if there is an existing file named `.cargo_vcs_info.json` in the project folder and raise an error. This is to avoid collisions with the file it generates of the same name.
However, there is no such logic for the other file it generates, `Cargo.toml.orig`. If such a file exists in the project folder, instead of an error, or one file taking precedence, cargo will generate a tarball containing two files with the same name. For example, from a package I uploaded as a test:
```sh
curl https://crates.io/api/v1/crates/a-/0.0.0--a-/download -L | gunzip | tar -tv
```
```text
-rw-r--r-- 0 0 0 8 29 Nov 1973 a--0.0.0--a-/.gitignore
-rw-r--r-- 0 0 0 150 31 Dec 1969 a--0.0.0--a-/Cargo.lock
-rw-r--r-- 0 0 0 641 31 Dec 1969 a--0.0.0--a-/Cargo.toml
-rw-r--r-- 0 0 0 160 29 Nov 1973 a--0.0.0--a-/Cargo.toml.orig <-- generated
-rw-r--r-- 0 0 0 14 29 Nov 1973 a--0.0.0--a-/Cargo.toml.orig <-- existing
-rw-r--r-- 0 0 0 45 29 Nov 1973 a--0.0.0--a-/src/main.rs
```
This PR is a two-line change to add this filename to the existing check for `.cargo_vcs_info.json`.
### How should we test and review this PR?
I have added a unit test to verify that the expected error is produced, copying the existing unit test for `.cargo_vcs_info.json`. I have manually tested the change locally and confirmed that it raises an error if the file exists, and has no effect if it does not. Given the small size of this change, I think this is sufficient, but please let me know if anything else is expected.
### Additional information
This raises a separate question of whether Cargo or crates.io should prohibit tarballs with duplicate filenames. It seems like most (all?) `tar` implementations will give precedence to the last file (which will be the existing copy here, not the generated one), but I don't believe this is specified behaviour, and it's possible that scripts scanning through tarballs directly (without first extracting to the filesystem) may not handle this case correctly. For example, I was working on a rough script to compare packaged libraries to their corresponding Git commits where possible, and this wasn't a case I had anticipated.
In any case, it seems like preventing such tarballs from being created by accident (this PR) is a good place to start.
Retry command invocation with argfile
### What does this PR try to resolve?
Fixes#10381
The goal is to invoke `rustc` and `rustdoc` with ``@path`` arg file as a fallback.
Since the `-Zcheck-cfg-features`[^1] has already been implemented, in the foreseeable future cargo will probably hit the “**command line too big**” more often on Windows. `rustdoc` and `rustc` both support ``@path`` argfile [^2][^3], so we can leverage the feature for the fallback logic.
The idea behind this PR is that we put the **retry logic** in `ProcessBuilder::exec*` functions and reuse them as much as possible.
### How should we test and review this PR?
Review it commit by commit is recommended.
Argfile fallback is hard to tested with integration tests, so I added some unit tests for `cargo_util::ProcessBuilder` and `cargo::ops::fix::FixArgs`.
If you do want to test the support of argfile manually, a new environment variable `__CARGO_TEST_FORCE_ARGFILE` is added for the sake of forcing cargo to use argfile for rustc invocations. For example, `__CARGO_TEST_FORCE_ARGFILE cargo test --test testsuite -- fix::` is usually a good start to test this feature.
[^1]: https://doc.rust-lang.org/beta/cargo/reference/unstable.html?highlight=check#check-cfg-features
[^2]: https://doc.rust-lang.org/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path
[^3]: https://doc.rust-lang.org/rustdoc/command-line-arguments.html#path-load-command-line-flags-from-a-path
Ensure host units don't depend on Docscrape units, fixes#10545
### What does this PR try to resolve?
This PR fixes issue #10545. The root cause was:
* Cargo workspace contains a host-type crate (e.g. a proc macro).
* `unit_dependencies` attempts to add `Docscrape` units as dependencies of the host-type crate.
* `activated_features` attempts to lookup `(dep, Host)` in the feature map, but only `(dep, NotHost)` exists.
The fix is to ensure that host-type crates do not have Docscrape units added as dependencies.
### How should we test and review this PR?
The added test `scrape_examples_issue_10545` provides a minimal example that fails before this fix, and succeeds after.
Don't error if no binaries were installed
### What does this PR try to resolve?
Fixes#10289, which contains a thorough description of the problem.
Briefly, if we interpret `cargo install` and `cargo install --bins` as "install
the binaries that are available", it should not be considered an error
if no binaries ended up being installed due to required features.
Instead, this should provide the user with a warning that this may not
have been what they intended.
### Additional information
Given that #9576 seems to have stalled, I figured I'd try to land this first [after all](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/cargo.20install.20--bins.20when.20no.20binaries.20match).
Use the correct flag in --locked --offline error message
### What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/10504
[Use the correct the flag in --locked --offline error message](17ec41515e)
[Add offline_and_locked_and_no_frozen test](7363f43d02)
### How should we test and review this PR?
- unit test
Don't treat host/target duplicates as duplicates
### What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/9519
Don't treat host/target duplicates as duplicates.
### How should we test and review this PR?
- Unit test.
- Create a manifest where a dependency shows up as both a normal dependency and a build-dependency. Run `cargo tree -d --target x86_64-unknown-linux-gnu`
Unstable --keep-going flag
## Summary
This PR adds an unstable `--keep-going` flag documented as follows:
> `cargo build --keep-going` (and similarly for `check`, `test` etc) will build as many crates in the dependency graph as possible, rather than aborting the build at the first one that fails to build.
>
> For example if the current package depends on dependencies `fails` and `works`, one of which fails to build, `cargo check -j1` may or may not build the one that succeeds (depending on which one of the two builds Cargo picked to run first), whereas `cargo check -j1 --keep-going` would definitely run both builds, even if the one run first fails.
>
> The `-Z unstable-options` command-line option must be used in order to use `--keep-going` while it is not yet stable:
>
> ```console
> cargo check --keep-going -Z unstable-options
> ```
## Prior art
[Buck](https://buck.build/) and [Bazel](https://bazel.build/) and Make all have this flag (though Bazel calls it `--keep_going` 🤮) with exactly this behavior.
## Motivation
I need this in order to make https://github.com/dtolnay/trybuild not super slow.
Trybuild wants to run Cargo on a bunch of test cases, each of which is a bin crate. The bad options currently available are:
- Give each test case its own target dir and run build on them in parallel. This is bad because all the test cases have the same dependencies in common (whatever `dev-dependencies` are declared by the project). If there are 100 test cases, all the dependencies would end up getting built 100 times, which is 100x slower than necessary despite the parallelism.
- Reuse a single target dir for all the test cases. Two Cargos can't operate in parallel on the same target directory, so this forces the test cases to be built serially. This is much slower than necessary on a many-core system, and compounds all of the overheads in Cargo because the project structure must be reloaded by each invocation.
The good option I'd like to switch to is:
- Run `cargo build --bins --keep-going --message-format=json` to build *all* the test cases in parallel. Use the filepaths in the JSON messages to ascribe diagnostics to which bin they're from.
If we interpret `cargo install` and `cargo install --bins` as "install
the binaries that are available", it should not be considered an error
if no binaries ended up being installed due to required features.
Instead, this should provide the user with a warning that this may not
have been what they intended.
Fixes#10289.
Part 1 of RFC2906 - Packages can inherit fields from their root workspace
Tracking issue: #8415
RFC: rust-lang/rfcs#2906
All changes were heavily inspired by #8664 and #9684
A big thanks to both `@yaymukund` and `@ParkMyCar.` I pulled a lot of inspiration from their branches.
I would also like to thank `@alexcrichton` for all the feedback on the first attempt. It has brought this into a much better state.
All changes have been made according to the RFC as well as `@alexcrichton's` [comment](https://github.com/rust-lang/cargo/pull/8664#issuecomment-704878103).
This is part 1 of many, as described by [this comment](https://github.com/rust-lang/cargo/pull/9684#issuecomment-943567692), [this comment](https://github.com/rust-lang/cargo/pull/9684#pullrequestreview-779070283) and redefined [by this one](https://github.com/rust-lang/cargo/pull/10497#issuecomment-1076301312).
This PR focuses on inheriting in root package, including:
- Add `MaybeWorkspace<T>` to allow for `{ workspace = true }`
- Add a new variant to `TomlDependency` to allow inheriting dependencies from a workspace
- Add `InheritableFields` so that we have somewhere to get a value from when we `resolve` a `MaybeWorkspace<T>`
- `license_file` and `readme` are in `InheritableFields` in this part but are not `MaybeWorkspace` for reasons [described here](https://github.com/rust-lang/cargo/pull/10497#issuecomment-1076470424)
- Add a method to `resolve` a `MaybeWorkspace<T>` into a `T` that can fail if we have nowhere to pull from or the workspace does not define that field
- Disallow inheriting from a different `Cargo.toml`
- This means that you can only inherit from inside the same `Cargo.toml`, i.e. it has both a `[workspace]` and a `[package]`
- Forcing this requirement allows us to test the implementations of `MaybeWorkspace<T>` and the new `TomlDependency` variant without having to add searching for a workspace root and the complexity with it
Remaining implementation work for the RFC
- Support inheriting in any workspace member
- Correctly inherit fields that are relative paths
- Including adding support for inheriting `license_file`, `readme`, and path-dependencies
- Path dependencies infer version directive
- Lock workspace dependencies and warn when unused
- Optimizations, as needed
- Evaluate any new fields for being inheritable (e.g. `rust-version`)
- Evaluate making users of `{ workspace = true }` define the workspace they pull from in `[package]`
Areas of concern:
- `TomlDependency` Deserialization is a mess, and I could not figure out how to do it in a cleaner way without significant headaches. I ended up having to do the same thing as last time [which was an issue](https://github.com/rust-lang/cargo/pull/9684#discussion_r728444103).
- Resolving on a `MaybeWorkspace` feels extremely verbose currently:
```rust
project.homepage.clone().map(|mw| mw.resolve(&features, "homepage", || get_ws(inheritable)?.homepage())).transpose()?,
```
This way allows for lazy resolution of inheritable fields and finding a workspace (in part 2) so you do not pay a penalty for not using this feature. The other bit of good news is `&features` will go away as it is just feature checking currently.
- This feature requires some level of duplication of code as well as remaking the original `TomlManifest` which adds extra length to the changes.
- This feature also takes a linear process and makes it potentially non-linear which adds lots of complexity (which will get worse in Part 2)
Please let me know if you have any feedback or suggestions!
HTTP registry implementation
Implement HTTP registry support described in [RFC 2789](https://github.com/rust-lang/rfcs/pull/2789).
Adds a new unstable flag `-Z http-registry` which allows cargo to interact with remote registries served over http rather than git. These registries can be identified by urls starting with `sparse+http://` or `sparse+https://`.
When fetching index metadata over http, cargo only downloads the metadata for needed crates, which can save significant time and bandwidth over git.
The format of the http index is identical to a checkout of a git-based index.
This change is based on `@jonhoo's` PR #8890.
cc `@Eh2406`
Remaining items:
- [x] Performance measurements
- [x] Make unstable only
- [x] Investigate unification of download system. Probably best done in separate change.
- [x] Unify registry tests (code duplication in `http_registry.rs`)
- [x] Use existing on-disk cache, rather than adding a new one.
-- `resolve` not takes `get_ws: impl FnOnce() -> CargoResult<T>`
-- removed `MaybeWorkspace` from `readme` and `license-file`
-- changed error messages and test names
Fix panic when artifact target is used for `[target.'cfg(<target>)'.dependencies`
With an artifact dependency like this in package `a`…
```toml
[dependencies.a]
path = "b"
artifact = "bin"
target = "$TARGET"
```
…and when using `$TARGET` like this in another package `b`…
```toml
[target.'cfg(target_arch = "$ARCHOF_$TARGET")'.dependencies]
c = { path = "../c" }
```
…it panics with `thread 'main' panicked at 'activated_features for invalid package: features did not find PackageId <dbg-info>`, but we would expect this to work normally.
### Tasks
- [x] reproduce issue in new test
- [x] figure out why the test is fixed but the real-world example isn't
- [x] find a fix
Fixes#10431 and #10452.
vendor: Don't allow multiple values for --sync
The --sync argument to cargo vendor currently takes a list, which makes it easy for it to eat the final path argument:
````
cargo vendor --manifest-path foo/Cargo.toml -s bar/Cargo.toml ./test_vendor/
error: failed to read ./test_vendor/
Caused by:
No such file or directory (os error 2)
````
Per discussion on #10441, this behavior is undesirable and hopefully used infrequently enough that we can change the UI for it. This patch will now only allow one value per --sync argument.
I didn't regenerate other doc files as it's not clear to me how/when that should be done.
Warning on conflicting keys
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
### What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/10299 and close https://github.com/rust-lang/cargo/issues/10317
### How should we test and review this PR?
- Warning on conflicting proc_macro and crate_types keys.
- Warning on conflicting dev-dependencies, build-dependencies, and default-features keys.
Registry functions return Poll to enable parallel fetching of index data
Adds `Poll` as a return type for several registry functions to enable parallel fetching of crate metadata with a future http-based registry.
Work is scheduled by calling the `query` and related functions, then waited on with `block_until_ready`.
This PR is based on the draft PR started by eh2406 here [#8985](https://github.com/rust-lang/cargo/pull/8985).
r? `@Eh2406`
cc `@alexcrichton`
cc `@jonhoo`
In order not to give up and create a basis for discussion while ending
my 3h oddisey on finding a fix for today, I present something that
seems to work even though I hope there are better ways to solve this.
This is a hacky way of making changes while leaving everything else,
despite it being deactivated for now, in the hopes to not forget
to put additional difficulties back in once this particular issue
is fixed.
updating, registries now update as needed. To force a registry to
ensure the latest copy of crate metadata is available, a new method
called `invalidate_cache` is added. This method will cause the registry
to update the next time `block_until_ready` is called.
Enable propagating host rustflags to build scripts
### What does this PR try to resolve?
This PR primarily fixes#10206, but in doing so it also slightly modifies the interface for the unstable `target-applies-to-host` feature (#9453), and adds the unstable `target-applies-to-host-kind` flag to mirror `target-applies-to-host` for build scripts and other host artifacts.
The commit messages have more in-depth discussion.
### How should we test and review this PR?
The test case from #10206 now works rather than producing an error. It has also been added a regression test case. A few additional test cases have also been added to handle the expected behavior around rustflags for build scripts with and without `target-applies-to-host-kind` enabled.
### Additional information
1. This changes the interface for `target-applies-to-host` so that it does not need to be specified twice to be used. And it can still be set through configuration files using the `[unstable]` table. However, we may(?) want to pick a stable format for in-file configuration of this setting unless we intend for it to only ever be a command-line flag.
2. It may be that `target-applies-to-host-kind` is never behavior we want users to turn on, and that it should therefore simply be removed and hard-coded as being `false`.
3. It's not entirely clear how `target-applies-to-host-kind` should interact with `-Zmultitarget`. If, for example, `requested_kinds = [HostTarget, SomeOtherTarget]` and `kind.is_host()`, should `RUSTFLAGS` take effect or not? For the time being I've just hard-coded the behavior for single targets, and the answer would be "no".
feat(search): Highlight search term
This supersedes #10116. For the requested colored-output tests, this followed the pattern of the `fix` tests which just detects whether colored output is used or not. The `cache_messages` actually verify the output is colored but that is because it can just compare to a rustc call's output. Getting the colored output correct by hand in a test (with all of the resets) is a bit messy and would be brittle.
This was done in an exercise in exploring ways to generalize colored output support in preparation for `cargo-add` doing some colored output as well.
I converted all output calls to use this approach, even if coloring wasn't used, for consistency. I considered coloring the overflow message but decided to hold off on that for now (either a warning-yellow or a hint-gray).
Fixes#9918
Add -Z check-cfg-features support for rustdoc
This PR is a follow to https://github.com/rust-lang/cargo/pull/10408 where support for compile-time checking of features was implemented for `rustc`.
At the time `rustdoc` support wasn't yet merged, but now that it has been [merged](https://github.com/rust-lang/rust/pull/94154), this pull-request add support for it in the `doc` and `test --doc` (doctest) mode.
r? `@alexcrichton`
Add common profile validation.
This fixes an oversight where `rustflags` is not nightly-gated in a profile override (like `[profile.dev.package.foo]`).
The problem is that the validation was only being done for the top-level profile.
The solution here is to consolidate common profile validation that should be done for both the top level and the overrides. In the process I also fixed validation of `codegen-backend` which also is shared. This will hopefully help prevent other oversights in the future.
Add -Z check-cfg-features to enable compile-time checking of features
This pull-request implements the "[Cargo support](https://rust-lang.github.io/rfcs/3013-conditional-compilation-checking.html#cargo-support)" section of [RFC 3013: Checking conditional compilation at compile time](https://rust-lang.github.io/rfcs/3013-conditional-compilation-checking.html#checking-conditional-compilation-at-compile-time).
The support is added in the form of an new unstable flags: `-Z check-cfg-features` that pass all possible features of a package to
`rustc` unstable `--check-cfg` command line as `--check-cfg=values(feature, ...)`. This enables compile time checking of `feature` values in `#[cfg]`, `cfg!` and `#[cfg_attr]`.
This new flag currently only affects `rustc` but `rustdoc` support will be added as soon as [it's support](https://github.com/rust-lang/rust/pull/94154) is merged.
Note than the intent is that this command line options become the default when stabilizing as suggested in the RFC:
> [..] it seems uncontroversial for Cargo to enable checking for feature = "..." values immediately [..]
Implement "artifact dependencies" (RFC-3028)
Tracking issue: #9096
#### Tasks
* [x] -Z unstable option called `bindeps`
* ✅ **(config)** allow parsing of newly introduced 'artifact' fields
* [x] into `TomlManifest`
* [x] into `Manifest`
- [x] ~~abort~~ warn if artifacts are used on stable
* ✅ **resolver** : assure artifact dependencies are part of the resolution process and unified into the dependency tree
* 🔬**compiler**: make it understand 'artifact' dependencies and pass new environment variables to the crate being build
* [x] `lib=false` should not be considered a rust library for the dependent, in unit and possibly resolve graph
* [x] assure profile settings are applied correctly
* [x] target overrides work
* [x] `target = "target"` in build deps
* [x] other targets on build deps
* [x] other targets on non-build deps
* [x] 'no-cross doc tests' seems like a constraint we should apply as well maybe
* [x] more confidence with `resolver = "2"`
* [x] assure artifact placement is correct (bin and various forms of lib)
* ✅ **serialization**: rewriting manifests (i.e. for publishing) does not discard artifact information
* [x] publishing keeps `artifact` and `lib` values
* **Other cargo subcommands**
* [x] `cargo metadata`
* leave unchanged
* [x] artifacts work with `cargo check`
* [x] artifacts work with rustdoc, such that it doesn't document them unless `lib=true`
* [x] `cargo tree` maybe?
* [x] `cargo clean` should clean artifacts - even though it's more complex, ultimately it deletes the `target` directory.
* [x] artifacts work with `cargo test` (and dev-dependencies)
* [x] doctests
* [x] try `reproducible` repository as well.
* 🧪 **tests** for more subtle RFC constraints
- [x] build scripts cannot access artifact environment variables at compile time, only at runtime)
- [x] assure 'examples' which also support crate-type fields like [[lib]] won't become artifacts themselves.
- [x] assure `--out-dir` does not leak artifacts - tested manually, it seemed to niche to add a test.
- [x] try `target="foo"` in artifact and assure it sees a decent error message
- [x] Assure RFC 3176 _doesn't_ work
* 🧹cleanup and finalization
- [x] assure no `TODO(ST)` markers are left in code
- [x] assure no tests are ignored
- [x] use `resolver = "1"` once to assert everything also works with the previous resolver, but leave it on "2".
#### Implementation and review notes
- artifacts and unstable options are only checked when transforming them from `TomlManifest` to `Manifest`, discarding artifact information if the unstable flag is not set. Nowhere else in code will the CLI options be checked again.
- `If no binaries are specified, all the binaries in the package will be built and made available.` - this should only refer to `[[bin]]` targets, not examples for instance.
- artifact binaries won't be uplifted, hence won't be present outside of their output directory
- ❗️We don't know how [package links](00e925f61f/src/cargo/core/compiler/unit_dependencies.rs (L380)) will affect artifacts for build dependencies. Should probably be thought through.
- ❗️The location of artifacts is only tested roughly to avoid having to deal with different output names on the four platforms that seem to matter (gnu, macos, windows msvc, windows gnu).
- `cargo tree` doesn't handle artifacts specifically, and it might be interesting to make clear if an artifact is only an artifact, or both artifact and dependency.
- Most error and warning messages can probably be more cargo-matic.
#### Questions
* Does `cargo` without the feature enabled have to complain about the "artifact" field in a dependency, like it does right now? It doesn't look like machinery for that exists in `do_read_manifest()`.
- ✔️It warns now
* Should parsing of artifact values, like "bin" be case sensitive?
- ✔️ It's case sensitive now, which should help with serde roundtripping.
#### Review Progress
* [x] address Josh's review notes one by one
* [x] introduce `IsArtifact` (see [this answer](https://github.com/rust-lang/cargo/pull/9992#discussion_r774928102)) (76cd48a2d62d74e043a1a482199c5bb920f50311)
* [x] prefer uplifting artifact deps that were written into the `deps` directory, but prefer to do that in [this PR instead](https://github.com/rust-lang/cargo/pull/9992)
* [x] add extra-tests as described in Josh's comment: " features get unified between a Rust library and a binary, and one that confirms features don't get unified between a Rust library and a binary for a different target?"
* [x] Make target-based artifact splitting work by porting parts of RFC-3176
* [x] test-support/cross-compile
* [x] namespace separation
* [x] re-create RFC-3176 what's not in RFC-3028, namely multidep support and all related tests
* [x] Address Eh2406 's review comments
* [x] Address Eric's comments
* [x] Unstable features need to be documented in unstable.md.
* [x] sort out [`target_data`](https://github.com/rust-lang/cargo/pull/9992#discussion_r787316229)
* [x] figure out [cargo metadata](https://github.com/rust-lang/cargo/pull/9992#discussion_r787320923)
* [x] See if target-data can work without an [index format update](https://github.com/rust-lang/cargo/pull/9992#issuecomment-1021697124).
* [x] truncate comments at 80-90 lines and remove unused methods, remove -Z unstable-options, use `cfg!(target_env = "msvc")`
* [x] add missing doc comments to newly added methods and funtions
* [x] simplify method parameters and inline some functions
* [x] add test and extend test to show additional issues
* [x] assure current set of tests works consistently, also on windows
Sponsored by [Profian](https://www.profian.com)
Tracking issue: https://github.com/rust-lang/cargo/issues/9096
Original PR: https://github.com/rust-lang/cargo/pull/9992
Add 'bindeps' -Z flag for later use
A test to validate artifact dependencies aren't currently parsed.
Parse 'artifact' and 'lib' fields.
Note that this isn't behind a feature toggle so 'unused' messages will
disappear.
Transfer artifact dependencies from toml- into manifest-dependencies
There are a few premises governing the operation.
- if unstable features are not set, warn when 'artifact' or 'lib' is
encountered.
- bail if 'lib' is encountered alone, but warn that this WOULD happen
with nightly.
- artifact parsing checks for all invariants, but some aren't tested.
Assure serialization of 'artifact' and 'lib' fields produces suitable values during publishing
This should be the only place were these fields matter and where a cargo
manifest is actually produced. These are only for internal use, no user
is typically going to see or edit them.
Place all artifact dependency tests inta their own module
This facilitates deduplication later and possibly redistribution into
other modules if there is a better fit.
Represent artifacts that are rust libraries as another ArtifactKind
This is more consistent and probably simpler for later use.
No need to reflect the TOML data structure.
Add tests to assure only 'lib = true' artifact deps are documented
RFC-3028 doesn't talk about documentation, but for lib=true it's clear
what the desired behaviour should be.
If an artifact isn't a library though, then for now, it's transparent,
maybe.
Many more tests, more documentation, mild `Artifact` refactor
The latter seems to be a better fit for what being an artifact
really means within cargo, as it literally turns being a library
on or off, and thus only optionally becoming a normal library.
refactor to prepare for artifact related checks
Don't show a no-lib warning for artifact dependencies (with lib = false)
Tests for more artifact dependency invariants
These are merely a proof of concept to show that we are not in
a position to actually figure out everything about artifacts
right after resolution.
However, the error message looks more like a fatal error and less
like something that can happen with a more elaborate error message
with causes.
This might show that these kind of checks might be better done later
right before trying to use the information for create compile units.
Validate that artifact deps with lib=true still trigger no-lib warnings
This triggers the same warning as before, for now without any
customization to indicate it's an artifact dependency.
Use warnings instead of errors
------------------------------
This avoids the kind of harsh end of compilation in favor of something
that can be recovered from. Since warnings are annoying, users will
probably avoid re-declaring artifact dependencies.
Hook in artifact dependencies into build script runs
Even though we would still have to see what happens if they have a lib
as well. Is it built twice?
Also
----
- fly-by refactor: fix typo; use ? in method returning option
- Propagate artifact information into Units; put artifacts into place
This means artifacts now have their own place in the 'artifact'
directory and uplifts won't happen for them.
- refactor and fix cippy suggestion
- fix build after rebasing onto master
Create directories when executing the job, and not when preparing it.
also: Get CI to work on windows the easy way, for now.
Set directories for artifact dependencies in build script runtimes
Test remaining kinds of build-script runtime environment variables
Also
----
- Fix windows tests, the quick way.
- Try to fix windows assertions, and generalize them
- Fix second test for windows, hopefully
test for available library dependency in build scripts with lib = true
probably generally exclude all artifact dependencies with lib=false.
Pass renamed dep names along with unit deps to allow proper artifact env names
Test for selective bin:<name> syntax, as well as binaries with dashes
Test to assure dependency names are transformed correctly
assure advertised binaries and directories are actually present
This wouldn't be the case if dependencies are not setup correctly,
for instance.
Also
----
- make it easier to see actual values even on failure
This should help figure out why on CI something fails that works
locally no matter what.
Turns out this is a race condition, with my machine being on the good
side of it so it doesn't show in testing. Fortunately it still can be
reproduced and easily tested for.
- refactor test; the race condition is still present though
- Force CI to pass here by avoiding checks triggering race.
- Fix windows build, maybe?
More tolerant is_file() checks to account for delay on CI
This _should_ help CI to test for the presence which is better than
not testing at all.
This appears to be needed as the output file isn't ready/present in time
for some reason.
The root cause of this issue is unknown, but it's definitely a race
as it rarely happens locally. When it happened, the file was always
present after the run.
Now we will learn if it is truly not present, ever, or if it's maybe
something very else.
Validate libs also don't see artifact dependencies as libraries with lib=false
Also
----
- Add prelimiary test for validating build-time artifacts
- Try to fix CI on gnu windows
Which apparently generates paths similar to linux, but with .exe suffix.
The current linux patterns should match that.
- refactor
Help sharing code across modules
allow rustc to use artifact dep environment variables, but…
…it needs some adjustments to actually setup the unit dependency graph
with artifacts as well.
Right now it will only setup dependencies for artifacts that are libs,
but not the artifacts themselves, completely ignoring them when they
are not libs.
Make artifact dependencies available in main loop
This is the commit message #2:
------------------------------
rough cut of support for artifact dependencies at build time…
…which unfortunately already shows that the binary it is supposed to
include is reproducibly not ready in time even though the path is
correct and it's present right after the run.
Could it be related to rmeta?
This is the commit message #3:
------------------------------
Fix test expectations as failure is typical than the warning we had before…
…and add some tolerance to existing test to avoid occasional failures.
This doesn't change the issue that it also doens't work at all for
libraries, which is nicely reproducable and hopefully helps to fix
this issue.
This is the commit message #4:
------------------------------
Probably the fix for the dependency issue in the scheduler
This means that bin() targets are now properly added to the job graph
to cause proper syncing, whereas previously apparently it would
still schedule binaries, but somehow consider them rmeta and thus
start their dependents too early, leading to races.
This is the commit message #5:
------------------------------
Don't accidentally include non-gnu windows tests in gnu windows.
Support cargo doc and cargo check
The major changes here are…
- always compile artifacts in build mode, as we literally want the
build output, always, which the dependent might rely on being present.
- share code between the rather similar looking paths for rustdoc and
rustc.
Make artifact messages appear more in line with cargo by using backticks
Also: Add first test for static lib support in build scripts
build-scripts with support for cdylib and staticlib
- Fix windows msvc build
No need to speculate why the staticlib has hashes in the name even
though nothing else.
staticlib and cdylib support for libraries
test staticlib and cdylibs for rustdoc as well.
Also catch a seemingly untested special case/warning about the lack
of linkable items, which probably shouldn't be an issue for artifacts
as they are not linkable in the traditional sense.
more useful test for 'cargo check'
`cargo check` isn't used very consistently in tests, so when we use it
we should be sure to actually try to use an artifact based feature
to gain some coverage.
verify that multiple versions are allowed for artifact deps as well.
also: remove redundant test
This is the commit message #2:
------------------------------
Properly choose which dependencies take part in artifact handling
Previously it would include them very generously without considering
the compatible dependency types.
This is the commit message #3:
------------------------------
a more complex test which includes dev-dependencies
It also shows that doc-tests don't yet work as rustdoc is run outside of
the system into which we integrate right now.
It should be possible to write our environment variable configuration
in terms of this 'finished compilation' though, hopefully with
most code reused.
This is the commit message #4:
------------------------------
A first stab at storing artifact environment variables for packages…
…however, it seems like the key for this isn't necessarily correct
under all circumstances. Maybe it should be something more specific,
don't know.
This is the commit message #5:
------------------------------
Adjust key for identifying units to Metadata
This one is actually unique and feels much better.
This is the commit message #6:
------------------------------
Attempt to make use of artifact environment information…
…but fail as the metadata won't match as the doctest unit is, of course,
its separate unit. Now I wonder if its possible to find the artifact
units in question that have the metadata.
Properly use metadata to use artifact environment variables in doctests
This is the commit message #2:
------------------------------
Add test for resolver = "2" and build dependencies
Interestingly the 'host-features' flag must be set (as is seemingly
documented in the flags documentation as well), even though I am not
quite sure if this is the 100% correct solution. Should it rather
have an entry with this flag being false in its map? Probably not…
but I am not quite certain.
This is the commit message #3:
------------------------------
set most if not all tests to use resolver = "2"
This allows to keep it working with the most recent version while
allowing to quickly test with "1" as well (which thus far was working
fine).
All tests I could imagine (excluding target and profiles) are working now
Crossplatform tests now run on architecture aarm64 as well.
More stringent negative testing
Fix incorrect handling of dependency directory computation
Previously it would just 'hack' the deps-dir to become something very
different for artifacts.
This could easily be fixed by putting the logic for artifact output
directories into the right spot.
A test for cargo-tree to indicate artifacts aren't handled specifically
Assure build-scripts can't access artifacts at build time
Actual doc-tests with access to artifact env vars
All relevant parsing of `target = [..]`
Next step is to actually take it into consideration.
A failing test for adjusting the target for build script artifacts using --target
Check for unknown artifact target triple in a place that exists for a year
The first test showing that `target="target"` deps seemingly work
For now only tested for build scripts, but it won't be much different
for non-build dependencies.
build scripts accept custom targets unconditionally
Support target setting for non-build dependencies
This is the commit message #2:
------------------------------
Add doc-test cross compile related test
Even though there is no artifact code specific to doc testing, it's
worth to try testing it with different target settings to validate
it still works despite doc tests having some special caseing around
target settings.
This is the commit message #3:
------------------------------
A test to validate profiles work as expected for build-deps and non-build deps
No change is required to make this work and artifact dependencies 'just work'
based on the typical rules of their non-artifact counterarts.
This is the commit message #4:
------------------------------
Adjust `cargo metadata` to deal with artifact dependencies
This commit was squashed and there is probably more that changed.
This is the commit message #5:
------------------------------
Show bin-only artifacts in "resolve" of metadata as well.
This is the commit message #6:
------------------------------
minor refactoring during research for RFC-3176
This will soon need to return multiple extern-name/dep-name pairs.
This is the commit message #7:
------------------------------
See if opt-level 3 works on win-msvc in basic profile test for artifacts
This is the same value as is used in the other test of the same name,
which certainly runs on windows.
This is the commit message #8:
------------------------------
refactor
Assure the type for targets reflect that they cannot be the host target,
which removes a few unreachable!() expressions.
Put `root_unit_compile_kind` into `UnitFor`
Previously that wasn't done because of the unused `all_values()`
method which has now been deleted as its not being used anyomre.
This allows for the root unit compile kind to be passed as originally
intended, instead of working around the previous lack of extendability
of UnitFor due to ::all_values().
This is also the basis for better/correct feature handling once
feature resolution can be depending on the artifact target as well,
resulting in another extension to UnitFor for that matter.
Also
----
- Fix ordering
Previously the re-created target_mode was used due to the reordering
in code, and who knows what kind of effects that might have
(despite the test suite being OK with it).
Let's put it back in place.
- Deactivate test with filename collision on MSVC until RFC-3176 lands
Avoid clashes with binaries called 'artifact' by putting 'artifact/' into './deps/'
This commit addresses review comment https://github.com/rust-lang/cargo/pull/9992#discussion_r772939834
Don't rely on operator precedence for boolean operations
Now it should be clear that no matter what the first term is,
if the unit is an artifact, we should enqueue it.
Replace boolean and `/*artifact*/ <bool>` with `IsArtifact::(Yes/No)`
fix `doc::doc_lib_false()` test
It broke due to major breakage in the way dependencies are calculated.
Now we differentiate between deps computation for docs and for building.
Avoid testing for doctest cross-compilation message
It seems to be present on my machine, but isn't on linux and it's
probably better to leave it out entirely and focus on the portions
of consecutive output that we want to see at least.
A test to validate features are unified across libraries and those in artifact deps in the same target
Allow aarch64 MacOS to crosscompile to an easily executable alternative target
That way more tests can run locally.
Support for feature resolution per target
The implementation is taken directly from RFC-3176 and notably lacks
the 'multidep' part.
Doing this definitely has the benefit of making entirely clear
'what is what' and helps to greatly reduce the scope of RFC-3176
when it's rebuilt based on the latest RF-3028, what we are implementing
right now.
Also
----
- A test which prooves that artifact deps with different target don't have a feature namespace yet
- Add a test to validate features are namespaced by target
Previously it didn't work because it relies on resolver = "2".
- 'cargo metadata' test to see how artifact-deps are presented
- Missed an opportunity for using the newly introduced `PackageFeaturesKey`
- Use a HashMap to store name->value relations for artifact environment variables
This is semantically closer to what's intended.
also: Remove a by now misleading comment
Prevent resolver crash if `target = "target"` is encountered in non-build dependencies
A warning was emitted before, now we also apply a fix.
Previously the test didn't fail as it accidentally used the old
resolver, which now has been removed.
Abort in parsing stage if nightly flag is not set and 'artifact' is used
There is no good reason to delay errors to a later stage when code
tries to use artifacts via environment variables which are not present.
Change wording of warning message into what's expected for an error message
remove unnecessary `Result` in `collect()` call
Improve logic to warn if dependencie are ignored due to missing libraries
The improvement here is to trigger correctly if any dependency of a
crate is potentially a library, without having an actual library target
as part of the package specification.
Due to artifact dependencies it's also possible to have a dependency
to the same crate of the same version, hence the package name
isn't necessarily a unique name anymore. Now the name of the actual
dependency in the toml file is used to alleviate this.
Various small changes for readability and consistency
A failing test to validate artifacts work in published crates as well
Originally this should have been a test to see target acquisition works
but this more pressing issue surfaced instead.
Make artifacts known to the registry data (backwards compatible)
Now artifacts are serialized into the registry on publish (at least
if this code is actually used in the real crates-io registry) which
allows the resolve stage to contain artifact information.
This seems to be in line with the idea to provide cargo with all
information it needs to do package resolution without downloading
the actual manifest.
Pick up all artifact targets into target info once resolve data is available
Even though this works in the test at hand, it clearly shows there
is a cyclic dependency between the resolve and the target data.
In theory, one would have to repeat resolution until it settles
while avoiding cycles.
Maybe there is a better way.
Add `bindeps`/artifact dependencies to `unstsable.md` with examples
Fix tests
Various small improvements
Greatly simplify artifact environment propagation to commands
Remove all adjustments to cargo-metadata, but leave tests
The tests are to record the status quo with the current code
when artifact dependencies are present and assure the information
is not entirely non-sensical.
Revert "Make artifacts known to the registry data (backwards compatible)"
This reverts commit adc5f8ad04840af9fd06c964cfcdffb8c30769b0.
Ideally we are able to make it work without altering the registry
storage format. This could work if information from the package
set is added to the resolve information.
Enrich resolves information with additional information from downloaded manifests
Resolve information comes from the registry, and it's only as rich as
needed to know which packages take part in the build.
Artifacts, however, don't influence dependency resolution, hence it
shouldn't be part of it.
For artifact information being present nonetheless when it matters,
we port it back to the resolve graph where it will be needed later.
Collect 'forced-target' information from non-workspace members as well
This is needed as these targets aren't present in the registry and
thus can't be picked up by traversing non-workspce members.
The mechanism used to pick up artifact targets can also be used
to pick up these targets.
Remove unnecessary adjustment of doc test
refactor `State::deps()` to have filter; re-enable accidentally disabled test
The initial rebasing started out with a separted `deps_filtered()`
method to retain the original capabilities while minimizing the chance
for surprises. It turned out that the all changes combined in this PR
make heavy use of filtering capabilities to the point where
`deps(<without filter>)` was unused. This suggested that it's required
to keep it as is without a way to inline portions of it.
For the original change that triggered this rebase, see
bd45ac81ba
The fix originally made was reapplied by allowing to re-use the
required filter, but without inlining it.
Always error on invalid artifact setup, with or without enabled bindeps feature
Clarify how critical resolver code around artifact is working
Remove workaround in favor of deferring a proper implementation
See https://github.com/rust-lang/cargo/pull/9992#issuecomment-1033394197
for reference and the TODO in the ignored test for more information.
truncate comments at 80-90c; cleanup
- remove unused method
- remove '-Z unstable-options'
- improve error message
- improve the way MSVC special cases are targetted in tests
- improve how executables are found on non MSVC
Avoid depending on output of rustc
There is cyclic dependency between rustc and cargo which makes it
impossible to adjust cargo's expectations on rustc without leaving
broken commits in rustc and cargo.
Add missing documentation
fix incorrect removal of non-artifact libs
This is also the first step towards cleaning up the filtering logic
which is still making some logic harder to understand than needs be.
The goal is to get it to be closer to what's currently on master.
Another test was added to have more safety regarding the overall
library inclusion logic.
inline `build_artifact_requirements_to_units()`
Simplify filtering
This adds a default filter to `state.deps(…)` making it similar to
what's currently in master, while creating another version of it
to allow setting a custom filter. This is needed as the default filter
won't allow build dependencies, which we need in this particular case.
`calc_artifact_deps(…)` now hard-codes the default filter which is
needed due to the use of `any` here:
c0e6abe384/src/cargo/core/compiler/unit_dependencies.rs (L1119)
.
Simplify filtering.
This patch implements more complete logic for applying rustflags to
build scripts and other host artifacts. In the default configuration, it
only makes build scripts (and plugins and whatnot) pick up on
`rustflags` from `[host]`, which fixes#10206 but otherwise preserves
existing behavior in all its inconsistent glory. The same is the case if
`target-applies-to-host` is explicitly set to `false`.
When `target-applies-to-host` is explicitly set to `true`, rustflags
will start to be applied in the same way that `linker` and `runner` are
today -- namely they'll be applied from `[target.<host triple>]` and
from `RUSTFLAGS`/`build.rustflags` if `--target <host triple>` is given.
The tests in question all tail to even build the build script, so the
contents of `main` don't matter, and make the tests seem more complex
than they really are.
A new test `build::no_dep_info_collision_when_cdylib_and_bin_coexist`
is added to prevent regression.
Note that on Windows MSVC cargo does not use metadata, so a collision
still happens when cdylib and bin targets coexist.
Stabilize `-Ztimings` as `--timings`
The `-Ztimings` option has existed for years, and many people use it to
profile and optimize their builds. It's one of the common reasons people
use nightly cargo.
The machine-readable JSON output may warrant further careful inspection
before we commit to a stable format. However, for the human-readable
output we don't need to make any commitment about the exact output.
Add a `--timings` option, as the stable equivalent to `-Ztimings`.
(Passing `html` or `json` requires `-Zunstable-options`, but the default `--timings` does not.)
Document the new option, and update the testsuite.
fix(install): Keep v1 file formatting the same
Inspired by #10345, I looked for other cases where
`toml_edit::easy::to_string` is used (which outputs inline tables) to
see if we should switch to `to_string_pretty`. The crates v1 file was
the only case I found.
As a side effect, we can no longer elide the empty `dev-dependencies`
table in published manifests. This was the behavior before `toml_edit`,
so not much of a loss.
Do not ignore `--features` when `--all-features` is present
This allows to specify dependency features when using `--all-features`, for example:
```shell
$ cargo run --example example --all-features --features="tracing/log"
```
You can test this in this repository: https://github.com/WaffleLapkin/cargo_all_some_features, it contains an example with ``required-features = [..., "tracing/log"]`` that is impossible to run with `--all-features` without this patch.
An attempt to fix#10333
`toml_edit` fixed a bug in 0.13.4 that this test was relying on that is
meant to help with rust-lang/cargo#10349. This basically restores us
back to the pre-toml_edit behavior for published manifests.
Inspired by #10345, I looked for other cases where
`toml_edit::easy::to_string` is used (which outputs inline tables) to
see if we should switch to `to_string_pretty`. The crates v1 file was
the only case I found.
As a side effect, we can no longer elide the empty `dev-dependencies`
table in published manifests. This was the behavior before `toml_edit`,
so not much of a loss.
This avoids dropping them wherever Cargo happens to run from, and
instead places them under the target directory.
This has the advantage, and disadvantage, that `cargo clean` will remove
them.
The `-Ztimings` option has existed for years, and many people use it to
profile and optimize their builds. It's one of the common reasons people
use nightly cargo.
The machine-readable JSON output may warrant further careful inspection
before we commit to a stable format. However, for the human-readable
output we don't need to make any commitment about the exact output.
Add a `--timings` option, as the stable equivalent to `-Ztimings`.
Stabilize the `html` output format, and require `-Zunstable-options` for
the `json` output format.
Document the new option, and update the testsuite.
The text-based timing information emits many additional lines, creating
quite a bit of verbosity. Remove in favor of the HTML report, as
suggested at
https://github.com/rust-lang/cargo/pull/10245#issuecomment-1003741052 .
If we re-add text-based timing information in the future, it could come
in the form of a text-based report, or as a duration printed on the same
line as the crate it measures rather than a separate line.
extra-link-arg-etc: support all link types (credit @davidhewitt)
This commit adds support for the remaining link types to `-Zextra-link-arg-etc`:
`
rustc-link-arg-tests
rustc-link-arg-benches
rustc-link-arg-examples
`
This would be useful in PyO3, where users writing Python extension modules (which do link against libpython) want to run cargo tests for extension module. As executables, these tests need to link against libpython.
This is a follow up of https://github.com/rust-lang/cargo/pull/9416 by `@davidhewitt`
This allows to specify dependency features when using `--all-features`,
for example:
```shell
$ cargo run --package a --example example --all-features --features="tracing/log"
```
do not compile test for bins flagged as `test = false`
### What does this PR try to resolve?
Fixes#10268#6683 introduced a behavior that compiles all bin targets, but for bins with `test = false` they shouldn't be compiled with `--test` as testbins.
### How should we test and review this PR?
In the first commit of this PR, I refines the test `test_filtered_excludes_compiling_examples` to reflect the current wrong behavior (test passed). The following two commits correct the behavior and the test accordingly. The last few commits encapsulate scattered target selection logic into functions on `CompileFilter`.
Port cargo from toml-rs to toml_edit
Benefits:
- A TOML 1.0 compliant parser
- Unblock future work
- Have `cargo init` add the current crate to the workspace, rather
than error
- #5586: Upstream `cargo-add`
TODO
- [x] Analyze performance and address regressions
- [x] Identify and resolve incompatibiies
- [x] Resolve remaining test failures, see
https://github.com/ordian/toml_edit/labels/cargo
- [x] ~~Switch the code from https://github.com/rust-lang/cargo/pull/10176 to only parse once~~ (this PR is being merged first)
The test new::git_default_branch would fail if the current user had
already configured a different default branch.
This patch changes the test to first write a .gitconfig file with the
default branch set to master. This puts us in a state where we still
have the old default, and then the subsequent change to the config file
will make sure that config changes are still respected.
Error when setting crate type of both dylib and cdylib in library
close https://github.com/rust-lang/cargo/issues/10231
Error when setting crate type of both dylib and cdylib in library. Cargo can't support that at this time, since they both output the same filename.
Benefits:
- A TOML 1.0 compliant parser
- Unblock future work
- Have `cargo init` add the current crate to the workspace, rather
than error
- #5586: Upstream `cargo-add`
Stabilize namespaced and weak dependency features.
This stabilizes the namespaced and weak dependency features. Support is now enabled on crates.io, so this should be ready to go.
As a part of this change, the new feature resolver is now enabled all of the time. This is fairly risky, since there are likely edge cases that haven't been exercised.
NOTE: Projects using `resolver="1"` *should* continue to have the same behavior, the old resolver behavior is emulated.
Closes#8813Closes#8832
Port cargo to clap3
### What does this PR try to resolve?
This moves cargo to the latest major version of clap.
This supersedes #10259 and #10262
### How should we test and review this PR?
For testing, I mostly relied on existing tests. I did manually validate that `cargo run <non-escaped command args>` behaved the same between both
```console
$ cargo run release --help
Finished dev [unoptimized + debuginfo] target(s) in 0.22s
Running `target/debug/cargo-release release --help`
cargo-release 0.18.8
...
$ cargo run --manifest-path ../cargo/Cargo.toml -- run release --help
Finished dev [unoptimized + debuginfo] target(s) in 0.05s
Running `/home/epage/src/personal/cargo/target/debug/cargo run release --help`
Finished dev [unoptimized + debuginfo] target(s) in 1.31s
Running `target/debug/cargo-release release --help`
cargo-release 0.18.8
...
```
For reviewing, I split out each deprecation resolution into a separate commit so its easy to focus on more mechanical changes (most of the deprecation fixes) from interesting ones (the port, the `Arg::multiple` deprecation)
### Additional information
- One parser change found by `cargo_config::includes` is that clap 2
would ignore any values after a `=` for flags.
`cargo config --show-origin` is a flag but the test passed `--show-origin=yes` which
happens to give the desired result for that test but is the same as
`--show-origin=no` or `--show-origin=alien-invasion`.
- `ArgMatches` now panics when accessing an undefined argument but clap
takes advantage of that for sharing code across commands that have
different subsets of arguments defined. I've extended clap so we can
"look before you leap" and put the checks at the argument calls to
start off with so its very clear what is tenuously shared. This
allows us to go in either direction in the future, either addressing
how we are sharing between commands or by moving this down into the
extension methods and pretending this clap feature doesn't exist
- On that topic, a test found clap-rs/clap#3263. For now, there is a
hack in clap. Depending on how we fix that in clap for clap 4.0, we
might need to re-address things in cargo.
- `value_of_os` now requires setting `allow_invalid_utf8`, otherwise it
asserts. To help catch this, I updated the argument definitions
associated with lookups reported by:
- `rg 'values?_os' src/`
- `rg 'values?_of_os' src/`
- clap now reports `2` for usage errors, so we had to bypass clap's
`exit` call to keep the same exit code.
- `cargo vendor --sync` did not use `multi_opt` and so it has both
multiple occurrences **and** multiple values. If we want to deprecate
this, we'll need `unstable-grouped` to be stablized (or pin our clap
version) and ensure each group has only 1 value.
- One parser change found by `cargo_config::includes` is that clap 2
would ignore any values after a `=` for flags.
`cargo config --show-origin` is a flag but the test passed `--show-origin=yes` which
happens to give the desired result for that test but is the same as
`--show-origin=no` or `--show-origin=alien-invasion`.
- The parser now panics when accessing an undefined attribute but clap
takes advantage of that for sharing code across commands that have
different subsets of arguments defined. I've extended clap so we can
"look before you leap" and put the checks at the argument calls to
start off with so its very clear what is tenuously shared. This
allows us to go in either direction in the future, either addressing
how we are sharing between commands or by moving this down into the
extension methods and pretending this clap feature doesn't exist
- On that topic, a test found clap-rs/clap#3263. For now, there is a
hack in clap. Depending on how we fix that in clap for clap 4.0, we
might need to re-address things in cargo.
- `value_of_os` now requires setting `allow_invalid_utf8`, otherwise it
asserts. To help catch this, I updated the argument definitions
associated with lookups reported by:
- `rg 'values?_os' src/`
- `rg 'values?_of_os' src/`
- clap now reports `2` for usage errors, so we had to bypass clap's
`exit` call to keep the same exit code.
BREAKING CHANGE: API now uses clap3
Be resilient to most IO error and filesystem loop while walking dirs
Let `PathSource::walk` be resilient to most IO errors and filesystem loop.
This PR also
- Add a test validating the resilience against filesystem loop to prevent regression.
- Emit warning when filesystem loop found while walking the filesystem. This is the only way I can think of now to solve #9528Fixes#10213.
Cargo has had pipelining enabled by default for a long time, without
issue. Remove support for `build.pipelining = false`. (Continue parsing
the option from the config file, but ignore it.)
Remove `build_script::build_script_scan_eacces` test case because cargo
ignores it and returns its path during a `cargo build`. The caller still
has a chance to hit the IO error if they does access it.
Stabilize the `strip` profile option, now that rustc has stable `-C strip`
https://github.com/rust-lang/rust/pull/90058/ stabilized `-Z strip` as `-C strip`. Stabilize the corresponding Cargo option.
Update the documentation to move it from the unstable reference to the profiles documentation.
Update the tests to account for stabilization.
https://github.com/rust-lang/rust/pull/90058/ stabilized `-Z strip` as
`-C strip`. Stabilize the corresponding Cargo option.
Update the documentation to move it from the unstable reference to the
profiles documentation.
Update the tests to account for stabilization, but make them no-ops on
stable/beta for now until rustc 1.58 releases.
When typing in a single character shortcut as a capital, it always
returns `b` as the suggestion as every one-letter abbreviation
is a lev distance 1 away from the capitalized one.
By making the levenshtein distance case insensitive, the case-mismatched
one-letter abbriviation (e.g. `C` to `c`) will be suggested, rather
than `b`
Detect filesystem loop during walking the projects
Resolves#9528
~~This PR also adds a new dependency `same-file` but since it's already a
dependency of `cargo-util`, so nothing added actually.~~
Use `walkdir` to detect filesystem loop and gain performance boost!
Support `term.quiet` configuration
Fixes#10128
This follows the existing support for `--verbose` and `term.verbose`.
I've renamed the related tests to be a bit clearer now there are more cases, and the existing quiet tests now prove that they hide the cargo log.
I'm unsure whether I'm supposed to regenerate the documentation as part of this?
Display alias target on 'cargo help <alias>`
```
Previously, `cargo help <alias>` resolved the alias and displayed the
help for the targeted subcommand. For example, if `br` were aliased to
`build --release`, `cargo help br` would display the manpage for
cargo-build.
With this patch, it will print "'br' is aliased to 'build --release'".
```
Addresses issue #10138.
This is my first patch to Cargo. I attempted to follow the style of the surrounding code. Please let me know if any changes are required: happy to make them. In particular, I wasn't sure if any tests exist for this path.
delete --host command and message
close https://github.com/rust-lang/cargo/issues/10121
The warning for this command has been around for a long time, and it seems safe to delete it now.
According to Alex's words in the issue, I think he supports deletion. I'm sorry if I misunderstood it and feel free to close it.
Improve I/O error message for fingerprint of build script
It is a bit rough but I don't think there is a network I/O error
in `pkg_fingerprint`. Checking only `io::Error` type should be fine.
Resolves#9881
Explicitly mark aliases in `cargo list`.
This I think makes it a little clearer that these are aliases, and matches the form used for built-in aliases.
Don't emit "executable" JSON field for non-executables.
The "executable" field of JSON artifact messages was accidentally filled (with the path to `index.html`) when documenting a binary target. This fixes it so that it is null.
Closes#10149
Document lib before bin.
This changes it so that documenting a library is required to finish before documenting a binary. The issue is that the binary may have intra-doc links to the library. If they are documented concurrently, then the links will sometimes fail (since it is a race). Or, if doing `cargo doc --bins`, then the library docs wouldn't exist at all.
Note that in the tests this introduces some more name collisions if you just run `cargo doc --bins` and there is a colliding library/binary name. There is some risk that someone might be trying to run the commands separately to get around the collision error, but I think it is unlikely.
This changes the behavior so that simple aliases that directly alias a
subcommand (with no arguments) pass-through to that subcommand, while
complex aliases (with arguments) show the alias.
So for example, `cargo help b` will show the manpage for `cargo-build`,
while `cargo help my-alias`, aliased to `build --release`, will show
"`my-alias` is aliased to `build --release`".
