Proposal: lockfile git custom merge driver

[simonbuchan]

• I'd be willing to implement this feature (contributing guide)

Describe the user story

At the moment, doing a git merge you are quite likely to get a merge conflict in the lockfile,
and it's a bit too easy to end up with a broken lockfile by attempting to manually merge,
instead of just running pnpm (before the merge is committed!).

Describe the solution you'd like

It would be nice if pnpm could be configured as a git custom merge driver, so that no user
interaction is required:

# .gitattributes
pnpm-lock.yaml -diff merge=pnpm-lockfile

# .git/config
[merge "pnpm-lockfile"]
  driver = pnpm install --lockfile-only

Unfortunately, this doesn't quite work, as merge drivers must take three temporary files as input.

Having a simple, out of the box solution that doesn't require adding anything additional (in particular,
before a pnpm install might happen to fetch a package for this) would be really nice.

Specifically, something like: pnpm merge-lockfile %A %O %B

Describe the drawbacks of your solution

As with anything requiring git config changes, you would still require a one-time step to enable it.

This is a pretty common step and has many patterns, e.g. husky
suggests adding a prepare script to handle it. I don't feel this is necessarily pnpm's problem, but I
suppose pnpm merge-lockfile --install would make some sense.

Describe alternatives you've considered

This could be done as an npm package, but keeping it up to date with pnpm would make the
implementation much more complex than it would be for pnpm, which nearly has this functionality
available (from an outsider's perspective)

[simonbuchan]

BTW I had a try at using a local script using @pnpm/merge-lockfile-changes to validate the idea:

import cp from "node:child_process";
import fs from "node:fs";

import yaml from "js-yaml";
import merge from "@pnpm/merge-lockfile-changes";

if (process.argv.includes("--install", 2)) {
  // [merge "pnpm-lockfile"]
  //     driver = node scripts/lockfile-merge.mjs %A %O %B
  cp.execSync(`git config "merge.pnpm-lockfile.driver" "node scripts/lockfile-merge.mjs %A %O %B"`, { stdio: "inherit" });
  process.exit(0);
}

const [mergedFilename, oursFilename, theirsFilename] = process.argv.slice(2);
const ours = yaml.load(fs.readFileSync(oursFilename, "utf-8"));
const theirs = yaml.load(fs.readFileSync(theirsFilename, "utf-8"));
const merged = merge.default(ours, theirs);
fs.writeFileSync(mergedFilename, yaml.dump(merged));

Based on the logic in @pnpm/lockfile-file's read functions, but it complains about workspace links, while the pnpm cli works... am I missing something?

TypeError: Invalid Version: link:packages/<first-internal-package-name>
    at new SemVer (<repo>\node_modules\.pnpm\semver@7.3.7\node_modules\semver\classes\semver.js:38:13)
    at compare (<repo>\node_modules\.pnpm\semver@7.3.7\node_modules\semver\functions\compare.js:3:3)
    at Object.gt (<repo>\node_modules\.pnpm←\semver@7.3.7\node_modules\semver\functions\gt.js:2:29)
    at mergeVersions (<repo>\node_modules\.pnpm\@pnpm+merge-lockfile-changes@3.0.3\node_modules\@pnpm\merge-lockfile-changes\lib\index.js:69:26)
    at mergeDict (<repo>\node_modules\.pnpm\@pnpm+merge-lockfile-changes@3.0.3\node_modules\@pnpm\merge-lockfile-changes\lib\index.js:50:30)
    at Object.mergeLockfileChanges [as default] (<repo>\node_modules\.pnpm\@pnpm+merge-lockfile-changes@3.0.3\node_modules\@pnpm\merge-lockfile-changes\lib\index.js:36:24)
    at file:///<repo>/scripts/lockfile-merge.mjs:19:29

[zkochan]

There was an attempt already: https://github.com/pnpm/merge-driver

But there are some issues. One of the issues is that conflicts may happen in package.json files as well and the order in which git merges the files is random. We can't really resolve the conflicts in the lockfile if the package.json file was not yet merged.

The second issue is that most conflicts happen on PRs, where you can't have a custom merge driver. Hence, this feature becomes a lot less useful.

[simonbuchan]

So is the current @pnpm/merge-lockfile-changes effectively not used then? That pays no attention to any other file.

[zkochan]

It is not used. We are thinking about 2 alternative solutions currently. One is to use different lockfiles for branches to avoid merge conflicts: #4475

The other idea is to change the lockfile format to reduce the number of conflicts: pnpm/rfcs#1

cc @gluxon @chengcyber

[simonbuchan]

Changing the formatting sounds a bit suspicious, I often use a (built into webstorm) "magic" yaml merge on lock file conflicts and it often results in duplicate sections. At least according to the output warning, this results in pnpm just binning the whole file, which I really want to avoid for all the usual reasons to have a lockfile. Are you saying a single file pnpm merge can not do better, due to needing package.jsons?

I did see the branch lockfile approach, but I don't get how that works if you want a PR merge to run CI on the main branch using the updated lockfile - seems like it would need CI to check if there's a branch lockfile and if so commit a lockfile merge: seems like a nightmare to get set up?

At least in my experience, conflicts in a lockfile require other files to be resolved too, so breaking PR auto merges is fine - I just want the manual branch merge to not result in broken lockfiles.

Here's a dumb idea; the merge driver just leaves the conflict markers and a precommit hook does the actual merge?

[zkochan]

pnpm install already fixes lockfiles with merge conflicts automatically, so you can just run pnpm install in the precommit hook.

[simonbuchan]

Sure, that's roughly what I was thinking, but having a random potentially 30s+ process happening on random commits during local work (especially during rebases!) is no fun.

I did give --lockfile-only a try, but if didn't do something wrong, that seems to do the full resolution and verification, simply not updating node_modules? I'm really hoping for a <1s process.

[zkochan]

Maybe we can implement something like pnpm install --fix-lockfile-only. So it would only run installation if the lockfile has merge conflicts.

[zkochan]

Actually, maybe it is how --lockfile-only should work by default. Not sure why it does full resolution for you. Maybe your lockfile was not up-to-date with the package.json file.

[simonbuchan]

Maybe I'm missing something, what does "installation" involve here? Just fetching missing package metadata? Logically least one side of the merge should have the missing data to reconstruct the merged file, but I get that would be a pain to maintain.

The install would have been with updated package.json's that also got magic merged (different packages getting updated). They in theory would have the content that the merged lockfile would be for.

[zkochan]

Probably you are right. I don't remember how the lockfile fixer works. I'll have to check.

[gluxon]

Thanks for the ping @zkochan. This thread is interesting and I do think there's room for some sort of pnpm install --fix-lockfile-only that doesn't do full resolution.

At the moment I think pnpm install is fast enough, but could see this new mode being valuable for others with larger repos.

[simonbuchan]

@gluxon keep in mind precommit hooks fire on every commit during a rebase (at least, from memory!) - avoiding hitting the network or messing with a bunch of FS (especially on Windows where it's slow) would be really nice. I'll give pnpm install --offline --lockfile-only a run and see how it goes though.

[zkochan]

I can update pnpm install --lockfile-only to finish in 1 second when the lockfile is good.

[ghiscoding]

@zkochan
Your last change to exist early on --lockfile-only with PR #4932 seems to be problematic and is exiting well hmm... too early ... at least with Lerna-Lite (a fork of Lerna with extra features to work with pnpm and workspace: protocol). I believe you know what Lerna is since I saw you commenting in the past in Lerna's project, but basically in Lerna-Lite I added a flag --sync-workspace-lock to update the lock file after executing a version bump in a Lerna-Lite project, it simply runs pnpm install --lockfile-only after updating all package.json versions and it was working fine with unit tests when I introduced it a month ago, but it now fail since today and it seems to be related to your latest version 7.4.0 release which includes that exist early change. I'm not sure if it's because my unit tests run in CI (it seems to work locally) but I'm still afraid that your last change might affect my new feature and exist early without updating the pnpm lock file in the monorepo. Are there any ways to force the execution the way it was previously implemented? Maybe adding --fix-lockfile will work but it seems odd to have to use it just to make the --lockfile-only really do the lock file update.

EDIT
calling pnpm install --lockfile-only --fix-lockfile does fix my unit tests (in this Lerna-Lite PR 233) but it seems odd to use and not expected by the user, I wish that you could possibly rethink or revert that exit early code, it seems problematic for project like Lerna-Lite

Basically I used the fix lock file just because that seems to be the last flag I can use that is still in the if condition you modified in PR #4932

pnpm/packages/core/src/install/index.ts

Lines 266 to 271 in 487527d

	if (
	!ctx.lockfileHadConflicts &&
	!opts.update &&
	!opts.fixLockfile &&
	installsOnly &&
	(

EDIT 2
I can now confirmed that it broke Lerna-Lite process that I mentioned in first paragraph, the pnpm i --lockfile-only no longer work in CI when executing a new version with Lerna-Lite, the pnpm-lock.yaml doesn't get detected as a git change and no longer updates most probably because it exited too early, it was working fine with previous pnpm (<=7.3.0).

Opened a new issue #4951

[mcmxcdev]

Just wanted to point out that there is now a new experimental lockfile format setting which was introduced in: https://github.com/pnpm/pnpm/releases/tag/v7.7.0

It's supposed to help with avoiding merge conflicts due to lockfile changes.