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

Fix #1247 - Maven build use node from PATH on mac os #1342

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

zulus
Copy link
Contributor

@zulus zulus commented Sep 24, 2023

Introduce NodeJSManager helpers to always produce ProcessBuilder with commands:

node_executable npm_location rest commands

these command is also used in NpmLaunchDelegate!

@zulus zulus force-pushed the node-path branch 5 times, most recently from 4bbd4a0 to d0cebb1 Compare September 25, 2023 09:30
@vrubezhny
Copy link
Contributor

@zulus I don't get how does it help to use embedded Node.js instead of a version that is found and used when npm (or NPM.CMD) is executed with

#!/usr/bin/env node

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

@zulus I don't get how does it help to use embedded Node.js instead of a version that is found and used when npm (or NPM.CMD) is executed with

#!/usr/bin/env node

Because rather than just run:
$: npm install something

Based on executable location search for cli.js (or npm-cli.js) and execute:

$ path_to_npm_builtin_or_from_property path_to_cli.js install something

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

Just check console log for tests (MacOS, Windows, Linux)

@vrubezhny
Copy link
Contributor

Won't this path_to_cli.js still be executed with #!/usr/bin/env node?

Just check console log for tests (MacOS, Windows, Linux)

I had Node.js 20 configured for this purpose on MacOS GitHub build action :)

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

Won't this path_to_cli.js still be executed with #!/usr/bin/env node?

Just check console log for tests (MacOS, Windows, Linux)

I had Node.js 20 configured for this purpose on MacOS GitHub build action :)

No, `#!' is run structure for unix "*sh" only, for node (any js interpreter) is just a comment, in same way works "runnable" PHP/python/ruby scripts. On windows npm.cmd, doing nearly same as my path, search for node.exe, cmd.js and later just run command "node jsfile other params"

@vrubezhny
Copy link
Contributor

On windows npm.cmd, doing nearly same as my path, search for node.exe, cmd.js and later just run command "node jsfile other params

So, npm is taken from your path (from the given cli.js path), while the node.js executable is still the system one?

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

After this path no both will be directly from embedder

Copy link
Contributor

@vrubezhny vrubezhny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@vrubezhny vrubezhny merged commit 5d992e8 into eclipse-wildwebdeveloper:master Sep 27, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants