Change exit codes for `updated` and `publish` #807

koddsson · 2017-05-05T21:27:45Z

Description

This patch:

Changes the exit code for the updated command from 0 to 1 when there are no changes to publish
Changes the exit code from publish from 1 to 0 and instead of throwing a error with a message that there is nothing to publish, logs that message out.

Motivation and Context

This change enables users that want to publish packages from a CI to more easily create a flow that relies on the exit codes of lerna commands.

Before this change CI builds would fail when lerna publish would return a non-zero exit code and it was not possible to tack lerna updated in front of it (!lerna updated & lerna publish) since it would always return a zero exit code.

Fixes #802

How Has This Been Tested?

I added a unit test for lerna updated and a integration test for lerna publish.

The lerna updated explicitly checks the exit code of that the command returns is 1 but the integration relies on the fact that if lerna publish would return a non zero exit code the test would fail.

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)

(This might be a breaking feature if we consider changing the exit code a breaking change, I'm not sure what you think)

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.

The docs don't mention exit codes as far as I can see but I can certainly add something to that effect to the docs if you'd like :)

Check if there are any packages to update before logging out the normal output and raise an error if there are no packages to update. Related-to: lerna#802

Instead of throwing an error we really just want to return a non-zero exit code. This patch adds the possibility to return a custom exit code (with a default of zero) and utilizes that possibility to return 1 as a exit code when a user runs the `updated` command and there are no packages that need updating. Related-to: lerna#802

Instead of throwing a error we want to just output a message that there is nothing for `publish` to do at this time since there are no changes. This helps in CI systems when you'd just want to run `lerna publish` without having to conditionally running the command based on if there are any changes. This changes the exit code so it would be a breaking change for anyone that currently relies on the exit code to be non-zero. Related-to: lerna#802

Passing `false` to the callback value will cause the exit code to be 1, allowing simpler logic.

koddsson · 2017-05-18T22:48:49Z

@evocateur I just noticed that you added some commits. I imagine managing a project like Lerna gets super hectic so if you need any more changes to this you can always ping me with what needs to change and I can get on it fairly quick (BST timezone tho 🇬🇧 )!

Excited for this to land ✨

evocateur · 2017-05-18T23:10:29Z

@koddsson Thanks for understanding! I was just about to merge and didn't want to bother you with fiddly stuff. :)

jwb · 2017-05-19T17:38:40Z

test/integration/lerna-publish.test.js

