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

Limit symlink-dir version to <6.0 in symlink-vendor-directory.js #7364

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

stollr
Copy link
Contributor

@stollr stollr commented Apr 11, 2024

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
Related issues/PRs -
License MIT
Documentation PR -

What's in this PR?

The symlink-dir package is required in the symlink-vendor-directory.js. Version 6.0 was just released and it requires node >= 18.12. But node 18 comes with npm 9, which is currently not supported by Sulu.

By the limititation of symlink-dir's version to something lower than 6.0 we make sure that npx does not load an incompatible version.

Why?

Running cd assets/admin/ && npm install to build the admin frontend fails with the following error:

$ npm install

sulu-skeleton@ preinstall /srv/htdocs/sulu/assets/admin
node ../../vendor/sulu/sulu/preinstall.js

/srv/htdocs/sulu/vendor/sulu/sulu/symlink-vendor-directory.js:38
throw new Error('Error occured while creating symlink: ' + error);
^

Error: Error occured while creating symlink: Error: Command failed: npx symlink-dir ../../vendor node_modules/@sulu/vendor
npm ERR! code ENOTSUP
npm ERR! notsup Unsupported engine for symlink-dir@6.0.0: wanted: {"node":">=18.12"} (current: {"node":"14.21.3","npm":"6.14.18"})
npm ERR! notsup Not compatible with your version of node/npm: symlink-dir@6.0.0
npm ERR! notsup Not compatible with your version of node/npm: symlink-dir@6.0.0
npm ERR! notsup Required: {"node":">=18.12"}
npm ERR! notsup Actual: {"npm":"6.14.18","node":"14.21.3"}

npm ERR! A complete log of this run can be found in:
npm ERR! /home/stollr/.npm/_logs/2024-04-11T14_29_09_259Z-debug.log
Install for [ 'symlink-dir@<7.0' ] failed with code 1

at /srv/htdocs/sulu/vendor/sulu/sulu/symlink-vendor-directory.js:38:19
at ChildProcess.exithandler (child_process.js:390:5)
at ChildProcess.emit (events.js:400:28)
at maybeClose (internal/child_process.js:1088:16)
at Socket.<anonymous> (internal/child_process.js:446:11)
at Socket.emit (events.js:400:28)
at Pipe.<anonymous> (net.js:686:12)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! sulu-skeleton@ preinstall: node ../../vendor/sulu/sulu/preinstall.js
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the sulu-skeleton@ preinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

The symlink-dir package is required in the `symlink-vendor-directory.js`. Version 6.0 was just released and it requires node >= 18.12. But node 18 comes with npm 9, which is currently not supported by Sulu.

By the limititation of symlink-dir's version to something lower than 6.0 we make sure that npx does not load an incompatible version.
@stollr stollr changed the base branch from 2.5 to 2.4 April 11, 2024 15:30
@stollr
Copy link
Contributor Author

stollr commented Apr 11, 2024

I have changed the target branch to 2.4. Hope this is correct.

@alexander-schranz alexander-schranz merged commit 34aa302 into sulu:2.4 Apr 12, 2024
8 checks passed
@alexander-schranz
Copy link
Member

@stollr thank you!

@alexander-schranz alexander-schranz added the Bug Error or unexpected behavior of already existing functionality label Apr 12, 2024
@stollr
Copy link
Contributor Author

stollr commented Apr 12, 2024

@alexander-schranz Is it possible to create a new release with the fix? Our builds are currently failing because of this :-(

@CapsE
Copy link

CapsE commented Apr 15, 2024

A new release would be great indeed. We have 3 projects that need deployments but our pipelines are blocked by this. I also can't setup a new SULU project locally right now which blocks development. I'll try to implent the proposed change myself so I can at least continue working. Thanks @stollr for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error or unexpected behavior of already existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants