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

Lerna will always publish all packages even if some are not changed #1357

Closed
egoist opened this issue Apr 4, 2018 · 47 comments
Closed

Lerna will always publish all packages even if some are not changed #1357

egoist opened this issue Apr 4, 2018 · 47 comments

Comments

@egoist
Copy link

egoist commented Apr 4, 2018

Expected Behavior

Only publish updated packages.

Current Behavior

All packages will be published, run lerna updated it lists all packages even if none of them are updated.

Possible Solution

Took a quick look at the code, not entirely sure how it works but seems it's related to:

const needsBump = (options.cdVersion || "").startsWith("pre")
? () => false
: /* skip packages that have not been previously prereleased */
node => semver.prerelease(node.version);
packages.forEach((node, name) => {
if (forced.has(name) || needsBump(node) || hasDiff(node)) {
candidates.add(node);
}
});

Does this mean the package will always be published if the current version is a prelease like x.x.x-beta.x and no cdVersion is present?

It works properly when running with lerna updated --cd-version prerelease, is this intended?

Steps to Reproduce (for bugs)

  1. Have some leaf packages whole versions are x.x.x-beta.x
  2. Run yarn lerna updated

Your Environment

Executable Version
lerna --version 3.0.0-beta.14 (same for 2.7, 2.8)
npm --version 5.5.1
yarn --version 1.5.1
node --version 8.9.3
@evocateur
Copy link
Member

I need more context: lerna.json? Previous version released?

With fixed versioning (the default), it is intentional when doing a "normal" release to bump all current prereleased packages to the new "normal" release. The assumption is that you are "graduating" the prerelease out of alpha/beta/whatever, and no "normal"-versioned package (in a lerna context) should ever depend on prereleased local dependencies.

@amkoehler
Copy link

amkoehler commented Apr 4, 2018

The assumption is that you are "graduating" the prerelease out of alpha/beta/whatever, and no "normal"-versioned package (in a lerna context) should ever depend on prereleased local dependencies.

This applies when using independent versioning too, correct? Not OP, but I similarly found that lerna updated always returns true if my packages are in a prerelease state (e.g. 4.0.0-2). Since the first step of lerna publish is to run the equivalent of lerna updated, it makes sense why the packges in a prerelease state would always get published.

@egoist
Copy link
Author

egoist commented Apr 5, 2018

lerna.json:

{
  "lerna": "3.0.0-beta.14",
  "version": "independent",
  "commands": {
    "publish": {
      "ignoreChanges": [
        "**/*.md",
        "**/example/**",
        "**/test/**"
      ]
    }
  },
  "npmClient": "yarn",
  "useWorkspaces": true
}

Previously versioned as 10.0.0-beta.x

@lili21
Copy link

lili21 commented Apr 9, 2018

same issue here. weird. I'm using lerna@2.9.0.

maybe lerna can output more information for debugging. like what make lerna think a package changed ?

// lerna.json
{
  "lerna": "2.9.0",
  "packages": [
    "packages/*"
  ],
  "npmClient": "yarn",
  "useWorkspaces": "true",
  "version": "independent"
}

@davidhouweling
Copy link

seems like a workspaces issue?

@lili21
Copy link

lili21 commented Apr 11, 2018

lerna updated should tell me what exactly updated in code level. what I changed ?

lerna updated --detail ?

@evocateur
Copy link
Member

evocateur commented Apr 11, 2018 via email

@amkoehler
Copy link

amkoehler commented Apr 12, 2018

Follow-up from my previous comment, when I run lerna updated it shows packages as updated even when nothing has changed after a release. lerna diff for each package outputs nothing. It wasn't because of the prerelease state as I'd previously thought.

$ lerna diff 
lerna info version 2.9.0
lerna info versioning independent
{
  "lerna": "2.9.0",
  "npmClient": "yarn",
  "useWorkspaces": true,
  "version": "independent"
}

edit: did some more digging and I found that if I ran lerna updated immediately after publishing it returned nothing (as expected), but if I made a code change in any one of my packages, all of them then showed up as changed when running lerna updated.

