fix(tools/node): merge NODE_OPTIONS with process environment, if set#5962
fix(tools/node): merge NODE_OPTIONS with process environment, if set#5962jamietanna wants to merge 2 commits intomainfrom
NODE_OPTIONS with process environment, if set#5962Conversation
| npm_config_update_notifier: 'false', | ||
| npm_config_fund: 'false', | ||
| NODE_OPTIONS: '--use-openssl-ca', | ||
| NODE_OPTIONS: getNodeOptions(penv.NODE_OPTIONS), |
There was a problem hiding this comment.
This is wrong, because this it written to file as: export NODE_OPTIONS=${NODE_OPTIONS---use-openssl-ca}
So i if renovate is setting NODE_OPTIONS it will discard our --use-openssl-ca option.
I'll refactor this, so it's added as node arg directly if the node version supports it.
base/src/cli/services/path.service.ts
Line 310 in 19b47c2
There was a problem hiding this comment.
i think this needs to be reverted, so we still set it if node options are not already set.
this is the reason why it's in node options and not a cli arg
There was a problem hiding this comment.
I've raised #6071 to document the existing behaviour
Through a new test, I seem to see that this is correctly loading in the --use-openssl-ca?
There was a problem hiding this comment.
this is still not the expected behavior.
sample:
on full docker image we run install-tool node.
if we now pass NODE_OPTIONS there, then they will be persisted.
so when renovate later runs with no NODE_OPTIONS, then it will use the unexpected persisted options.
fe4c27f to
d2fbb89
Compare
As noted in renovatebot/renovate#40942, we need to introduce a means to control the `NODE_OPTIONS` environment variable to specify the memory limits for a given Node child process. Before we do this, Containerbase needs to append the `--use-openssl-ca` option, instead of replacing the existing `NODE_OPTIONS`. Co-authored-by: Claude Sonnet 4.5 <jamie.tanna+github-copilot@mend.io>
d2fbb89 to
7a26a6f
Compare
viceice
left a comment
There was a problem hiding this comment.
it still doesn't work as we expect
| npm_config_update_notifier: 'false', | ||
| npm_config_fund: 'false', | ||
| NODE_OPTIONS: '--use-openssl-ca', | ||
| NODE_OPTIONS: getNodeOptions(penv.NODE_OPTIONS), |
There was a problem hiding this comment.
this is still not the expected behavior.
sample:
on full docker image we run install-tool node.
if we now pass NODE_OPTIONS there, then they will be persisted.
so when renovate later runs with no NODE_OPTIONS, then it will use the unexpected persisted options.
As noted in renovatebot/renovate#40942,
we need to introduce a means to control the
NODE_OPTIONSenvironmentvariable to specify the memory limits for a given Node child process.
Before we do this, Containerbase needs to append the
--use-openssl-caoption, instead of replacing the existing
NODE_OPTIONS.