moduleFileHash in MODULE.bazel.lock causes frequent Git merge conflicts

[linzhp]

Description of the bug:

The value of moduleFileHash changes every time any dependency in the whole repo is changed. This is not scalable in a monorepo with thousands of developers committing concurrently. Imagine the scenario:

From base revision with "moduleFileHash": "x"

Developer 1 upgrades library a, which produced this as part of the diff:

-   "moduleFileHash": "x",
+   "moduleFileHash": "y",

Meanwhile, developer 2 upgrades another library b, which is totally unrelated to a, and produce this:

-   "moduleFileHash": "x",
+   "moduleFileHash": "z",

If developer 1 landed her diff first, developer 2 won't be able to land hers due to Git merge conflict. So developer 2 will have to rebase onto the main branch and update her diff to be

-   "moduleFileHash": "y",
+   "moduleFileHash": "aa",

While she went for a coffee break, developer 3 landed yet another diff that upgrade another unrelated library, causing moduleFileHash to change:

-   "moduleFileHash": "y",
+   "moduleFileHash": "bb",

Now developer 2 has to rebase again... In the end, developer 2 has to yell to all developers in this big corporation: STOP UPGRADING ANY DEPENDENCIES BEFORE I LAND MY CHANGE

Which category does this issue belong to?

External Dependency

[Wyverald]

This does sound like a very probable scenario, though I'm drawing a blank on how it should be resolved. We need the module file hash in the lockfile to know whether the locked stuff is up-to-date.

Maybe if we had segmented MODULE.bazel files (#17880) and recorded the hash of each segment individually? Do you reckon that's enough?

[fmeum]

Options I see:

1. Replace the hash with a canonicalized JSON description of the module file contents that are relevant for module resolution (roughly bazel_deps but not use_repos). While that does make conflicts more friendly as they aren't concentrated on a single line, it's still hard for humans to correctly resolve the conflict and not introduce subtle mistakes.

2. Provide tooling that can serve as a custom git merge driver and automatically resolves these conflicts. I don't know whether git allows the driver for a particular file to use contents from another file (MODULE.bazel) though, which would be required to do this fully automatically. If that's not possible, it would require a custom step (e.g. a CI action that triggers on rebases).

3. The most drastic one: Replace the module resolution part of the lockfile with a go.sum-style append-only lock mechanism. Simply record the hash of the module, source and patch files of each module involved during resolution and rely on the repository cache to provide offline access to these files given their hashes. Since module resolution is deterministic, this is sufficient to reconstruct the entire dependency tree including module extension usages. Resolving a conflict by keeping all the lines will always be safe and a command such as bazel mod tidy could be used to clean up unused hashes.

In fact, 3 is the only option I can think of that realizes "trust on first use" semantics for module files: With the current format, every merge conflict or change to the module file followed by a server restart will fetch module files again, relying on our promise of their immutability to ensure nobody has tampered with them.

[linzhp]

I like 3 best. We rarely run into merge conflicts in go.sum in Uber's Go monorepo because there is no check sum for the whole file and because it is sorted

[fmeum]

An example of the format I envision, borrowing heavily from go.sum:

{
  "lockfileVersion": 5,
  "moduleHashes": {
    "rules_bar@2.3.4/MODULE.bazel": "sha256-...",
    "rules_bar@2.3.4/source.json": "sha256-...",
    "rules_foo@1.2.3/MODULE.bazel": "sha256-...",
    "rules_foo@1.2.3/source.json": "sha256-...",
    "rules_foo@1.2.4/MODULE.bazel": "sha256-...",
    "rules_foo@1.2.4/source.json": "sha256-..."
  },
  "extensionResults": {
    "@@rules_foo~1.2.3//extensions:my_ext.bzl%my_ext": {
      "sha256-ABC..": {
        "fileDeps": [
          "@@//:foo.lock"
        ],
        "generatedRepoSpecs": {
          ...
        }
      },
      "sha256-CDE..": {
        "fileDeps": [],
        "generatedRepoSpecs": {
          ...
        }
      }
    }
  }
}

The hashes in the keys of the inner extensionResults map are opaque hashes of:

• the transitive .bzl files
• the statically declared os/arch requirements
• the statically declared environment variables
• the accumulated extension usages
• the accumulated file deps and their hashes

This format has a few new properties:

• Merge conflicts should be more rare since they can only occur if extensions or module dependencies that are next to each other in sorted order are affected by the change.
  Isolated extension usages can help make this even less likely.
• Merge conflicts can always be resolved by an appropriate deep JSON merge that is shallow on generatedRepoSpecs and fileDeps.
  If an extension produces different results with the same inputs on different branches due to non-hermeticity, choosing either of those should be fine.
• When a dependency is updated, the lockfile still provides useful information to speed up the resolution step (by using locally cached registry files) and to ensure that modules don't change after they are first encountered (trust on first use).
• We don't have to include hashes for bazel_tools or local overrides.
• This performs worse on CI runners with no repository cache. Alternatively, we could maintain a directory in the source tree that stores the module and source.json files.
• Due to the "append-only" storage for extension results, this can mitigate the problem of unstable extension hashes: Each of them will get its own separate entry.
• Due to the use of opaque hashes for extension results, error messages on out-of-date lockfiles with --lockfile_mode=error can be less helpful. We could fix this by adding more clear-text information to the results.
• Lockfiles can grow over time (similar to go.sum), which also slows down the linear search over the fileDeps, so we need some kind of clean up logic.
  This is straightforward for the module part, but less so for the extension results.

I haven't fully thought this through yet and may have missed crucial cons.