Added aliases to subcommand typo suggestions. #7288
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Note: I've been away from both Cargo and Rust entirely for a few months, but figured I'd try to get back into things while I was sitting here at RustConf, and this seemed like a straightforward issue, so this code is likely...uh...rusty. Please tell me where I am very wrong and I will fix. :) |
|
Thanks @zachlute! I wonder if this could perhaps use the Otherwise this looks great! |
Update from rust-lang/rust#63637.
…crichton Allow using 'config.toml' instead of just 'config' files. Fixes rust-lang#7273 Note that this change only makes 'config.toml' optional to use instead of 'config'. If both exist, we will print a warning and prefer 'config', since that would be the existing behavior if both existed today. We should also consider a separate change to make config.toml the default and update docs, etc.
Basic standard library support. This is not intended to be useful to anyone. If people want to try it, that's great, but do not rely on this. This is only for experimenting and setting up for future work. This adds a flag `-Zbuild-std` to build the standard library with a project. The flag can also take optional comma-separated crate names, like `-Zbuild-std=core`. Default is `std,core,panic_unwind,compiler_builtins`. Closes rust-lang/wg-cargo-std-aware#10. Note: I can probably break some of the refactoring into smaller PRs if necessary. ## Overview The general concept here is to use two resolvers, and to combine everything in the Unit graph. There are a number of changes to support this: - A synthetic workspace for the standard library is created to set up the patches and members correctly. - Decouple `unit_dependencies` from `Context` to make it easier to manage. - Add `features` to `Unit` to keep it unique and to remove the need to query a resolver. - Add a `UnitDep` struct which encodes the edges between `Unit`s. This removes the need to query a resolver for `extern_crate_name` and `public`. - Remove `Resolver` from `BuildContext` to avoid any confusion and to keep the complexity focused in `unit_dependencies`. - Remove `Links` from `Context` since it used the resolver. Adjusted so that instead of checking links at runtime, they are all checked at once in the beginning. Note that it does not check links for the standard lib, but it should be safe? I think `compiler-rt` is the only `links`? I currently went with a strategy of linking the standard library dependencies using `--extern` (instead of `--sysroot` or `-L`). This has some benefits but some significant drawbacks. See below for some questions. ## For future PRs - Add Cargo.toml support. See rust-lang/wg-cargo-std-aware#5 - Source is not downloaded. It assumes you have run `rustup component add rust-src`. See rust-lang/wg-cargo-std-aware#11 - `cargo metadata` does not include any information about std. I don't know how this should work. - `cargo clean` is not std-aware. - `cargo fetch` does not fetch std dependencies. - `cargo vendor` does not vendor std dependencies. - `cargo pkgid` is not std-aware. - `--target` is required on the command-line. This should default to host-as-target. - `-p` is not std aware. - A synthetic `Cargo.toml` workspace is created which has to know about things like `rustc-std-workspace-core`. Perhaps rust-lang/rust should publish the source with this `Cargo.toml` already created? - `compiler_builtins` uses default features (pure Rust implementation, etc.). See rust-lang/wg-cargo-std-aware#15 - `compiler_builtins` may need to be built without debug assertions, see [this](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L210-L214). Could maybe use profile overrides. - Panic issues: - `panic_abort` is not yet supported, though it should probably be easy. It could maybe look at the profile to determine which panic implementation to use? This requires more hard-coding in Cargo to know about rustc implementation details. - [This](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L186-L201) should probably be handled where `panic` is set for `panic_abort` and `compiler_builtins`. I would like to get a test case for it. This can maybe be done with profile overrides? - Using two resolvers is quite messy and causes a lot of complications. It would be ideal if it could only use one, though that may not be possible for the foreseeable future. See rust-lang/wg-cargo-std-aware#12 - Features are hard-coded. See rust-lang/wg-cargo-std-aware#13 - Lots of various platform-specific support is not included (musl, wasi, windows-gnu, etc.). - Default `backtrace` is used with C compiler. See rust-lang/wg-cargo-std-aware#16 - Sanitizers are not built. See rust-lang/wg-cargo-std-aware#17 - proc_macro has some hacky code to synthesize its dependencies. See rust-lang/wg-cargo-std-aware#18. This may not be necessary if this uses `--sysroot` instead. - Profile overrides cause weird linker errors. That is: ```toml [profile.dev.overrides.std] opt-level = 2 ``` Using `[profile.dev.overrides."*"]` works. I tried fiddling with it, but couldn't figure it out. We may also want to consider altering the syntax for profile overrides. Having to repeat the same profile for `std` and `core` and `alloc` and everything else would not be ideal. - ~~`Context::unit_deps` does not handle build overrides, see rust-lang#7215.~~ FIXED ## Questions for this PR - I went with the strategy of using `--extern` to link the standard lib. This seems to work, and I haven't found any problems, but it seems risky. It also forces Cargo to know about certain implicit dependencies like `compiler_builtins` and `panic_*`. The alternative is to create a sysroot and copy all the crates to that directory and pass `--sysroot`. However, this is complicated by pipelining, which would require special support to copy `.rmeta` files when they are generated. Let me know if you think I should use a different strategy. I'm on the fence here, and I think using `--sysroot` may be safer, but adds more complexity. - As an aside, if rustc ever tries to grab a crate from sysroot that was not passed in via `--extern`, then it results in duplicate lang items. For example, saying `extern crate proc_macro;` without specifying `proc_macro` as a dependency. We could prevent rustc from ever trying by passing `--sysroot=/nonexistent` to prevent it from trying. Or add an equivalent flag to rustc. - How should this be tested? I added a janky integration test, but it has some drawbacks. It requires internet access. It is slow. Since it is slow, it reuses the same target directory for multiple tests which makes it awkward to work with. - What interesting things are there to test? - We may want to disable the test before merging if it seems too annoying to make it the default. It requires rust-src to be downloaded, and takes several minutes to run, and are somewhat platform-dependent. - How to test that it is actually linking the correct standard library? I did tests locally with a modified libcore, but I can't think of a good way to do that in the test suite. - I did not add `__CARGO_DEFAULT_LIB_METADATA` to the hash. I had a hard time coming up with a test case where it would matter. - My only thought is that it is a problem because libstd includes a dylib, which prevents the hash from being added to the filename. It does cause recompiles when switching between compilers, for example, when it normally wouldn't. - Very dumb question: Why exactly does libstd include a dylib? This can cause issues (see rust-lang/rust#56443). - This should probably change, but I want to better understand it first. - The `bin_nostd` test needs to link libc on linux, and I'm not sure I understand why. I'm concerned there is something wrong there. libstd does not do that AFAIK.
- man pages - Slightly reword deprecation notice. - Include --all in man pages. - Update some additional usages in code and docs.
Rename `--all` to `--workspace` ## Background close: rust-lang#6977 ## Description - Warn when using the 'all' option - Changed help text
don't need to copy this string This removes a `String::clone` that I noticed when profiling no-op builds of cargo, benchmarks show a barely visible improvement. Looks like it was added in rust-lang#6880, but I am not sure why.
`map_dependencies` is doing a deep clone, so lets make it cheaper This removes a `FeatureMap::clone` that I noticed when profiling no-op builds of cargo, benchmarks show a ~5% improvement. Looks like rust-lang#6880 means that there is a ref to every `Summery` so the `Rc::make_mut` dose a deep clone.
…chton guide: add section about the cargo home This PR adds a section about the $CARGO_HOME to the cargo guide. [Rendered](https://github.com/matthiaskrgr/cargo/blob/cargo_home_doc/src/doc/src/guide/cargo-home.md)
alexcrichton
added
S-waiting-on-author
Fix some man pages where the files weren't rebuilt. A few recent PRs edited the wrong files.
Fix test for changes in plugin API. Updates for rust-lang/rust#64041
doc: capitalization change for consistency.
add an option to specify ssl version Fixes rust-lang#6684
Support for named profiles (RFC 2678) Tracking issue: rust-lang#6988 Implementation according to the [RFC](https://github.com/rust-lang/rfcs/blob/master/text/2678-named-custom-cargo-profiles.md).
…alexcrichton unify the quote in Cargo.toml A bit nit-picked... It tries to use double-quotes in Cargo.toml instead of single quote, that would be more uniform in this file.
As described here https://rust-lang.github.io/rust-clippy/master/#needless_borrow. rust-clippy is complaining that the related borrows are unnecessary.
…alexcrichton Public dependency refactor and re-allow backjumping There were **three** attempts at vanquishing exponential time spent in Public dependency resolution. All failures. All three started with some refactoring that seams worth saving. Specifically the data structure `public_dependency` that is used to test for Public dependency conflicts is large, tricky, and modified in line. So lets make it a type with a name and move the interactions into methods. Next each attempt needed to know how far back to jump to undo any given dependency edge. I am fairly confident that any full solution will need this functionality. I also think any solution will need a way to represent richer conflicts than the existing "is this pid active". So let's keep the `still_applies` structure from the last attempt. Last each attempt needs to pick a way to represent a Public dependency conflict. The last attempt used three facts about a situation. - `a1`: `PublicDependency(p)` witch can be read as the package `p` can see the package `a1` - `b`: `PublicDependency(p)` witch can be read as the package `p` can see the package `b` - `a2`: `PubliclyExports(b)` witch can be read as the package `b` has the package `a2` in its publick interface. This representation is good enough to allow for `backjumping`. I.E. `find_candidate` can go back several frames until the `age` when the Public dependency conflict was introduced. This optimization, added for normal dependencies in rust-lang#4834, saves the most time in practice. So having it for Public dependency conflicts is important for allowing real world experimentation of the Public dependencies feature. We will have to alter/improve/replace this representation to unlock all of the important optimizations. But I don't know one that will work for all of them and this is a major step forward. Can be read one commit at a time.
Removed redundant borrow As described here https://rust-lang.github.io/rust-clippy/master/#needless_borrow. rust-clippy is complaining that the related borrows are unnecessary.
Pulls in alexcrichton/curl-rust#304 which fixes a bug from the last curl update in rust-lang#7308. This bug was not introduced by the Cargo PR itself but rather by updating the `curl` submodule in the `curl-sys` crate. Without this bugfix all downloads of a crate will make a new connection to crates.io, which drastically increases download time since setting up a connection takes so long.
These are just wasted syscalls for our purposes, no need to issue updates to the modification/creation/access times of files we unpack!
Disable preserving mtimes on archives These are just wasted syscalls for our purposes, no need to issue updates to the modification/creation/access times of files we unpack!
Removing hash from output files when using MSVC This is the fix for rust-lang#7358 suggested by @alexcrichton . Tested and working. I see a few tests failling but seem unrelated. I'll investigate them tomorrow.
add dependencies for `pkg-config`
Otherwise, it complains:
run pkg_config fail: "Failed to run `\"pkg-config\" \"--libs\" \"--cflags\" \"openssl\"`: No such file or directory (os error 2)"
Update `curl-sys` dependency requirement Pulls in alexcrichton/curl-rust#304 which fixes a bug from the last curl update in rust-lang#7308. This bug was not introduced by the Cargo PR itself but rather by updating the `curl` submodule in the `curl-sys` crate. Without this bugfix all downloads of a crate will make a new connection to crates.io, which drastically increases download time since setting up a connection takes so long.
This fixes rust-lang#7471 and fixes rust-lang#7255 by preventing the .wasm file from being treated as an executable binary, so `cargo test` and `cargo run` will no longer try to execute it directly. This change is only made for emscripten, which outputs a .js file as the primary executable entry point, as opposed to other WebAssembly targets for which the .wasm file is the only output.
…chton Mark Emscripten's .wasm files auxiliary This fixes rust-lang#7471 and fixes rust-lang#7255 by preventing the .wasm file from being treated as an executable binary, so `cargo test` and `cargo run` will no longer try to execute it directly. This change is only made for Emscripten, which outputs a .js file as the primary executable entry point, as opposed to other WebAssembly targets for which the .wasm file is the only output.
This fixes an accidental regression from rust-lang#7425 where `PATH` was being augmented on Windows with the wrong search path for target/host libraries. This commit fixes the issue by simply always calculating the host/target library paths for `TargetInfo`, and then we explicitly use the same `TargetInfo` for filling out information in `Compilation`. Closes rust-lang#7475
Fix wrong directories in PATH on Windows This fixes an accidental regression from rust-lang#7425 where `PATH` was being augmented on Windows with the wrong search path for target/host libraries. This commit fixes the issue by simply always calculating the host/target library paths for `TargetInfo`, and then we explicitly use the same `TargetInfo` for filling out information in `Compilation`. Closes rust-lang#7475
Update SPDX list to 3.6 Fixes rust-lang#7477
Fixes rust-lang#7278. Adds the ability to list all available aliases (by enumerating configuration subkeys) and to provide them to the suggestion distance calculator. Also adds tests for alias suggestions.
…o suggest-alias
|
Ugh, as usual I am bad at git. I tried to update my branch through the GitHub Desktop UI and now it lists every commit I updated as part of this PR? This is dumb, going to just open a new clean PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7278.
Adds the ability to list all available aliases (by enumerating configuration subkeys) and to provide them to the suggestion distance calculator.
Also adds tests for alias suggestions.