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

Fix crash in lerna version when using gitSignTag #3832

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Fix crash in lerna version when using gitSignTag #3832

merged 2 commits into from
Sep 11, 2023

Conversation

squidfunk
Copy link
Contributor

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
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
Contributor Author

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
Contributor

fahslaj commented Sep 11, 2023

@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
13 checks passed
@fahslaj
Copy link
Contributor

fahslaj commented Sep 11, 2023

Thanks @squidfunk !

@squidfunk
Copy link
Contributor Author

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.

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.

None yet

2 participants