workspace: recover from missing operation #2905
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
jonathantanmy
force-pushed
the
jt/missingop
branch
3 times, most recently
from
2098893 to
af8b6d1
Compare
yuja
reviewed
Feb 1, 2024
jonathantanmy
force-pushed
the
jt/missingop
branch
2 times, most recently
from
1e405be to
8344a76
Compare
|
After some internal investigation at work, I've updated the code to also be more resilient when the current tree of the working copy is also missing (so, not only the operation). I've addressed all the comments (hopefully) except for the ones that involve some refactoring (moving |
jonathantanmy
force-pushed
the
jt/missingop
branch
from
8344a76 to
347817c
Compare
yuja
reviewed
Feb 3, 2024
martinvonz
reviewed
Feb 3, 2024
jonathantanmy
force-pushed
the
jt/missingop
branch
3 times, most recently
from
d08d1b8 to
9ebcb89
Compare
Move this function from cli_util.rs, since workspace.rs is the only caller. This function will be enlarged in a subsequent commit.
A subsequent commit will need to handle the return value of check_stale_working_copy() in 3 different ways, so a boolean will soon not be sufficient. In preparation for that, inline is_stale() into its caller, converting it into a "match".
jonathantanmy
force-pushed
the
jt/missingop
branch
2 times, most recently
from
537e1f1 to
6b4cec1
Compare
|
LGTM, but I'll let Yuya give the final approval. Thanks, both of you! |
If the operation corresponding to a workspace is missing for some reason (the specific situation in the test in this commit is that an operation was abandoned and garbage-collected from another workspace), currently, jj fails with a 255 error code. Teach jj a way to recover from this situation. When jj detects such a situation, it prints a message and stops operation, similar to when a workspace is stale. The message tells the user what command to run. When that command is run, jj loads the repo at the @ operation (instead of the operation of the workspace), creates a new commit on the @ commit with an empty tree, and then proceeds as usual - in particular, including the auto-snapshotting of the working tree, which creates another commit that obsoletes the newly created commit. There are several design points I considered. 1) Whether the recovery should be automatic, or (as in this commit) manual in that the user should be prompted to run a command. The user might prefer to recover in another way (e.g. by simply deleting the workspace) and this situation is (hopefully) rare enough that I think it's better to prompt the user. 2) Which command the user should be prompted to run (and thus, which command should be taught to perform the recovery). I chose "workspace update-stale" because the circumstances are very similar to it: it's symptom is that the regular jj operation is blocked somewhere at the beginning, and "workspace update-stale" already does some special work before the blockage (this commit adds more of such special work). But it might be better for something more explicitly named, or even a sequence of commands (e.g. "create a new operation that becomes @ that no workspace points to", "low-level command that makes a workspace point to the operation @") but I can see how this can be unnecessarily confusing for the user. 3) How we recover. I can think of several ways: a) Always create a commit, and allow the automatic snapshotting to create another commit that obsoletes this commit. b) Create a commit but somehow teach the automatic snapshotting to replace the created commit in-place (so it has no predecessor, as viewed in "obslog"). c) Do either a) or b), with the added improvement that if there is no diff between the newly created commit and the former @, to behave as if no new commit was created (@ remains as the former @). I chose a) since it was the simplest and most easily reasoned about, which I think is the best way to go when recovering from a rare situation.
jonathantanmy
force-pushed
the
jt/missingop
branch
from
6b4cec1 to
b5f8298
Compare
|
Thanks everyone. I'll go ahead and merge this PR. |
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.
In the commit message, I've added a discussion of alternate ways of how we can handle this situation, with my rationale for how I ended up doing it in the commit. If there are other (and maybe better) ways of handling this, feel free to let us know.
Checklist
If applicable:
CHANGELOG.md