@@ -11,6 +11,23 @@ const lastCommitMessage = (cwd) =>
  execa.stdout("git", ["log", "-1", "--format=%B"], { cwd }).then(normalizeNewline);

 describe("lerna publish", () => {
+  test.concurrent("exists with a exit code 0 when there are no updates", async () => {


exists => exits.

Shouldn't there be an integration test showing the non-zero exit status results when the update command detects no changes?

They both use the same underlying utility, so the UpdatedCommand unit test is sufficient validation.

It is now possible to run `lerna publish` in CI unconditionally, only publishing when changes are actually detected, and never failing when it decides to not publish anything. Previously: - `lerna publish` when there are no updates to publish would throw an error - `lerna updated` when there are no updates would `exit 0`, making it ineffective as a chained filter (e.g., `lerna updated && lerna publish`) Now: - `lerna publish` when there are no updates is a no-op, exiting successfully with a helpful log message - `lerna updated` when there are no updates will exit non-zero (but _not_ throw an error), enabling it to be an effective filter Fixes lerna#802

evocateur · 2017-06-19T16:30:17Z

src/commands/PublishCommand.js

@@ -124,7 +124,8 @@ export default class PublishCommand extends Command {
      : [packagesToPublish];

    if (!this.updates.length) {
-      callback(new Error("No updated packages to publish."));
+      this.logger.info("No updated packages to publish.");
+      callback(null, true);


Per #887, we definitely need to pass false as the second argument to callback(), here. Otherwise it seems we attempt to make a git commit on no changes, and that's just wrong.

dan3721 · 2017-08-04T21:52:38Z

Is this fixed? I'm experiencing this in 2.0.0

? Are you sure you want to publish the above changes? Yes lerna ERR! execute Error: On branch master lerna ERR! execute Your branch is up-to-date with 'origin/master'. lerna ERR! execute Changes not staged for commit: lerna ERR! execute modified: lerna.json lerna ERR! execute modified: packages/bar/package.json lerna ERR! execute modified: packages/foo/package.json lerna ERR! execute lerna ERR! execute no changes added to commit lerna ERR! execute lerna ERR! execute at Function.module.exports.sync (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\node_modules\execa\index.js:277:26) lerna ERR! execute at Function.execSync (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\ChildProcessUtilities.js:59:30) lerna ERR! execute at Function.commit (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\GitUtilities.js:84:39) lerna ERR! execute at PublishCommand.gitCommitAndTagVersion (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\commands\PublishCommand.js:612:30) lerna ERR! execute at PublishCommand.commitAndTagUpdates (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\commands\PublishCommand.js:582:27) lerna ERR! execute at PublishCommand.execute (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\commands\PublishCommand.js:261:16) lerna ERR! execute at PublishCommand._attempt (C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\Command.js:289:21) lerna ERR! execute at C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\Command.js:276:15 lerna ERR! execute at C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\Command.js:298:13 lerna ERR! execute at C:\Users\BusDa001\AppData\Roaming\npm\node_modules\lerna\lib\commands\PublishCommand.js:495:11 lerna ERR! execute callback with error lerna ERR! execute { Error: On branch master lerna ERR! execute Your branch is up-to-date with 'origin/master'. lerna ERR! execute Changes not staged for commit: lerna ERR! execute modified: lerna.json lerna ERR! execute modified: packages/bar/package.json lerna ERR! execute modified: packages/foo/package.json

This change was missing from the PR: lerna#807 Relates to the issue: lerna#887 Currently running `lerna publish` when no packages need updating causes the task to crash unnecessarily. This small fix causes the command to complete if no updates are found, instead of trying to continue to the next section.

iMoses · 2017-08-22T15:15:19Z

@evocateur If we know where the issue is, why aren't we fixing it?
Will it be fixed in the near future?

evocateur · 2017-08-22T16:29:42Z

Contributions welcome.

iMoses · 2017-08-23T14:27:47Z

@evocateur I can make a PR, no problem, but I'm not sure what the best way to test it.
I haven't seen any tests verifying behavior when no changes were made.
Any pointers here to get me started?

lock · 2018-12-27T12:10:34Z

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Kristján Oddsson added 3 commits May 4, 2017 22:33

updated: throw error when no packages to update

ae24ae7

Check if there are any packages to update before logging out the normal output and raise an error if there are no packages to update. Related-to: lerna#802

evocateur approved these changes May 7, 2017

View reviewed changes

evocateur added the Tag: Breaking Change label May 7, 2017

evocateur added this to the v2.0.0 milestone May 7, 2017

evocateur added 5 commits May 18, 2017 14:20

Exit early instead of overloading execute callback

bb8f635

Passing `false` to the callback value will cause the exit code to be 1, allowing simpler logic.

only assert exit code in UpdatedCommand unit tests

0ec211e

don't forget the dang done() callback

57894d2

remove stale snapshots

a6b3d8c

no need for exit code handling in runCommand

76ce208

evocateur merged commit 880fdf3 into lerna:master May 18, 2017

jwb reviewed May 19, 2017

View reviewed changes

nutgaard mentioned this pull request Jun 14, 2017

Publish gives: "nothing to commit, working directory clean" #883

Closed

evocateur reviewed Jun 19, 2017

View reviewed changes

evocateur mentioned this pull request Jun 19, 2017

Stack trace on no changes when running publish #887

Closed

alisd23 mentioned this pull request Aug 9, 2017

Prevent crash on publish when no updated packages #966

Closed

9 tasks

lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change exit codes for `updated` and `publish` #807

Change exit codes for `updated` and `publish` #807

koddsson commented May 5, 2017

koddsson commented May 18, 2017

evocateur commented May 18, 2017

jwb May 19, 2017

evocateur May 19, 2017

evocateur Jun 19, 2017

dan3721 commented Aug 4, 2017

iMoses commented Aug 22, 2017

evocateur commented Aug 22, 2017

iMoses commented Aug 23, 2017

lock bot commented Dec 27, 2018

Change exit codes for updated and publish #807

Change exit codes for updated and publish #807

Conversation

koddsson commented May 5, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

koddsson commented May 18, 2017

evocateur commented May 18, 2017

jwb May 19, 2017

Choose a reason for hiding this comment

evocateur May 19, 2017

Choose a reason for hiding this comment

evocateur Jun 19, 2017

Choose a reason for hiding this comment

dan3721 commented Aug 4, 2017

iMoses commented Aug 22, 2017

evocateur commented Aug 22, 2017

iMoses commented Aug 23, 2017

lock bot commented Dec 27, 2018

Change exit codes for `updated` and `publish` #807

Change exit codes for `updated` and `publish` #807