feat: add git-branch-lockfile config to generate lockfile in each branch #4475

chengcyber · 2022-03-24T16:06:57Z

Summary

Ref #4324

Detail

Add a new setting called git-branch-lockfile specified .npmrc to turn on lockfile assembly feature.
When this feature is on, pnpm install will update pnpm-lock.<branch_name>.yaml instead of pnpm-lock.yaml, this helps avoiding PR conflicts on a single pnpm-lock.yaml file.
Add a new arg called --merge-git-branch-lockfiles for pnpm install. After PR merged into the main branch, run pnpm install --merge-git-branch-lockfiles to update the main lockfile and remove the others.

How to test it

Unit tests are added to test this feature, including

lockfile-file: read, write, lockfileName
core/install: gitBranchLockfile & mergeGitBranchLockfiles

zkochan · 2022-03-25T14:55:06Z

Thanks for working on this!

packages/lockfile-file/src/gitChecks.ts

packages/lockfile-file/src/lockfileName.ts

packages/lockfile-file/src/read.ts

zkochan · 2022-03-26T01:36:31Z

Add a new arg called --clean-git-branch-lockfiles for pnpm install. After PR merged into the main branch, run pnpm install --clean-git-branch-lockfiles to update the main lockfile and remove the others.

As far as I understand from your changes, the branch lockfiles are completely ignored. They are just removed. What we need instead is something like a --merge-lockfiles option, which would merge the branch lockfiles with the main lockfile. I think for the merge to work, it is enough to read both the branch and the main lockfile and extend the packages field of the branch lockfile with the packages field of the main lockfile. pnpm should then do everything automaically.

I mean this field:

pnpm/pnpm-lock.yaml

Line 3579 in bc35dff

packages:

chengcyber · 2022-03-26T08:40:00Z

Updates:

Syntax/logic tweaks
extract gitChecks to @pnpm/git-utils

As far as I understand from your changes, the branch lockfiles are completely ignored. They are just removed.

Yes

What we need instead is something like a --merge-lockfiles option, which would merge the branch lockfiles with the main lockfile. I think for the merge to work, it is enough to read both the branch and the main lockfile and extend the packages field of the branch lockfile with the packages field of the main lockfile.

After a second thought, i think it's essential to merge something into the main lockfile(packages field as you said). therefore i got some questions:

Assuming there are multiple lockfiles, pnpm-lock.yaml, pnpm-lock.a.yaml, pnpm-lock.b.yaml, pnpm-lock.c.yaml.

Q1: Does merge order matter?

If so, Q2: How to define order?

From git, it might be main <- a <- b <- c. Is it the real order? One might be run `pnpm install` on branch c before a and b, but the MR for c is merged later than a and b.

Q3: Can i use timestamp in fs?

Not sure. `pnpm install` might be run in different machines, I wonder this would makes `timestamp` unreliable.

For packages field, it seems order doesn't matter.

Q4: Conflicts among those lockfiles.

Like the conflicts on `pnpm-lock.yaml`. Those branch lockfiles conflicts as well. Is there any merge algorithm in codebase?

It seems conflicts doesn't a big issue when merging

~~What do you think? @zkochan~~

I will refactor cleanGitBranchLockfiles to mergeGitBranchLockfiles

zkochan · 2022-03-26T12:29:06Z

I think order doesn't matter (sort the branches alphabetically). The only thing that matters is that you merge the branch lockfiles into the main lockfile. So the entries in the branch lockfiles should have bigger priority than the ones in the main lockfile.

chengcyber · 2022-03-26T14:36:28Z

Updates:

packages in lockfiles on branch are assigned into the main lockifile when --merge-git-branch-lockfiles

I think order doesn't matter (sort the branches alphabetically).

Q: I wonder why sort the branches when the order doesn't matter?

zkochan · 2022-03-26T14:37:39Z

Q: I wonder why sort the branches when the order doesn't matter?

To make it deterministic.

chengcyber · 2022-03-26T14:39:04Z

To make it deterministic.

Got it. For now, i don't sort everything. but i think eventually the keys of the packages field will be sorted by pnpm automatically.

packages/headless/src/index.ts

packages/lockfile-file/src/read.ts

packages/lockfile-file/src/lockfileName.ts

zkochan · 2022-03-26T15:09:05Z

What will happen if I rebase my branch from the main branch? Should I regenerate my branch lockfile?

chengcyber · 2022-03-26T15:50:22Z

I would like to write down the whole workflow from a user perspective.

When git-branch-lockfiles=true in .npmrc

User A checkout a branch names feat-a, and run pnpm install
The first time pnpm-lock.feat-a.yaml is missing. So the readWantedLockfile returns the content of pnpm-lock.yaml
pnpm logic...
The new lockfile is written to pnpm-lock.feat-a.yaml (pnpm-lock.yaml no changes)
Dependencies change, rerun pnpm install, every lockfile changes are applied into pnpm-lock.feat-a.yaml
...
User A rebases the main branch to feat-a branch. and run pnpm install
readWantedLockfile returns the content of pnpm-lock.feat-a.yaml

Note:

When useGitBranchLockfile = true, readWantedLockfile first try to return the content of pnpm-lock.<branch>.yaml, if it doesn't exists, returns the content of pnpm-lock.yaml. Expect the case when useGitBranchLockfile = true && mergeGitBranchLockfiles = true, readWantedLockfile returns the content pnpm-lock.yaml, ignore other branch lock files.

In a nutshell, every lockfile changes are applied to the branch lockfile.

User A,B,C creates MR A,B,C merged into the main branch.
There are three lockfiles pnpm-lock.feat-a.yaml, pnpm-lock.feat-b.yaml, pnpm-lock.feat-c.yaml in the main branch
If i run pnpm install w/o args, pnpm-lock.main.yaml will be created
If i run pnpm install --merge-git-branch-lockfiles, readWantedLockfile returns the content of pnpm-lock.yaml, and merge the packages field of the all other lockfiles into the main Lockfile.
pnpm logic...
writeWantedLockfile writes the new lockfile into pnpm-lock.yaml
cleanGitBranchLockfiles is called to remove all other branch lock files.

Note:

For now, there is no differences between the default branch and the feature branches. pnpm install --merge-git-branch-lockfiles is a explicit way to merge branch lockfiles.

In a nutshell, no magic in the git default branch.

I thought about other designs:

Treat git default branch specially, if run pnpm install under the main branch, automatically set --merge-git-branch-lockfiles

Pros:

Convenience

Cons:

Implicit might be confusing.
Lack of flexibility, what if user want to do merge-git-branch-lockfiles on other branch instead of the main branch. (Like some repo set main as default but develops mainly on next branch)

A new setting maybe called merge-git-branch-lockfiles-branch-pattern (Bad naming :\ ). User can specify

merge-git-branch-lockfiles-branch-pattern[]=main
merge-git-branch-lockfiles-branch-pattern[]=release-*
merge-git-branch-lockfiles-branch-pattern[]=next

When pnpm install runs in those branches, set --merge-git-branch-lockfiles automatically.

Pros:

Convenience
Flexibility

Cons:

Implicit might be confusing

wbern · 2022-03-27T07:11:49Z

I think you nailed the design, but that's just me! Being dependent on git for this feature is perhaps unfortunate to some but I think we can afford to be opinionated here as the original problem is with regards to git merge conflicts.

chengcyber · 2022-03-27T09:34:36Z

Hey @wbern , thank you to push this issue move forward. Since we are targeting git merge conflicts, the solution depends on git seems reasonable :)

zkochan · 2022-03-27T20:43:15Z

The new lockfile is written to pnpm-lock.feat-a.yaml (pnpm-lock.yaml no changes)

This should happen only if there were changes to the lockfile.

If i run pnpm install w/o args, pnpm-lock.main.yaml will be created

I don't get it, so you'll have to remember to always run pnpm install --merge-git-branch-lockfiles on the main branch? I guess it might be fine if a user will never directly push to the main branch but only PRs will be used.

zkochan · 2022-03-27T23:49:31Z

packages/config/src/index.ts

+  pnpmConfig.useGitBranchLockfile = (() => {
+    if (typeof pnpmConfig['gitBranchLockfile'] === 'boolean') return pnpmConfig['gitBranchLockfile']
+    return false
+  })()


Why is this needed? The default value of use-git-branch-lockfile is set to false at line 184

The idea is to keep the same code style with useLockfile

packages/core/test/install/gitBranchLockfile.test.ts

zkochan · 2022-03-28T00:12:38Z

packages/lockfile-file/src/read.ts

+      if (opts.mergeGitBranchLockfiles) {
+        result.lockfile = await _mergeGitBranchLockfiles(result.lockfile, pkgPath, pkgPath, opts)
+      }


should this be inside the loop?

zkochan · 2022-03-28T00:17:11Z

packages/lockfile-file/test/read.test.ts

+  })
+})
+
+test('readWantedLockfile() when useGitBranchLockfile and mergeGitBranchLockfiles', async () => {


Is there a scenario in which useGitBranchLockfile is false and mergeGitBranchLockfiles is true?

It could happen. The behavior is same as useGitBranchLockfile = true and mergeGitBranchLockfiles = true. That means useGitBranchLockfile is of no use when mergeGitBranchLockfiles = true

zkochan · 2022-03-28T00:18:18Z

pnpm-lock.yaml

@@ -8278,7 +8295,7 @@ packages:
    resolution: {integrity: sha512-oUwtPJ1W0SKD0Tr+wqu92c5xuCeQqB3hSCHasn/ZgjFdA9iDGNkNf2Zi9ztY7X+hNuMib23LNGRm6+uN+KLE3g==}
    engines: {node: '>=8.10.0'}
    peerDependencies:
-      eslint: '*'
+      eslint: '>=5.16.0 || *'


Update your pnpm version. You are not using the latest v6, that is why the lockfile gets changed like this.

zkochan · 2022-03-29T00:46:18Z

This is a big change, so I recommend you to publish a version from this branch and try it in your team. Gather feedback to see if it works for you.

chengcyber · 2022-03-29T15:20:56Z

This is a big change, so I recommend you to publish a version from this branch and try it in your team. Gather feedback to see if it works for you.

This make sense. Is it possible to publish an alpha version officially?

zkochan · 2022-03-29T23:08:23Z

🚢 pnpm@7.0.0-pr4475.0

nikoladev · 2022-03-30T14:29:54Z

🚢 pnpm@7.0.0-pr4475.0

@zkochan FYI, running pnpm add -g pnpm@^7.0.0-beta.2 installs the version from this PR. I assume because the p in -pr4475.0 has a higher value than the b in -beta.2, so by using ^ users will inadvertently get the pnpm version from this PR.

It probably also means that a future 7.0.0-beta.3 version will be ignored for 7.0.0-pr4475.0 if using ^ when installing, though I can't say that for sure.

I don't know what a good solution is, though.

zkochan · 2022-03-30T14:32:55Z

This is confusing indeed. Maybe we need to change how canary versions are installed and disallow installing them by range. So pnpm@^7.0.0-beta.2 would install 7.0.0-beta.2 even if 7.0.0-beta.3 is out.

nikoladev · 2022-03-30T15:36:22Z

Hmmm, that seems like it would break expectations. Because I would want to get newer pre-release versions of a package if I use ^7.0.0-beta.2.

Maybe we could change the naming of test releases like this one? So we could name it something like 0.0.0-pr4475.0. This would still allow users to use ^ for pre-releases but remove the confusion (because I don't think anyone will be installing a pre-release of version 0.0.0).

chengcyber · 2022-04-01T22:14:23Z

Hi @zkochan ,

I am adapting our monorepo to pnpm@7, since this alpha version is based on pnpm@7. I run into the NODE_PATH issue in isolated node link mode, which should be fixed by #4513

Latest main branch rebased, could you kindly publish an alpha version for this PR?

zkochan · 2023-09-15T21:48:28Z

Hey @chengcyber

I just realized that we don't have any information about this in the docs. Even the settings are not documented. We need to add a new page about this feature and describe how to use it.

chengcyber · 2023-09-23T13:37:37Z

Hi @zkochan, you are right. I created a PR to add documents for this feature at pnpm/pnpm.io#455

Crayon-ShinChan · 2023-12-09T05:39:08Z

vercel can't detect the file pnpm-lock.branch.yaml, is there any solution for this?

Now specific issue is:

create a new branch, which has pnpm-lock.yaml originally
When I add a new package into package.json, then I got a pnpm-lock.branch.yaml
push code to vercel, it will deploy my code and vercel use pnpm install --frozen-lockfile

So I ended up got the error in vercel:

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with package.json

One way to solve this is to delete pnpm-lock.yaml file in my branch, but according to https://pnpm.io/git. This is not an encouraged way due to pnpm-lock.yaml or pnpm-lock.branch.yaml help us:

it enables faster installation for CI and production environments, due to being able to skip package resolution
it enforces consistent installations and resolution between development, testing, and production environments, meaning the packages used in testing and production will be exactly the same as when you developed your project

Is there any better solution for this?

chengcyber marked this pull request as ready for review March 25, 2022 06:30

chengcyber requested a review from zkochan as a code owner March 25, 2022 06:30

zkochan requested changes Mar 26, 2022

View reviewed changes

chengcyber requested a review from zkochan March 26, 2022 14:36

zkochan reviewed Mar 26, 2022

View reviewed changes

chengcyber requested a review from zkochan March 27, 2022 09:34

zkochan reviewed Mar 28, 2022

View reviewed changes

chengcyber force-pushed the feat-lockfile-assembly branch from da14d53 to 7f307b4 Compare April 1, 2022 22:12

chengcyber added 3 commits April 2, 2022 01:34

feat: add git-branch-lockfile config to generate lockfile in each branch

771978d

feat: add --clean-git-branch-lockfiles arg to pnpm install

781addd

chore: test lockfile name

3713348

kenrick95 mentioned this pull request Aug 16, 2022

Lockfile assembly: pnpm install --merge-git-branch-lockfiles does not update main lockfile #5212

Closed

This was referenced Aug 16, 2022

chore(deps): update all non-major dependencies ddadaal/tsgrpc#4

Merged

Update pnpm to v7.9.3 joakimgunst/msp-next#44

Merged

Update pnpm to v7 yukukotani/eslint-plugin-chakra-ui#63

Merged

chore(deps): update pnpm to v7.9.5 elonehoo/vitem#10

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add git-branch-lockfile config to generate lockfile in each branch #4475

feat: add git-branch-lockfile config to generate lockfile in each branch #4475

chengcyber commented Mar 24, 2022 •

edited

zkochan commented Mar 25, 2022

zkochan commented Mar 26, 2022 •

edited

chengcyber commented Mar 26, 2022 •

edited

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022 •

edited

wbern commented Mar 27, 2022

chengcyber commented Mar 27, 2022

zkochan commented Mar 27, 2022

zkochan Mar 27, 2022

chengcyber Mar 29, 2022

zkochan Mar 28, 2022

zkochan Mar 28, 2022

chengcyber Mar 29, 2022 •

edited

zkochan Mar 28, 2022

zkochan commented Mar 29, 2022

chengcyber commented Mar 29, 2022

zkochan commented Mar 29, 2022

nikoladev commented Mar 30, 2022

zkochan commented Mar 30, 2022

nikoladev commented Mar 30, 2022

chengcyber commented Apr 1, 2022

zkochan commented Sep 15, 2023

chengcyber commented Sep 23, 2023

Crayon-ShinChan commented Dec 9, 2023 •

edited

feat: add git-branch-lockfile config to generate lockfile in each branch #4475

feat: add git-branch-lockfile config to generate lockfile in each branch #4475

Conversation

chengcyber commented Mar 24, 2022 • edited

Summary

Detail

How to test it

zkochan commented Mar 25, 2022

zkochan commented Mar 26, 2022 • edited

chengcyber commented Mar 26, 2022 • edited

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022

zkochan commented Mar 26, 2022

chengcyber commented Mar 26, 2022 • edited

wbern commented Mar 27, 2022

chengcyber commented Mar 27, 2022

zkochan commented Mar 27, 2022

zkochan Mar 27, 2022

Choose a reason for hiding this comment

chengcyber Mar 29, 2022

Choose a reason for hiding this comment

zkochan Mar 28, 2022

Choose a reason for hiding this comment

zkochan Mar 28, 2022

Choose a reason for hiding this comment

chengcyber Mar 29, 2022 • edited

Choose a reason for hiding this comment

zkochan Mar 28, 2022

Choose a reason for hiding this comment

zkochan commented Mar 29, 2022

chengcyber commented Mar 29, 2022

zkochan commented Mar 29, 2022

nikoladev commented Mar 30, 2022

zkochan commented Mar 30, 2022

nikoladev commented Mar 30, 2022

chengcyber commented Apr 1, 2022

zkochan commented Sep 15, 2023

chengcyber commented Sep 23, 2023

Crayon-ShinChan commented Dec 9, 2023 • edited

chengcyber commented Mar 24, 2022 •

edited

zkochan commented Mar 26, 2022 •

edited

chengcyber commented Mar 26, 2022 •

edited

chengcyber commented Mar 26, 2022 •

edited

chengcyber Mar 29, 2022 •

edited

Crayon-ShinChan commented Dec 9, 2023 •

edited