qiwi / multi-semantic-release

Proof of concept that wraps semantic-release to work with monorepos.
BSD Zero Clause License
86 stars 34 forks source link

Smth reverts package.json changes after `prepare` step #1

Closed antongolub closed 4 years ago

antongolub commented 4 years ago

https://github.com/qiwi/substrate/commit/5e6b5d296de222fe2a8657878717845ac77fc220

antongolub commented 4 years ago

@dhoulb, could you take a look?

I seems that updated manifest deps were reverted before the publish, but package version was bumped as expected.

1) https://github.com/antongolub/release-testing/commit/aaff4708897256b0f8581bd25245aaf015ab2c62 2) https://github.com/antongolub/release-testing/commit/6565409b8e7df9d330eda9485cf9d27ed4cf79fd 3) https://github.com/antongolub/release-testing/commit/906143642c0192918a455a18c6c6d30b27176ae7

dhoulb commented 4 years ago

@antongolub Sorry, I took a quick look at this, but it was taking too long to understand your setup and I couldn't justify any more time on it.

The multi-semantic-release package was only intended as a proof of concept. I didn't consider it to be production ready — I wrote up a plan for semantic-release to allow them to support monorepos natively but I guess they don't have the time to implement it.

Since I created multi-semantic-release I've changed my opinion on semantic-release and monorepos. I think the step of coordinating and inter-relating the packages is too complicated to ever work well (and would be a maintenance nightmare).

As an alternative I now publish the entire repo to NPM and reference it using e.g. import { myFunc } from "my-package/my-sub-package"; — I find this hugely simplifies things.

dhoulb commented 4 years ago

@antongolub I've thought about this a bit further and I think it's an actual issue in multi-semantic-release

In the prepare step, each package writes its deps then immediately calls the prepare step for all other plugins (including the @semantic-release/git plugin's prepare step which is the one that commits the changes back to the step).

This means that when the prepare step in the git plugin is called for first package, the changes are committed even though not all packages have written their package.json yet.

It needs reworking to so that ALL package.json files are written before the prepare step is called for any of the plugins.

The way I would do this is adjust updateManifestDeps() in createInlinePluginCreator.js so that it loops through all packages and writes them all (on the first run only) before any plugins have their prepare step called.

Unfortunately I don't have time to do this right now — but if you have a little time to fix it please do!

antongolub commented 4 years ago

@dhoulb, Hi Dave.

Thanks for the feedback. We'll try to check this conjecture asap.

antongolub commented 4 years ago

@dhoulb, Dave

semrel runs its own git plugin prepare before the synthetic inlinePlugin, so I attached the deps updater right after the last commitAnalyzer run. Dirty hack right here, right now.

Anyway, it looks to be working: https://github.com/antongolub/release-testing/commit/834daffbaa72bc1c798cf0c92840503ca31d2cac

UPD#1 I guess, notesGenerator is even a better place. It goes right before the prepare step:

  await plugins.verifyRelease(context);

  nextRelease.notes = await plugins.generateNotes(context);

  await plugins.prepare(context);

  if (options.dryRun) {

UDP#2 And finally it woks as expected. Here's the fix: https://github.com/qiwi/multi-semantic-release/pull/2. Result: https://github.com/antongolub/release-testing/commit/e14b6d40e77abe8f5fc65fb7ace5e6e64d05469a I need some extra time to prepare PR.

antongolub commented 4 years ago

@dhoulb, Dave

I'm also trying to replace execa hook and setImmediate loops with a signal bus. I found a good one — yanickrochon/promise-events.

But I was faced with another strange problem. The first published package release somehow completes the whole multi-sem-rel session, as like it triggers process.exit(0) and ignores the rest promises

await Promise.all(packages.map((pkg) => releasePackage(pkg, createInlinePlugin, multiContext)));

If you have free time, could you take a look?

Here's the code: pull/2 Here's the build log: builds/166994513

Screenshot 2020-05-20 at 11 36 55
antongolub commented 4 years ago

It seems that semrel behaviour in CI differs from local. Logged: Inline plugin publish step was not loaded and was not triggered. So I just moved signal point to pre-success/fail stage and finally multi-sem-rel works now: builds/167111295. Unbelievable 🚀

qiwibot commented 4 years ago

:tada: This issue has been resolved in version 2.5.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

dhoulb commented 4 years ago

@antongolub Great work!

antongolub commented 4 years ago

This is not the end. It's necessary to fix semrel plugin config resolver.