[rush-lib] Add a config to pnpm-config.json about injected install for cross subspace workspace dependencies #4674
Conversation
g-chao
requested review from
iclanton,
octogonz,
apostolisms,
D4N14L,
dmichon-msft and
patmill
as code owners
.github/workflows/ci.yml
Outdated
| - name: Rush test (rush-lib) | ||
| run: node apps/rush/lib/start-dev.js test --verbose --production --timeline | ||
| # - name: Rush test (rush-lib) | ||
| # run: node apps/rush/lib/start-dev.js test --verbose --production --timeline |
There was a problem hiding this comment.
As discussed with @octogonz offline, we may want to comment out this line temporary as it is not a correct set up.
Here is the problem:
For this ci.yaml file,
Line 46, it uses the rush version defined in rush.json to perform the install
Line 53, it uses the rush version defined in rush.json to perform the build and test
Line 59, it uses the rush version from rush-lib folder to perform the build and test
If I understand correctly here, Line 59 reuses the rush install results from line 46. However, the rush versions used on line 46 and line 59 are different
This causes CI check failure when updating pnpm-sync-lib version.
In this PR, we updated the pnpm-sync-lib version.
So, in the above CI configuration, Line 46, when it completes, it generates a pnpm-sync.json based the pnpm-sync-lib version bundled in rush version defined in rush.json.
Line 59, since we updated the pnpm-sync-lib version in this PR, when using rush version from rush-lib (which has a updated pnpm-sync-lib) folder to perform the build and test, it will throw an error saying pnpm-sync.json version is out of date!
It is due to Rush test (rush-lib) step reuses the installation from previous step, but the rush version used in the previous step is not same as this step.
There was a problem hiding this comment.
We will reenable this after the PR is merged.
However @iclanton I think it is fundamentally incorrect for this CI pipeline to invoke rush test using a different version of Rush than rush install.
There was a problem hiding this comment.
@g-chao fixed it by adding a separate rush install step
g-chao
force-pushed
the
chao/add-pnpm-sync-config
branch
from
60ccc13
to
e574231
Compare
…lwaysInjectDependenciesFromOtherSubspaces"
| @@ -55,5 +55,8 @@ jobs: | |||
| - name: Ensure repo README is up-to-date | |||
| run: node repo-scripts/repo-toolbox/lib/start.js readme --verify | |||
|
|
|||
| - name: Rush update (rush-lib) | |||
| run: node apps/rush/lib/start-dev.js update --recheck | |||
There was a problem hiding this comment.
@g-chao It is technically a bug that --recheck is needed here. I bet you can solve that by adding the pnpm-sync version to ILastInstallFlagJson, but we can try that in a separate PR to avoid holding up your work.
Summary
In the subspace design, we mentioned that injecting is required wherever a workspace:* dependency refers to a project in a separate subspace.
This PR is support this feature.
Details
We add a new configuration called
autoInjectedInstallForCrossSubspaceWorkspaceDependencytopnpm-config.json.When it is on, all cross subspace workspace dependencies will be injected install.
The default value is
false.How it was tested
Manually tested with the Rushstack locally.
Impacted documentation
N/A