Implement external credential process. (RFC 2730)
This adds a config setting for an external process to run to fetch the token for a registry. See `unstable.md` for more details.
As part of this, it adds a new `logout` command. This is currently gated on nightly with the appropriate `-Z` flag.
I have included four sample wrappers that integrate with the macOS Keychain, Windows Credential Manager, GNOME libsecret, and 1password. I'm not sure if we'll ultimately ship these, but I would like to. Primarily this provided a proof-of-concept to see if the design works.
**Patch Walkthrough**
This is a brief overview of the changes:
- Adds the `logout` command. With `cargo logout -Z unstable-options`, this allows removing the `token` from `.cargo/credentials`. With `cargo logout -Z credential-process`, this launches the process with the `erase` argument to remove the token from storage.
- Credential-process handling is in the `ops/registry/auth.rs` module. I think it is pretty straightforward, it just launches the process with the appropriate store/get/erase argument.
- `ops::registry::registry()` now returns the `RegistryConfig` to make it easier to pass the config information around.
- `crates/credential/cargo-credential` is a helper crate for writing credential processes.
- A special shorthand of the `cargo:` prefix for a credential process will launch the named process from the `libexec` directory in the sysroot (or, more specifically, the `libexec` directory next to the `cargo` process). For example `credential-process = "cargo:macos-keychain"`. My intent is to bundle these in the pre-built rust-lang distributions, and this should "just work" when used with rustup. I'm not sure how that will work with other Rust distributions, but I'm guessing they can figure it out. This should make it much easier for users to get started, but does add some integration complexity.
**Questions**
- I'm on the fence about the name `credential-process` vs `credentials-process`, which sounds more natural? (Or something else?)
- I'm uneasy about the behavior when both `token` and `credential-process` is specified (see `warn_both_token_and_process` test). Currently it issues a warning and uses `token`. Does that make sense? What about the case where you have `registries.foo.token` for a specific registry, but then have a general `registry.credential-process` for the default (it currently warns and uses the token, maybe it should not warn?)?
- I am still pretty uneasy with writing FFI wrappers, so maybe those could get a little extra scrutiny? They seem to work, but I have not extensively tested them (I tried login, publish, and logout). I have not previously used these APIs, so I am not familiar with them.
- Testing the wrappers I think will be quite difficult, because some require TTY interaction (and 1password requires an online account). Or, for example in the macOS case, it has GUI dialog box where I can use my fingerprint scanner. Right now, I just build them in CI to make sure they compile.
- 1password is a little weird in that it passes the token on the command-line, which is not very secure on some systems (other processes can see these sometimes). The only alternative I can think of is to not support `cargo login` and require the user to manually enter the token in the 1password GUI. I don't think the concern is too large (1password themselves seem to think it is acceptable). Should this be OK?
- I'm a little uneasy with the design of `cargo login`, where it passes the token in stdin. If the wrapper requires stdin for user interaction (such as entering a password to unlock), this is quite awkward. There is a hack in the 1password example that demonstrates using `/dev/tty` and `CONIN$`, which *seems* to work, but I'm worried is fragile. I'm not very comfortable passing the token in environment variables, because those can be visible to other processes (like CLI args), but in some situations that shouldn't be too risky. Another option is to use a separate file descriptor/handle to pass the token in. Implementing that in Rust in a cross-platform way is not terribly easy, so I wanted to open this up for discussion first.
Add contributor guide.
This consolidates and extends the contributor information in a single place. This is an mdbook project, along with a CI job which will build and deploy it to GitHub Pages at <https://rust-lang.github.io/cargo/contrib/>.
You can view a rendered version here: <https://ehuss.github.io/cargo/contrib/>
I don't know if this will actually be helpful to anyone, but I figured it's worth a shot.
NOTE: The CI deploy is designed to preserve the existing gh-pages content. However, it will **delete the history** on that branch. I think that should be fine, there doesn't seem to be too much interesting stuff there. I do have a backup in my fork, though. Some extra scrutiny on the code might be wise. The reason it deletes the history is because deploying mdbook on every push would balloon the repository size.
This commit updates Cargo's build of host dependencies to build them
with optimization level 0 by default instead of matching the profile of
the final binary.
Since Cargo's inception build dependencies have, by default, been built
in a profile that largely matches the profile of the final target
artifact. Build dependencies, however, rarely actually need to be
optimized and are often executing very small tasks, which means that
optimizing them often wastes a lot of build time. A great example of
this is procedural macros where `syn` and friends are pretty heavyweight
to optimize, and the amount of Rust code they're parsing is typically
quite small, so the time spent optimizing rarely comes as a benefit.
The goal of this PR is to improve build times on average in the
community by not spending time optimizing build dependencies (build
scripts, procedural macros, and their transitive dependencies). The PR
will not be a universal win for everyone, however. There's some
situations where your build time may actually increase:
* In some cases build scripts and procedural macros can take quite a
long time to run!
* Cargo may not build dependencies more than once if they're shared with
the main build. This only applies to builds without `--target` where
the same crate is used in the final binary as in a build script.
In these cases, however, the `build-override` profile has existed for
some time know and allows giving a knob to tweak this behavior. For
example to get back the previous build behavior of Cargo you would
specify, in `Cargo.toml`:
[profile.release.build-override]
opt-level = 3
or you can configure this via the environment:
export CARGO_PROFILE_RELEASE_BUILD_OVERRIDE_OPT_LEVEL=3
There are two notable features we would like to add in the future which
would make the impact of a change like this smaller, but they're not
implemented at this time (nor do we have concrete plans to implement
them). First we would like crates to have a way of specifying they
should be optimized by default, despite default profile options. Often
crates, like lalrpop historically, have abysmal performance in debug
mode and almost always (even in debug builds) want to be built in
release mode. The second feature is that ideally crate authors would be
able to tell Cargo to minimize the number of crates built, unifying
profiles where possible to avoid double-compiling crates.
At this time though the Cargo team feels that the benefit of changing
the defaults is well worth this change. Neither today nor directly after
this change will be a perfect world, but it's hoped that this change
makes things less bad!
reset lockfile information between resolutions
#8249 pointed out that some kind of lockfile data was leaking between calls to the resolver. @ehuss made a reproducing test case. This PR resets the `LockedMap` data structure when calling `register_previous_locks`.
lets see if CI likes it.
fix#8249
This stops using `to_string` as a proxy for this now-provided precise API.
This reverts commit b71927224fd9306b2b5bd2b4f8c22268eadfeb6a and bupms the
dependency version in Cargo.toml.
Use associated constants directly on primitive types instead of modules
This PR is in no way critical. It's more of a code cleanup. It comes as a result of me making https://github.com/rust-lang/rust/pull/70857 and search-and-replacing all usage of the soft-deprecated ways of reaching primitive type constants.
It makes the code slightly shorter, that's basically it. And showcases the recommended way of reaching these consts on new code :)
Add windows-gnu CI and fix tests
One remaining failure:
```
---- features::feature_off_dylib stdout ----
running `d:\a\1\s\target\debug\cargo.exe build --features f1`
running `d:\a\1\s\target\debug\cargo.exe run -p bar`
thread 'features::feature_off_dylib' panicked at '
Expected: execs
but: exited with exit code: 101
--- stdout
--- stderr
Compiling foo v0.0.1 (D:\a\1\s\target\cit\t663\foo)
Compiling bar v0.0.1 (D:\a\1\s\target\cit\t663\foo\bar)
Finished dev [unoptimized + debuginfo] target(s) in 0.69s
Running `target\debug\bar.exe`
thread 'main' panicked at 'assertion failed: `(left == right)`
left: `"f1"`,
right: `"no f1"`', bar\src\main.rs:5:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\bar.exe` (exit code: 101)
', crates\cargo-test-support\src\lib.rs:833:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
features::feature_off_dylib
```
I disassembled the dylibs and `cargo run -p bar` correctly rebuilt it inside `target/debug/deps/` but did not copy it to `target/debug`. To further confirm, calling `cp target/debug/deps/foo.dll target/debug/` manually solved the issue.
Any idea?
----
I left `FIXME` in places where import lib should be added with https://github.com/rust-lang/cargo/pull/6875.
`TOOLCHAIN: nightly-x86_64-pc-windows-gnu` can be replaced with beta on Thursday.
Several updates to token/index handling.
This attempts to tighten up the usage of token/index handling, to prevent accidental leakage of the crates.io token.
* Make `registry.index` config a hard error. This was deprecated 4 years ago in #2857, and removing it helps simplify things.
* Don't allow both `--index` and `--registry` to be specified at the same time. Otherwise `--index` was being silently ignored.
* `registry.token` is not allowed to be used with the `--index` flag. The intent here is to avoid possibly leaking a crates.io token to another host.
* Added a warning if source replacement is used and the token is loaded from `registry.token`.
Closes#6545
