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

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

Closed
f1ames opened this issue Sep 2, 2024 · 3 comments · 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 · Fixed by ckeditor/ckeditor5-dev#1011
Assignees
Labels
squad:platform Issue to be handled by the Platform team. type:bug This issue reports a buggy (incorrect) behavior.

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:platform Issue to be handled by the Platform 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
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Sep 27, 2024
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
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
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Oct 2, 2024
Fix (release-tools): Passing an odd number as a value of the `concurrency` option will not crash the `executeInParallel()` function. Closes ckeditor/ckeditor5#17025.

Other: The CKEditor 5 Dev packages are now ESM.

Other: The `@ckeditor/jsdoc-plugins` package is no longer available as CKEditor 5 documentation uses TypeScript sources to prepare API docs. The package is no longer use by us anywhere. Hence, we decided to remove a dead code.

MAJOR BREAKING CHANGE (utils): The `builds.getDllPluginWebpackConfig()` function is now asynchronous now.

MAJOR BREAKING CHANGE (utils): The following functions from the `bundler` object are no longer available: `createEntryFile()`, `getEditorConfig()`, `getPlugins()`.

MAJOR BREAKING CHANGE (utils): The following functions from the `stream` object are no longer available: `isTestFile()`, `isSourceFile()`, `isJSFile()`.

MAJOR BREAKING CHANGE (utils): The `styles.themeLogger()` function is no longer exposed publicly.

MAJOR BREAKING CHANGE (tests): The IntelliJ Karma configuration path needs to be updated. Now, it ends with the `cjs` suffix instead of `js`.

MAJOR BREAKING CHANGE (docs): The package uses only TypeDoc to build the documentation. The `build()` function no longer supports `type` property in the configuration.

MAJOR BREAKING CHANGE (docs): An array of ignored files must be specified as `ignoreFiles` instead of negating a glob pattern in `sourceFiles`.

MAJOR BREAKING CHANGE: The following packages have been converted to ESM. To load them, use an `import` statement instead of `require()`.
  * [`@ckeditor/ckeditor5-dev-build-tools`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-build-tools)
  * [`@ckeditor/ckeditor5-dev-bump-year`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-bump-year)
  * [`@ckeditor/ckeditor5-dev-ci`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-ci)
  * [`@ckeditor/ckeditor5-dev-dependency-checker`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-dependency-checker)
  * [`@ckeditor/ckeditor5-dev-docs`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-docs)
  * [`@ckeditor/ckeditor5-dev-release-tools`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-release-tools)
  * [`@ckeditor/ckeditor5-dev-stale-bot`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-stale-bot)
  * [`@ckeditor/ckeditor5-dev-tests`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-tests)
  * [`@ckeditor/ckeditor5-dev-transifex`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-transifex)
  * [`@ckeditor/ckeditor5-dev-translations`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-translations)
  * [`@ckeditor/ckeditor5-dev-utils`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-utils)
  * [`@ckeditor/ckeditor5-dev-web-crawler`](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-web-crawler)
@CKEditorBot CKEditorBot modified the milestones: upcoming, iteration 79 Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:platform Issue to be handled by the Platform team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants