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.

Sign up for GitHub

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

Jump to bottom

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

Closed
f1ames opened this issue Sep 2, 2024 · 3 comments · May be fixed by ckeditor/ckeditor5-dev#1011
Closed

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

f1ames opened this issue Sep 2, 2024 · 3 comments · May be fixed by ckeditor/ckeditor5-dev#1011
Assignees
@martnpaneq
Labels
squad:devops Issue to be handled by the Devops team. type:bug This issue reports a buggy (incorrect) behavior.
Milestone
upcoming

Comments

@f1ames
Copy link
Contributor

f1ames commented Sep 2, 2024

📝 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

  • Browser: Not relevant.
  • OS: macOS Sonoma 14.3 (Apple M3 Pro chip).
  • First affected CKEditor version: Not relevant.
  • Installed CKEditor plugins: Not relevant.

If you'd like to see this fixed sooner, add a 👍 reaction to this post.
@f1ames f1ames added type:bug This issue reports a buggy (incorrect) behavior. squad:devops Issue to be handled by the Devops team. labels Sep 2, 2024
@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Sep 9, 2024
@pomek
Copy link
Member

pomek commented Sep 13, 2024

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.

@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Sep 20, 2024
@pomek
Copy link
Member

pomek commented Sep 24, 2024

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

@martnpaneq martnpaneq self-assigned this Sep 25, 2024
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Sep 25, 2024
This was referenced Sep 26, 2024
Merged
Closed
Closed
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Sep 27, 2024
@pomek
Merge pull request #1015 from ckeditor/ck/17025
6127e07 
Fix (release-tools): Passing an odd number as a value of the `concurrency` option will not crash the `executeInParallel()` function. Closes ckeditor/ckeditor5#17025.
@pomek pomek mentioned this issue Sep 27, 2024
Draft
6 tasks
@pomek
Copy link
Member

pomek commented Sep 27, 2024

Fixed on an ESM migration branch (ckeditor/ckeditor5-dev#1011).

@pomek pomek closed this as completed Sep 27, 2024
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Sep 27, 2024
@CKEditorBot CKEditorBot added this to the upcoming milestone Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martnpaneq martnpaneq

Labels
squad:devops Issue to be handled by the Devops team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Milestone
upcoming
Development

Successfully merging a pull request may close this issue.

Converted ckeditor5-dev-* packages to ESM ckeditor/ckeditor5-dev
4 participants
@f1ames @pomek @CKEditorBot @martnpaneq