Skip to content

Fix crash in lerna version when using gitSignTag#3832

Merged
fahslaj merged 2 commits intolerna:mainfrom
squidfunk:fix/gpg-race-condition
Sep 11, 2023
Merged

Fix crash in lerna version when using gitSignTag#3832
fahslaj merged 2 commits intolerna:mainfrom
squidfunk:fix/gpg-race-condition

Conversation

@squidfunk
Copy link
Copy Markdown
Contributor

@squidfunk squidfunk commented Sep 8, 2023

When instructing lerna to sign tags, gpg often runs out of memory because it isn't designed to be run in parallel (see similar issue on another repository). Thus, when signGitTag is set to true, signing must be carried out serially.

Description

If gitTagSign is set to true, this PR will sign tags one after another, omitting the race condition

Motivation and Context

Signing git tags

How Has This Been Tested?

I'm not sure how to test it, as we'd need to somehow mock gitTag to check if we call it concurrently or not.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Possibly related

@fahslaj
Copy link
Copy Markdown
Contributor

fahslaj commented Sep 8, 2023

Hi @squidfunk , do you happen to have a public repo that this error occurs in? I created a small test repo and set up gpg locally and was not able to reproduce the issue you describe.

@squidfunk
Copy link
Copy Markdown
Contributor Author

squidfunk commented Sep 9, 2023

Thanks for investigating. I don't currently have a public repo exposing this behavior. The problem only occurs sporadically, and you need to have several CPUs available and run lerna version concurrently on several packages so that tags are signed in parallel. I believe it's a race condition. Let me try to create a reproduction.

@fahslaj
Copy link
Copy Markdown
Contributor

fahslaj commented Sep 11, 2023 •
edited
Loading

@squidfunk Thank you for the extra context. I tested this on a larger repo with many packages and was able to reproduce the issue after all and confirm that the proposed solution fixes it. I've pushed a small formatting change and will merge this as soon as CI passes.

@fahslaj fahslaj merged commit 4940f2d into lerna:main Sep 11, 2023
@fahslaj
Copy link
Copy Markdown
Contributor

fahslaj commented Sep 11, 2023

Thanks @squidfunk !

@squidfunk
Copy link
Copy Markdown
Contributor Author

squidfunk commented Sep 11, 2023

Perfect! Thanks for the quick merge! Sorry that I wasn't more responsive, I had some other things on my plate today, but glad you could reproduce the problem ☺️ Next time I will provide a reproduction right away.

@squidfunk squidfunk deleted the fix/gpg-race-condition branch September 11, 2023 18:08
@ghiscoding ghiscoding mentioned this pull request Dec 4, 2023
Closed
@ivml ivml mentioned this pull request Dec 8, 2023
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fahslaj fahslaj fahslaj approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@squidfunk @fahslaj