@fzaninotto
Copy link

Same problem here on react-admin, some packages contain no changes after v2.0.2 (e.g. ra-data-graphql), but lerna wants to publish them anyway. Theses packages are dependent on another package that changed since v2.0.2 (react-admin), but it's only as peerDependencies and devDependencies, not as dependencies.

Should lerna force a republish if a non-production dependency changed?

@kevinbarabash
Copy link

I'm having the same issue. lerna diff appears to be accurate. Could use it to determine which packages need updating?

@evocateur
Copy link
Member

evocateur commented Jun 15, 2018 via email

@evocateur
Copy link
Member

evocateur commented Jun 15, 2018 via email

@kevinbarabash
Copy link

If I have some packages that are all using ^1.0.0 of some dependency and then that dependency gets bumped to 1.1.0 I would expect the packages that depend on it not require publishing unless one of those packages actually uses the new feature added in 1.1.0 in which case I would need to change that package to use that new feature. I'd also update its deps to be ^1.1.0 since it now requires that new feature.

@evocateur
Copy link
Member

evocateur commented Jun 15, 2018 via email

@fzaninotto
Copy link

A test case repro would go a long way toward fixing it.

I tracked down the code responsible for that in utils/collect-updates (I think), but there is no test around that I could use as an example for building a new test case.

@evocateur
Copy link
Member

evocateur commented Jun 15, 2018 via email

@fzaninotto
Copy link

I tried to understand how your tests are organized and where the dependency resolution is tested in commands/publish tests, but I didn't manage to, sorry. It would be much easier if you had unit tests in the utils/ directory to take inspiration from.

@evocateur
Copy link
Member

It's in the publish tests because it was only originally extracted to share with the "changed" subcommand (which used to be called "updated"), which uses the exact same logic to list what has changed since the last release. Doing tests under collect-updates would duplicate 80% of the existing tests under commands/publish.

I started poking at it, and I've noticed some gaps in coverage from recent changes I made. I'm trying to figure out if it was intentional to have changes in devDependencies trigger package bumps...

@jwickens
Copy link
Contributor

At a more fundamental level when I run

lerna updated --since=current-branch

with version 2.5.1 I get all the packages listed

But when I do it with 2.4.0 I get no packages listed (what I expected)

Surely there's a test to make sure updated returns no packages when --since is set to HEAD ?

@evocateur
Copy link
Member

evocateur commented Jul 12, 2018 via email

@t-kelly
Copy link

t-kelly commented Jul 17, 2018

@evocateur looks like this is the problem code for me:

const needsBump = (cdVersion || "").startsWith("pre")
? () => false
: /* skip packages that have not been previously prereleased */
node => semver.prerelease(node.version);

semvar.prerelease() returns an array containing the prerelease tag and number, which in turn makes needsBump truthy. This results in any package that uses a prerelease tag to be bumped, even if they have not changed (aka hasDiff() is falsy).

If I remove the needsBump() condition from line 59, only the packages that have changed get bumped and things work as intended.

What is the purpose of needsBump()?

Related:

@evocateur
Copy link
Member

@t-kelly Thanks for the detailed analysis!

I want to say that needsBump ternary was related to ensuring that “graduating” a prerelease (alpha to beta, or release candidate to general release) would ensure nothing gets “left behind,” even if nothing has changed since the immediately preceding prerelease.

There is almost definitely a bug there in independent mode. I don’t remember if the “graduation” scenario is completely handled in lerna publish or not...

@lorenzomigliorero
Copy link

lorenzomigliorero commented Jul 27, 2018

Any news about this bug?

@verekia
Copy link

verekia commented Jul 27, 2018

For those who might be in that situation, I didn't realize that if you update one of your package that is a dependency of your other packages it updates all of those as well. Seems pretty obvious now that I think about it, but because the version bump happens automatically in the package.json files, it can be confusing.

@evocateur
Copy link
Member

There is a significant refactoring of versioning and publishing about to land, so I haven't had time to look into this directly. I still don't have a clear reproduction.

@t-kelly
Copy link

t-kelly commented Jul 30, 2018

@evocateur -- you should be able to reproduce it by:

  1. Create a project with a few packages and zero dependencies in those packages.
  2. Set the versions of those packages to -beta.1
  3. Make a change to one package.
  4. Run lerna publish. You will see that even though only one package has changed, all packages get updated because they have beta in their version name.

@evocateur
Copy link
Member

@t-kelly If you're using the default versioning scheme, that's expected behavior if you're doing a non-prerelease.

@t-kelly
Copy link

t-kelly commented Jul 31, 2018

Even if I'm going from beta.1 to beta.2? Because they're all being updated even I do that.

I agree it should update all packages from 1.0.0-beta.1 -> 1.0.0 if I do a non-prerelease.

@evocateur
Copy link
Member

I personally did a lot of context-sensitive (i.e., "partial") beta prereleases in lerna/lerna itself (using "fixed" versioning) and never encountered this issue, so I'd really like a complete picture (config, commands, repo state) to get to the bottom of it.

@Adam13531
Copy link

@evocateur I came up with a repro for this since I've been experiencing this issue for many months. I pushed to a repo here, but I've listed all of the steps below for creating the problem from scratch.

My platform:

  • Windows
  • npm 6.1.0
  • Node 8.11.1
  • Lerna 3.1.4

Repro steps:

  • Create repo in GitHub
  • cd into the repo
  • npm init -y
  • npm install --save-dev lerna
  • .\node_modules\.bin\lerna.cmd init --independent
  • cd packages
  • mkdir package1
  • cd package1
  • npm init -y
  • cd ..
  • mkdir package2
  • cd package2
  • npm init -y
  • Manually change version in package2/package.json to "1.0.0-adam.1"
  • Commit everything
  • Push (just to avoid this issue)
  • Run lerna version
    • Pick 1.0.1 for package1
    • Pick "custom prerelease" and type "adam" so that it ends up as 1.0.0-adam.2
      Here's the output I got - Changes:
      • package1: 1.0.0 => 1.0.1
      • package2: 1.0.0-adam.1 => 1.0.0-adam.2
  • Add a file to package1 called "a.txt" with the contents "hello world"
  • Commit a.txt
  • Run lerna changed

Expected: only package1 changed
Actual: package1 and package2 changed

@brmscheiner
Copy link

brmscheiner commented Aug 31, 2018

Not sure if this will be helpful for debugging or just cloud the issue further, but our team at work uses lerna and encounters this issue on some of our systems. We have found that two computers can have many things in common but have different results when running lerna publish

  • Macbook Pros running High Sierra
  • lerna version 2.11.0
  • yarn version 1.7.0
  • node 8.11.2
  • on origin master

One asks to update 30+ repos the other needs to patch only 4. The issue persists after blowing up node_modules and reinstalling, or even after downloading a brand new repo.

@t-kelly
Copy link

t-kelly commented Sep 1, 2018

@brmscheiner sounds like that needs an issue of its own. Not sure its related to this thread. Recommend you create a new issue with this so its easier to track by the maintainers! 😄

@koresar
Copy link

koresar commented Sep 16, 2018

From the top of my head I remember that lerna v1.0 behaved correctly.

@yvele
Copy link

yvele commented Oct 19, 2018

What is the state of this issue? Is this still under investigation?

I'm a bit confused as everything worked pretty fine before v3 🤔

@kondei
Copy link

kondei commented Oct 26, 2018

please fix it

@evolutionxbox
Copy link

Been wondering if I was the only one experiencing this.

I have a simple project running in independent mode:

.
├── lerna.json
├── package-lock.json
├── package.json
└── packages
    ├── button
    └── table

When I update button both packages show up as changed in lerna changed. Despite neither having any dependencies.

@yvele
Copy link

yvele commented Oct 30, 2018

