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

Gracefully handle gpg signature info in git log output #833

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Oct 9, 2023

If the user has showSignature = true configured for their Git environment, any invocation of git log will print additional information about the gpg signature, which causes an error. With this update, such gpg signature lines are ignored.

📦 Published PR as canary version: 7.2.4--canary.833.2fed8db.0

✨ Test out this PR locally via:

npm install [email protected]
# or 
yarn add [email protected]

@linear
Copy link

linear bot commented Oct 9, 2023

AP-3591 CLI breaks when `showSignature` is configured for Git

How is the user affected? And what is the expected behavior?

If the user has showSignature = true configured for their Git environment, any invocation of git log will print additional information about the gpg signature. This then gets passed along to the index as if it's a commit hash, but that's not valid and causes a GraphQL error.

How many and/or what class of users does this impact?

Users with showSignature = true configured in their local or global Git config.

Is there a workaround?

Disable showSignature.

What are the steps for reproducing the issue?

See #806

Other information

We could add --no-show-signature to the git log commands we run, but that wouldn't work with older Git versions. Instead it would be better to parse the output of the git log command to trim any suffix information such as the gpg signature.

@ghengeveld ghengeveld requested a review from thafryer October 9, 2023 12:05
@ghengeveld ghengeveld added the patch Auto: Increment the patch version when merged label Oct 9, 2023
Copy link
Member

@thafryer thafryer left a comment

Choose a reason for hiding this comment

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

LGTM!

@ghengeveld ghengeveld added this pull request to the merge queue Oct 10, 2023
Merged via the queue into main with commit 2fed8db Oct 10, 2023
15 checks passed
@ghengeveld ghengeveld deleted the ghengeveld/ap-3591-cli-breaks-when-showsignature-is-configured-for-git branch October 10, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Auto: Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants