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

The checkVersionAvailability() in ckeditor5-dev-release-tools does not work on Windows #17191

Closed
psmyrek opened this issue Oct 1, 2024 · 2 comments · Fixed by ckeditor/ckeditor5-dev#1018
Assignees
Labels
package:dev squad:platform Issue to be handled by the Platform team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@psmyrek
Copy link
Contributor

psmyrek commented Oct 1, 2024

The checkVersionAvailability() in ckeditor5-dev-release-tools does not work on Windows, because its arguments are wrapped inside ' by shellEscape(). Example command that is generated by this util looks like:

npm show '@ckeditor/ckeditor5-upload'@'43.0.0' version

Then, it is passed down to shelljs as:

npm show "'@ckeditor/ckeditor5-upload'@'43.0.0'" version

which throws an error:

npm error code ENOENT
npm error syscall open
npm error path D:\Projekty\ckeditor5\ckeditor\ckeditor5-upload'@'43.0.0'\package.json
npm error errno -4058
npm error enoent Could not read package.json: Error: ENOENT: no such file or directory, open 'D:\Projekty\ckeditor5\ckeditor\ckeditor5-upload'@'43.0.0'\package.json'
npm error enoent This is related to npm not being able to find a file.
npm error enoent

See https://github.com/ckeditor/ckeditor5-dev/blob/0e35f4972c7c8dfdef8055f9b0dbc0cca46003d1/packages/ckeditor5-dev-release-tools/lib/utils/checkversionavailability.js#L22

It works well on non-Windows environments.

Proposed solution

To make all npm-related commands cross-platform compatible, we could use either npm or pacote packages to fetch the required data from npm registry instead of calling shell command.

@psmyrek psmyrek added type:bug This issue reports a buggy (incorrect) behavior. squad:platform Issue to be handled by the Platform team. package:dev labels Oct 1, 2024
@pomek
Copy link
Member

pomek commented Oct 2, 2024

To make all npm-related commands

Could we modify only commands that use the registry directly? I mean, npm whoami could remain untouched.

@psmyrek
Copy link
Contributor Author

psmyrek commented Oct 2, 2024

Sure, it should work. Probably only commands with escaped arguments cause problems on Windows.

@pomek pomek self-assigned this Oct 2, 2024
@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. 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 Oct 3, 2024
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Oct 7, 2024
Other (release-tools): We do not spawn an npm process to download a package manifest from the npm registry. Instead, we send an HTTP request using the `pacote` package. Closes ckeditor/ckeditor5#17191.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Oct 7, 2024
@CKEditorBot CKEditorBot added this to the iteration 79 milestone Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:dev 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.

3 participants