Skip to content

Using git-webkit pr via SSH dumps errors #47141

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

Conversation

briannafan
Copy link
Contributor

@briannafan briannafan commented Jun 24, 2025

@briannafan briannafan self-assigned this Jun 24, 2025
@briannafan briannafan requested a review from emw-apple June 24, 2025 23:10
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 25, 2025
Copy link
Contributor

@emw-apple emw-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

> git webkit pr --no-add --no-rebase
Total errors found: 0 in 1 files
Could not access credentials from keychain. Please run `security unlock-keychain` before continuing. (C/o/n/t/i/n/u/e): 

So you need to change the options field to be a list, but otherwise looks good.

Comment on lines 33 to 38
if Terminal.choose(
f'Could not access credentials from keychain. Please run `security unlock-keychain` before continuing.',
default='Continue',
options='Continue',
) == 'Continue':
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I might prefer failing here, instead of prompting for continuation? Especially since the remedy is to run another terminal command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot of sense, especially since this happens during ssh sessions where it's harder to just open a new terminal window!

@briannafan briannafan removed the merging-blocked Applied to prevent a change from being merged label Jun 25, 2025
@briannafan briannafan force-pushed the eng/Using-git-webkit-pr-via-SSH-dumps-errors branch from a6bcf3a to 5700f62 Compare June 25, 2025 17:35
@briannafan briannafan added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 25, 2025
rdar://151526906
https://bugs.webkit.org/show_bug.cgi?id=242647

Reviewed by Elliott Williams.

Catch the KeyringError and return a helpful message that suggests running
`security unlock-keychain`.

* Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py:
(handle_keyring_error):
(credentials):

Canonical link: https://commits.webkit.org/296626@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Using-git-webkit-pr-via-SSH-dumps-errors branch from 5700f62 to 2cfc024 Compare June 25, 2025 17:45
@webkit-commit-queue
Copy link
Collaborator

Committed 296626@main (2cfc024): https://commits.webkit.org/296626@main

Reviewed commits have been landed. Closing PR #47141 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 2cfc024 into WebKit:main Jun 25, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 25, 2025
@briannafan briannafan deleted the eng/Using-git-webkit-pr-via-SSH-dumps-errors branch June 25, 2025 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants