Skip to content

[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

Merged
merged 4 commits into from
Feb 17, 2023

Conversation

dmichon-msft
Copy link
Contributor

@dmichon-msft dmichon-msft commented Feb 10, 2023

Summary

Adds another code path to @rushstack/rush-sdk with higher precedence than install-run-rush.js that will try to use a version of @microsoft/rush-lib provided via environment variable RUSH_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 the RUSH_LIB_PATH environment variable on process.env, so that it will propagate to child processes. When initializing, @rushtack/rush-sdk will check this environment variable before falling back to invoking install-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 of EnvironmentVariables.ts in @microsoft/rush-lib.

@dmichon-msft
Copy link
Contributor Author

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 install-run-rush.js, since that is the approach of last resort.

Copy link
Member

@TheLarkInn TheLarkInn left a comment

Choose a reason for hiding this comment

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

LGTM

@octogonz
Copy link
Collaborator

Impacted documentation

Readme for @rushstack/rush-sdk.

It also should probably be added to Rush's EnvironmentVariables.ts, and also the https://rushjs.io/pages/configs/environment_vars/ page.

Copy link
Collaborator

@octogonz octogonz left a 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 dmichon-msft merged commit 838c4ae into microsoft:main Feb 17, 2023
@sonnymarton
Copy link

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.

4 participants