-
Notifications
You must be signed in to change notification settings - Fork 631
[rush-sdk] Use rush-lib from parent process #3972
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
Conversation
Complementary to #3878 ; this code path should be tried before the code path added there (since its a quick read of a variable), but both should be tried before falling back to invoking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
8332156
to
dacf936
Compare
common/changes/@microsoft/rush/rush-sdk-env_2023-02-10-23-15.json
Outdated
Show resolved
Hide resolved
It also should probably be added to Rush's EnvironmentVariables.ts, and also the https://rushjs.io/pages/configs/environment_vars/ page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with various suggestions
@dmichon-msft please see the issue recently reported. [[rush] Installing @latest rush with a downgraded rushVersion specified in rush.json · Issue #3979 · microsoft/rushstack (github.com)](#3979) |
Summary
Adds another code path to
@rushstack/rush-sdk
with higher precedence thaninstall-run-rush.js
that will try to use a version of@microsoft/rush-lib
provided via environment variableRUSH_LIB_PATH
. This environment variable will get set by@microsoft/rush-lib
when loaded and propagated to child processes.Details
When the
Rush
module in@microsoft/rush-lib
loads, it sets theRUSH_LIB_PATH
environment variable onprocess.env
, so that it will propagate to child processes. When initializing,@rushtack/rush-sdk
will check this environment variable before falling back to invokinginstall-run-rush.js
.How it was tested
Added unit tests to
@rushstack/rush-sdk
to test primary loader code paths.Validated that
process.env.RUSH_LIB_PATH
was set when the unit tests were invoked via the new build of Rush (and needed to update the unit tests to clear the value).Impacted documentation
Readme for
@rushstack/rush-sdk
. Comments on public API ofEnvironmentVariables.ts
in@microsoft/rush-lib
.