Skip to content
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

[rush] Provide an option to make the committed shrinkwrap capture the exact installation plan #1300

Closed
octogonz opened this issue May 27, 2019 · 5 comments · Fixed by #1850
Closed
Assignees
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start!

Comments

@octogonz
Copy link
Collaborator

octogonz commented May 27, 2019

Although rush install always produces a /deterministic/ outcome for a given input, the installation plan may not exactly correspond to the shrinkwrap file (aka "lockfile"). In other words, rush install is a deterministic function of common/config/rush/pnpm-lock.yaml file plus the package.json files for all the projects, not the shrinkwrap file alone.

Benefits: This feature was introduced to minimize spurious churn in the shrinkwrap file, which frequently causes annoying merge conflicts in a busy monorepo.

Drawbacks: People have found this feature counterintuitive, because it changes the meaning of the shrinkwrap file. Also in some situations it's important for the exact installation plan to be tracked by Git (e.g. when applying a hotfix to an old release branch, where you want to avoid disturbing any other package versions).

Algorithm details

Here's a summary of the algorithm when using PNPM:

There are 3 files:
A. common/config/rush/pnpm-lock.yaml - the "committed" version that is tracked by Git
B. common/temp/pnpm-lock.yaml - the version that PNPM sees when invoked in common/temp
C. common/temp/pnpm-lock-preinstall.yaml - a backup copy of pnpm-lock.yaml, for diagnostic purposes to compare with B

rush install reads A, makes some fixes to it, and writes out B and C before invoking pnpm install

pnpm install reads B and might rewrite it with some changes, making it different from C. (But by design, rush install does not copy these changes back to A; the deviations are allowable because they are supposed to be fully deterministic. The reason is to minimize churn that causes Git merge conflicts.)

rush update is like rush install but it does copy B -> A, but only when Rush determines that the changes are interesting.

rush update --full deletes B, then runs pnpm install to create B, then copies B -> A

Proposed changes

  1. Add a rush.json setting for disabling this optimization. Maybe we could call it minimizeShrinkwrapChurn, which would be true when the optimization is enabled.

  2. We could also add a command-line parameter such as rush update --resync-shrinkwrap to force copying common/temp/pnpm-lock.yaml --> common/config/rush/pnpm-lock.yaml if there are any differences.

  3. When the files are different, Rush install could print an informational message to the log indicating this situation. Something like: "The installation has differences from the shrinkwrap file because minimizeShrinkwrapChurn was enabled."

@iclanton

@octogonz octogonz added the enhancement The issue is asking for a new feature or design change label May 27, 2019
@octogonz
Copy link
Collaborator Author

@octogonz octogonz added effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start! labels Jun 5, 2019
@HarryGifford
Copy link

@sachinjoseph Do you know if there's been any progress on this?

@octogonz
Copy link
Collaborator Author

@iclanton could you clarify why this was closed? Was it fixed? Was it punted?

@sachinjoseph
Copy link
Member

@octogonz Turning on pnpm --frozen-lockfile in experiments also disables shrinkwrap churn optimization.

@sachinjoseph
Copy link
Member

@octogonz Do you mean that this issue should remain open until a --frozen-lockfile flag's equivalent is supported by Rush install on all the three package managers we currently support?

@iclanton iclanton moved this to Closed in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start!
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants