ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.41k stars 3.69k forks source link

Prepackage script fails if OS reports odd number of cores #17025

Open f1ames opened 3 weeks ago

f1ames commented 3 weeks ago

📝 Provide detailed reproduction steps (if any)

IMPORTANT: This happens only if OS reports odd number of cores (see details below).

  1. Go to ckeditor5 repo.
  2. Run yarn (or whatever other commands are needed for repo initial setup).
  3. Run yarn run release:prepare-packages --compile-only --verbose.

✔️ Expected result

The script runs without error.

❌ Actual result

$ yarn run release:prepare-packages --compile-only --verbose
yarn run v1.22.22
$ node ./scripts/release/preparepackages.js --compile-only --verbose
Version 43.0.0
[STARTED] Verify the repository.
[SKIPPED] Verify the repository.
[STARTED] Check the release directory.
[COMPLETED] Check the release directory.
[STARTED] Preparation phase.
[SKIPPED] Preparation phase.
[STARTED] Compilation phase.
[STARTED] Preparing the "ckeditor5" package files.
[COMPLETED] Preparing the "ckeditor5" package files.
[STARTED] Preparing "ckeditor5-build-*" builds.
[OUTPUT] Status: 1/6.
[OUTPUT] Status: 2/6.
[OUTPUT] Status: 3/6.
[OUTPUT] Status: 4/6.
[OUTPUT] Status: 5/6.
[OUTPUT] Status: 6/6.
[COMPLETED] Preparing "ckeditor5-build-*" builds.
[STARTED] Compiling TypeScript in `ckeditor5-*` packages.
[FAILED] Cannot read properties of undefined (reading 'push')
[FAILED] Cannot read properties of undefined (reading 'push')
TypeError: Cannot read properties of undefined (reading 'push')
    at /Users/kkrzton/Workspace/ckeditor5/node_modules/@ckeditor/ckeditor5-dev-release-tools/lib/utils/executeinparallel.js:165:28
    at Array.reduce (<anonymous>)
    at getPackagesGroupedByThreads (/Users/kkrzton/Workspace/ckeditor5/node_modules/@ckeditor/ckeditor5-dev-release-tools/lib/utils/executeinparallel.js:158:18)
    at Object.executeInParallel (/Users/kkrzton/Workspace/ckeditor5/node_modules/@ckeditor/ckeditor5-dev-release-tools/lib/utils/executeinparallel.js:62:28)
    at async _Task.run (/Users/kkrzton/Workspace/ckeditor5/node_modules/listr2/dist/index.cjs:2049:11)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

❓ Possible solution

This happens only if OS reports odd number of cores. On my mac, require( 'os' ).cpus().length returns 11. This breaks logic in executeinparallel.js where some array indexes are calculated based on nr of cores and it ends up with indexes like 0.5, 1.5, etc:

https://github.com/ckeditor/ckeditor5-dev/blob/e84c7019a61fa31c233e961afed014c1c9303989/packages/ckeditor5-dev-release-tools/lib/utils/executeinparallel.js#L49

https://github.com/ckeditor/ckeditor5-dev/blob/e84c7019a61fa31c233e961afed014c1c9303989/packages/ckeditor5-dev-release-tools/lib/utils/executeinparallel.js#L159-L165

📃 Other details


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

pomek commented 2 weeks ago

I propose fixing it inside the release tools, as otherwise, we would need to update all repositories where we use the same rules.

So, if a user passed an odd number, let's try to round it to an integer.

pomek commented 3 days ago

Let's wait till the CKEditor 5 Dev ESM migration is merged to avoid conflicts with merging changes between branches.