Allow selecting the Node.js binary OSD uses #3508

AMoo-Miki · 2023-02-28T20:37:10Z

Description

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD.

The startup scripts check OSD_NODE_HOME and NODE_HOME before falling back to use the bundled or system-wide Node.js binary.
Update package.json to replace node with use_node.
Update the build scripts to copy use_node into releases
Make tests that execute node internally, honor the binary being used.

Issues Resolved

Fixes #1219
Fixes #3502
Partially serves #3095

Check List

peterzhuamazon

Hi @AMoo-Miki,

Great change and I have a few questions.

Thanks.

scripts/use_node

peterzhuamazon · 2023-02-28T21:25:52Z

scripts/use_node

@@ -0,0 +1,97 @@
+#!/bin/sh


Not recommended as ubuntu is using ash linking to sh.

This is what OSD used before but I see that OS uses #!/usr/bin/env bash. Shall I use that?

Ahhh, if that is the case better just keep #!/bin as not all the docker images install env by default as well.
On debian I have already force a #!/bin/bash just to avoid.

I really dont have too much solution to this generalized script tho.

If OS is already using env, then we probably can just use.
At least env is more common than which not being installed.
Thanks.

@peterzhuamazon I did some reading and most of what I found reasonable indicate a preference for using #!/bin/sh.
https://stackoverflow.com/a/55927235
https://unix.stackexchange.com/a/496523
https://www.ixsystems.com/blog/migrating-from-linux-to-freebsd/

Thanks @AMoo-Miki !

I have no issue with that then.

So it was intentional to not make this use_node.sh and set as bin/bash? If so should we migrate the other shell scripts to this format?

@AMoo-Miki any comment about the above ^

We should standardize what we do. I will raise an issue for it.

scripts/use_node

codecov-commenter · 2023-02-28T21:47:45Z

Codecov Report

Merging #3508 (90ef3d6) into main (1e127b0) will increase coverage by 0.05%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3508      +/-   ##
==========================================
+ Coverage   66.41%   66.47%   +0.05%     
==========================================
  Files        3206     3206              
  Lines       61597    61597              
  Branches     9503     9503              
==========================================
+ Hits        40909    40944      +35     
+ Misses      18410    18380      -30     
+ Partials     2278     2273       -5

