Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[beta 1.75] Backport 1password fixes #12994

Merged
merged 2 commits into from
Nov 17, 2023
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 17, 2023

This backports these fixes to 1.75 for the cargo-credential-1password helper. I'd like to publish these fixes and get the docs updated sooner rather than later. Unfortunately it uses an unpublished version of cargo-credential, so I can't publish directly from master.

Backports:

bors added 2 commits November 16, 2023 20:09
…r=ehuss

cargo-credential-1password: Add missing `--account` argument to `op signin` command

### What does this PR try to resolve?

Without this the account chooser is shown by the `op signin` command, even though the user has already specified an account via the `--account` command line argument to the `cargo-credential-1password` CLI.

Note that the `--vault` in this case does not need to be forwarded to `op`, since it is irrelevant for the `op signin` command.

### How should we test and review this PR?

- Have a 1password installation with multiple accounts
- Use `global-credential-providers = ["cargo-credential-1password --account my.1password.com"]` in the cargo config file
- Run e.g. `cargo publish`
- Notice how you are seeing an account switcher even though `--account` was used
- Apply this patch and notice that the account switcher is no longer there and the correct account is selected automatically

### Additional information

see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/1password.20credentials.20provider
cargo-credential-1password: Fix README

### What does this PR try to resolve?

The README.md file of the cargo-credential-1password package does not match the implemented reality (anymore?).

- `--sign-in-address` and `--email` are not actually supported by the implementation
- manually calling `op signin` appears to be unnecessary

This PR also adds configuration instructions directly to the README.md so that they show up on crates.io.
@rustbot
Copy link
Collaborator

rustbot commented Nov 17, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot
Copy link
Collaborator

rustbot commented Nov 17, 2023

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against rust-1.75.0. Please double check that you specified the right target!

@rustbot rustbot added A-credential-provider Area: credential provider for storing and retreiving credentials A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2023
@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 17, 2023

📌 Commit 8862fd3 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2023
@bors
Copy link
Contributor

bors commented Nov 17, 2023

⌛ Testing commit 8862fd3 with merge c67dced...

@bors
Copy link
Contributor

bors commented Nov 17, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing c67dced to rust-1.75.0...

1 similar comment
@bors
Copy link
Contributor

bors commented Nov 17, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing c67dced to rust-1.75.0...

@bors bors merged commit c67dced into rust-lang:rust-1.75.0 Nov 17, 2023
17 of 21 checks passed
@bors
Copy link
Contributor

bors commented Nov 17, 2023

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 20, 2023
…nglo

[beta-1.75] Update cargo

2 commits in 6790a5127895debec95c24aefaeb18e059270df3..1d8b05cdd1287c64467306cf3ca2c8ac60c11eb0
2023-11-10 17:09:35 +0000 to 2023-11-20 01:39:31 +0000
- [beta-1.75.0] chore: bump `cargo-credential-*` crates as e58b84d broke stuff (rust-lang/cargo#13009)
- [beta 1.75] Backport 1password fixes (rust-lang/cargo#12994)

r? ghost
@ehuss ehuss added this to the 1.75.0 milestone Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-credential-provider Area: credential provider for storing and retreiving credentials A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants