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(publish): ensure zero exit code when EWORKINGTREE warning occurs #3327

Merged
merged 3 commits into from Mar 2, 2023
Merged

fix(publish): ensure zero exit code when EWORKINGTREE warning occurs #3327

merged 3 commits into from Mar 2, 2023

Conversation

paul-leo
Copy link
Contributor

@paul-leo paul-leo commented Sep 15, 2022

fixed error when run "lerna publish from-package" in package.json scripts

Description

when verifyWorkingTreeClean error happend set exitCode to 0

Motivation and Context

How Has This Been Tested?

1、add publish:'lerna publish from-package" to package.json->"scripts"
2、exec "npm run publish" in terminal
3、everything well done

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.

@paul-leo
Copy link
Contributor Author

fixed

@sirctseb
Copy link

sirctseb commented Nov 17, 2022

@juristr we recently upgraded from version 3.22.1 to version 6. When using lerna publish from-package --no-git-reset, we get the issue described here: the publish succeeds, but the process exit code is 128, so our build containers are marked as failures.

Some more context:

  1. The issue occurs when there is no .git directory (as it the case in CI, since we don't include the .git directory in the image), and can be reproduced locally by simply removing or renaming the .git directory
  2. The issue is introduced in version 4.0.0. Version 3.22.1 works just fine. There are a lot of commits in that release, I haven't found a way to git bisect because I can't yarn link lerna directly into my project, and I haven't identified any suspicious changes in the publish implementation that would explain it, however:
  3. the change in this PR resolves our problem, although it is not clear why (in particular, it is not clear why it wasn't a problem in 3.22.1)

Do you think this PR should be accepted?

@juristr
Copy link
Contributor

juristr commented Nov 21, 2022

@sirctseb I'll bring this issue to the team's attention so we can look into it. There are a couple of potential issues we've already seen that might occur when there's no Git repo set up because Lerna requires Git for many of its operations. Some are legit because the Git information is needed as part of the operation, but for others it should not be required.

@sirctseb
Copy link

sirctseb commented Dec 9, 2022

Thanks @juristr. Let me know if we can help with any additional information from our use-case. We do not need (or want) our .git directory in the docker image we us in CI when we call lerna publish from-package and my understanding is that that operation does not require any knowledge of git for that operation, with evidence from the fact that version 3 worked with no issue and this message from the code:

this.logger.notice("FYI", "Unable to verify working tree, proceed at your own risk");

We're exploring workarounds like using patch-package to include the change from this PR in our local installation, but would rather get correct working behavior back into your repo.

Thanks!

@JamesHenry JamesHenry changed the title fix(publish): fix error when run publish in pkg.json scripts fix(publish): ensure zero exit code when EWORKINGTREE warning occurs Mar 2, 2023
@JamesHenry JamesHenry merged commit 9c00a33 into lerna:main Mar 2, 2023
@JamesHenry
Copy link
Member

Thank you @pengzai-dev!

@Cabeda
Copy link

Cabeda commented Mar 20, 2023

Is there an estimated date for a new release with this fix?

@JamesHenry
Copy link
Member

Thanks for your patience on this, it slipped my mind completely - we will cut a release this week for sure

@JamesHenry
Copy link
Member

Lerna 6.6.0 is now available which includes this, many thanks!

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

5 participants