Flag	Coverage Δ
Linux	`66.41% <ø> (+<0.01%)`	⬆️
Windows	`66.41% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

see 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

peterzhuamazon

Just a nitpik

scripts/use_node

kavilla · 2023-03-10T22:29:36Z

scripts/use_node

+
+SCRIPT="$0"
+
+OS=$(uname -s | awk '{print tolower}')


awk '{print tolower}'

is failing for me so I don't get an OS. $OS outputs as empty for me on my Ubuntu

Thanks. Peter made this suggestion. I will revert back to what we had before.

I reverted this piece.

@peterzhuamazon I had to revert this change.

scripts/use_node

kavilla · 2023-03-14T10:35:51Z

Did we want to update stuff where we reference node like these and update the plugin generation scripts:

https://github.com/opensearch-project/OpenSearch-Dashboards/tree/main/packages/osd-plugin-helpers
OpenSearch-Dashboards/packages/osd-plugin-generator/src/cli.ts

Line 85 in 1e127b0

usage: 'node scripts/generate_plugin',
OpenSearch-Dashboards/packages/osd-plugin-generator/template/package.json.ejs

Line 7 in 1e127b0

"plugin-helpers": "node ../../scripts/plugin_helpers",

And update the packages package.json like:

OpenSearch-Dashboards/packages/osd-ace/package.json

Line 8 in 1e127b0

"build": "node ./scripts/build.js",

AMoo-Miki · 2023-03-14T20:36:47Z

I did not change the instructions since use_node is meant for internal execution and users get to choose which node they want when they use it to execute individual js files.

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]>

kavilla

Awesome!

Barring the creation of the follow standardization and some dev documentation. LGTM!

joshuarrrr

My bash-fu is incredibly limited, so I'm not sure I'm able to catch much in the actual implementation, but looks good to me!

joshuarrrr · 2023-03-20T23:16:09Z

src/dev/build/tasks/bin/scripts/opensearch-dashboards-keystore.bat


-ENDLOCAL
+ENDLOCAL


nit - missing blank line?

joshuarrrr · 2023-03-20T23:19:12Z

@AMoo-Miki @kavilla This is just targeting 2.x, right? No need to backport to 1.x?

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> (cherry picked from commit cb472dc) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]>

AMoo-Miki · 2023-03-22T19:54:31Z

@AMoo-Miki @kavilla This is just targeting 2.x, right? No need to backport to 1.x?

Correct. 1.x will need some extra love to use this.

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]>

…3633) * added prepare command Signed-off-by: kappassov <[email protected]> * Use mirrors to download Node.js binaries to escape sporadic 404 errors (#3619) Issue Resolved: #3618 Signed-off-by: Anan Zhuang <[email protected]> * Allow selecting the Node.js binary OSD uses (#3508) New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> * Clean up and rebuild @osd/pm (#3570) * Fix the build failures * Removed some unused packages * Update gist url templates in Lychee's exclusion list Signed-off-by: Miki <[email protected]> * Refactor dev tool console to use opensearch-js client to send request (#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch Signed-off-by: Su <[email protected]> * updates after the 1st review Signed-off-by: kappassov <[email protected]> --------- Signed-off-by: kappassov <[email protected]> Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: Su <[email protected]> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Zhongnan Su <[email protected]>

…3633) * added prepare command Signed-off-by: kappassov <[email protected]> * Use mirrors to download Node.js binaries to escape sporadic 404 errors (#3619) Issue Resolved: #3618 Signed-off-by: Anan Zhuang <[email protected]> * Allow selecting the Node.js binary OSD uses (#3508) New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> * Clean up and rebuild @osd/pm (#3570) * Fix the build failures * Removed some unused packages * Update gist url templates in Lychee's exclusion list Signed-off-by: Miki <[email protected]> * Refactor dev tool console to use opensearch-js client to send request (#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch Signed-off-by: Su <[email protected]> * updates after the 1st review Signed-off-by: kappassov <[email protected]> --------- Signed-off-by: kappassov <[email protected]> Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: Su <[email protected]> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Zhongnan Su <[email protected]> (cherry picked from commit b0146e7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…3633) (#3651) * added prepare command * Use mirrors to download Node.js binaries to escape sporadic 404 errors (#3619) Issue Resolved: #3618 * Allow selecting the Node.js binary OSD uses (#3508) New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. * Clean up and rebuild @osd/pm (#3570) * Fix the build failures * Removed some unused packages * Update gist url templates in Lychee's exclusion list * Refactor dev tool console to use opensearch-js client to send request (#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch * updates after the 1st review --------- (cherry picked from commit b0146e7) Signed-off-by: kappassov <[email protected]> Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: Su <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Zhongnan Su <[email protected]>

…pensearch-project#3633) * added prepare command Signed-off-by: kappassov <[email protected]> * Use mirrors to download Node.js binaries to escape sporadic 404 errors (opensearch-project#3619) Issue Resolved: opensearch-project#3618 Signed-off-by: Anan Zhuang <[email protected]> * Allow selecting the Node.js binary OSD uses (opensearch-project#3508) New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> * Clean up and rebuild @osd/pm (opensearch-project#3570) * Fix the build failures * Removed some unused packages * Update gist url templates in Lychee's exclusion list Signed-off-by: Miki <[email protected]> * Refactor dev tool console to use opensearch-js client to send request (opensearch-project#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch Signed-off-by: Su <[email protected]> * updates after the 1st review Signed-off-by: kappassov <[email protected]> --------- Signed-off-by: kappassov <[email protected]> Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: Su <[email protected]> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Zhongnan Su <[email protected]>

New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> Signed-off-by: David Sinclair <[email protected]>

…pensearch-project#3633) * added prepare command Signed-off-by: kappassov <[email protected]> * Use mirrors to download Node.js binaries to escape sporadic 404 errors (opensearch-project#3619) Issue Resolved: opensearch-project#3618 Signed-off-by: Anan Zhuang <[email protected]> * Allow selecting the Node.js binary OSD uses (opensearch-project#3508) New reusable use_node scripts take care of selecting the correct Node.js binary based OSD_NODE_HOME, OSD_HOME, and whatever is in the PATH. It also takes care of finding the binary on FreeBSD. * The startup scripts check `OSD_NODE_HOME` and `NODE_HOME` before falling back to use the bundled or system-wide Node.js binary. * Update `package.json` to replace `node` with `use_node`. * Update file templates and shell scripts to internally use `use_node`. * Update the build scripts to copy `use_node` into releases * Make tests that execute node internally, honor the binary being used. Signed-off-by: Miki <[email protected]> * Clean up and rebuild @osd/pm (opensearch-project#3570) * Fix the build failures * Removed some unused packages * Update gist url templates in Lychee's exclusion list Signed-off-by: Miki <[email protected]> * Refactor dev tool console to use opensearch-js client to send request (opensearch-project#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch Signed-off-by: Su <[email protected]> * updates after the 1st review Signed-off-by: kappassov <[email protected]> --------- Signed-off-by: kappassov <[email protected]> Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: Su <[email protected]> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Zhongnan Su <[email protected]> Signed-off-by: David Sinclair <[email protected]>

joshuali925 · 2023-07-11T01:11:38Z

scripts/use_node

+while [ -h "$SCRIPT" ] ; do
+  loc=$(ls -ld "$SCRIPT")
+  # Drop everything prior to ->
+  link=$(expr "$loc" : '.*-> \(.*\)$')


why not something like realpath "$SCRIPT"?

nvm i just found realpath is not part of posix

AMoo-Miki requested a review from a team as a code owner February 28, 2023 20:37

AMoo-Miki force-pushed the allow-alt-node branch from 54526fc to 0556cfd Compare February 28, 2023 21:08

peterzhuamazon reviewed Feb 28, 2023

View reviewed changes

scripts/use_node Outdated Show resolved Hide resolved

AMoo-Miki force-pushed the allow-alt-node branch from 0556cfd to a149fad Compare February 28, 2023 22:03

peterzhuamazon reviewed Mar 1, 2023

View reviewed changes

scripts/use_node Outdated Show resolved Hide resolved

AMoo-Miki force-pushed the allow-alt-node branch from a149fad to 3cb0909 Compare March 1, 2023 18:29

AMoo-Miki mentioned this pull request Mar 2, 2023

Adding support for OSD_NODE_HOME #2208

Closed

7 tasks

AMoo-Miki requested a review from peterzhuamazon March 6, 2023 22:41

ashwin-pc assigned kavilla Mar 7, 2023

AMoo-Miki force-pushed the allow-alt-node branch from b63875d to bce4f0a Compare March 8, 2023 00:31

kavilla reviewed Mar 10, 2023

View reviewed changes

scripts/use_node Show resolved Hide resolved

AMoo-Miki force-pushed the allow-alt-node branch 2 times, most recently from 0762642 to 8e0d6a3 Compare March 13, 2023 22:50

AMoo-Miki force-pushed the allow-alt-node branch 2 times, most recently from 8c38067 to 88f95e3 Compare March 14, 2023 19:35

AMoo-Miki force-pushed the allow-alt-node branch from 88f95e3 to 90ef3d6 Compare March 14, 2023 20:56

kavilla approved these changes Mar 16, 2023

View reviewed changes

joshuarrrr approved these changes Mar 20, 2023

View reviewed changes

src/dev/build/tasks/bin/scripts/opensearch-dashboards-keystore.bat

ENDLOCAL

ENDLOCAL

Copy link

Member

joshuarrrr Mar 20, 2023

Choose a reason for hiding this comment

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

nit - missing blank line?

joshuarrrr added backport 2.x v2.7.0 labels Mar 20, 2023

joshuarrrr merged commit cb472dc into opensearch-project:main Mar 20, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 20, 2023

[Backport 2.x] Allow selecting the Node.js binary OSD uses #3637

Merged

kavilla mentioned this pull request Mar 28, 2023

Use scripts/use_node instead of node #3719

Open

seraphjiang mentioned this pull request Apr 28, 2023

Add consolidated release notes for 2.7.0 opensearch-project/opensearch-build#3452

Merged

joshuali925 reviewed Jul 11, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow selecting the Node.js binary OSD uses #3508

Allow selecting the Node.js binary OSD uses #3508

AMoo-Miki commented Feb 28, 2023 •

edited

Loading

peterzhuamazon left a comment

peterzhuamazon Feb 28, 2023

AMoo-Miki Feb 28, 2023

peterzhuamazon Feb 28, 2023 •

edited

Loading

peterzhuamazon Feb 28, 2023

peterzhuamazon Feb 28, 2023

AMoo-Miki Mar 1, 2023

peterzhuamazon Mar 1, 2023

kavilla Mar 10, 2023

kavilla Mar 14, 2023

AMoo-Miki Mar 16, 2023

codecov-commenter commented Feb 28, 2023 •

edited

Loading

peterzhuamazon left a comment

kavilla Mar 10, 2023

AMoo-Miki Mar 13, 2023

AMoo-Miki Mar 13, 2023

AMoo-Miki Mar 13, 2023

kavilla commented Mar 14, 2023

AMoo-Miki commented Mar 14, 2023 •

edited

Loading

kavilla left a comment

joshuarrrr left a comment

joshuarrrr Mar 20, 2023

joshuarrrr commented Mar 20, 2023

AMoo-Miki commented Mar 22, 2023

joshuali925 Jul 11, 2023 •

edited

Loading

Allow selecting the Node.js binary OSD uses #3508

Allow selecting the Node.js binary OSD uses #3508

Conversation

AMoo-Miki commented Feb 28, 2023 • edited Loading

Description

Issues Resolved

Check List

peterzhuamazon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Feb 28, 2023 • edited Loading

Codecov Report

peterzhuamazon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla commented Mar 14, 2023

AMoo-Miki commented Mar 14, 2023 • edited Loading

kavilla left a comment

Choose a reason for hiding this comment

joshuarrrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuarrrr commented Mar 20, 2023

AMoo-Miki commented Mar 22, 2023

joshuali925 Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

AMoo-Miki commented Feb 28, 2023 •

edited

Loading

peterzhuamazon Feb 28, 2023 •

edited

Loading

codecov-commenter commented Feb 28, 2023 •

edited

Loading

AMoo-Miki commented Mar 14, 2023 •

edited

Loading

joshuali925 Jul 11, 2023 •

edited

Loading