Simplify op heads interface #2749
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We move `.jj/repo/op_heads/*` into `.jj/repo/op_heads/heads/*` almost a year ago, in commits 90a66ec and 37ba175. We said we would drop support for it in 0.9+. I think we said that before we started doing monthly releases, but I we're still past the goal of 6 months (which is what I think we were aiming for).
Consider how one would implment the current `OpHeadsStore` interface for a cloud-based backend. After `OpHeadsStore::add_op_head()` is called, the set of op heads temporarily contains two heads (typically) until `OpHeadsStore::remove_op_head()` is called. That's not invalid, but it's annoying to have to deal with that state more than necessary. Also, it's unnecessarily inefficient to send the addition and removal of op heads as separate RPCs. This patch therefore adds a `update_op_heads()` method that takes a list of old heads to remove and a single new head to add. Coming patches will start migrating to that method.
`OpHeadsStoreLock::promote_new_op()` doesn't add much over the new `update_op_heads()`, so let's switch to the latter.
I think the idea behind `handle_ancestor_ops()` was to let our backend at Google delegate the work to the server, which could then avoid walking ancestors. However, we're now thinking that we're going to make our server resolve divergent operations on its own instead, so the client will never see more than one op head, unless it manually creates the second op head itself (e.g. because the user ran two concurrent commands). In those cases it should be fine to do the walk. So let's simplify the trait by removing the function.
This gets us closer to being able to use the new `update_op_heads()` function here (without calling it multiple times).
martinvonz
force-pushed
the
push-osxwsnloqwtr
branch
from
147326d to
e852db2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
If applicable:
CHANGELOG.md