@evolutionxbox I have the same problem

@evolutionxbox
Copy link

@yvele do you merge using --no-ff?

@Hoishin
Copy link

Hoishin commented Nov 13, 2018

If you have this problem and you manually put tags instead of lerna:

  • changed/version/publish commands use @lerna/collect-updates, which make use of git describe git command.
  • git describe only finds annotated tags by default
  • When lerna puts tags, it runs git tag ${tag} -m ${tag}. The -m option makes them "annotated tags".
  • When you manually put tags (git tag ${tag}), it is "unannotated tags".
  • So when you run lerna commands, it cannot find the manually put unannotated tags.

The workarounds are:

  • When you manually tag, use annotated tag (git tag ${tag} -m ${tag})

The PR above makes lerna find unannotated tags with git describe --tags, but if finding only annotated tags is intended behaviour, I'm willing to submit another PR for clearer documentation regarding to this.

@yvele
Copy link

yvele commented Nov 13, 2018

@yvele do you merge using --no-ff?

@evolutionxbox Nope but it may have happened a couple of times in my git history. Why?

@randy-askin
Copy link

randy-askin commented Dec 12, 2018

I believe the line should be changed from:

const needsBump = (commandOptions.bump || "").startsWith("pre")

to:

const needsBump = (!commandOptions.bump || commandOptions.bump.startsWith("pre"))

That way if someone performs a fixed major minor or patch, the unrelated prereleased packages will be included for graduation. But if they perform a fixed premajor preminor prepatch or prerelease OR if they're using the independent versioning mode with the interactive prompt, the unrelated prereleased packages would not be included.

@evocateur
Copy link
Member

@randy-askin How does that change differ from the actual current value?

const needsBump = (commandOptions.bump || "").startsWith("pre")

@randy-askin
Copy link

In the case of independent non-bump versioning, e.g. use of the interactive prompt to pick exactly what versions you want, commandOptions.bump is undefined, so "".startsWith("pre") is false and the ternary picks up any prereleased packages regardless of whether they've changed or not. In my example, if bump is undefined (for independent/interactive versions) OR starts with pre (for bump versions), the ternary is true and the unchanged packages are not promoted unnecessarily.

@evocateur
Copy link
Member

Oh, jeez, you're totally right. Thanks for explaining it clearly to my toddler-addled brain. :)

@evocateur evocateur added bug and removed discussion labels Jan 7, 2019
@evocateur
Copy link
Member

So I made the change and none of my unit tests failed, 😭

diff --git a/utils/collect-updates/collect-updates.js b/utils/collect-updates/collect-updates.js
index dfedf36..2dcce43 100644
--- a/utils/collect-updates/collect-updates.js
+++ b/utils/collect-updates/collect-updates.js
@@ -56,10 +56,11 @@ function collectUpdates(filteredPackages, packageGraph, execOpts, commandOptions
     candidates = new Set();
 
     const hasDiff = makeDiffPredicate(committish, execOpts, commandOptions.ignoreChanges);
-    const needsBump = (commandOptions.bump || "").startsWith("pre")
-      ? () => false
-      : /* skip packages that have not been previously prereleased */
-        node => node.prereleaseId;
+    const needsBump =
+      !commandOptions.bump || commandOptions.bump.startsWith("pre")
+        ? () => false
+        : /* skip packages that have not been previously prereleased */
+          node => node.prereleaseId;
 
     packages.forEach((node, name) => {
       if (forced.has(name) || needsBump(node) || hasDiff(node)) {

*straps on big-kid test-writing boots*

@evocateur
Copy link
Member

v3.10.1 hopefully fixes this

@lock
Copy link

lock bot commented Apr 9, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 9, 2019
nicolo-ribaudo pushed a commit to babel/lerna that referenced this issue Jul 4, 2019
Fixes lerna#1357

Huge thanks to all the contributors on that issue, especially @Adam13531
for their exceptionally detailed reproduction and @randy-askin for their
clear (and patient!) identification of the ternary bug.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.