workspace: make recovery commit empty instead of deleting everything #3149
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
The recovery commit we create when we run into a stale working copy with a missing operation currently has an empty tree. Our commit backend at Google creates an index of which files changed in each commit. That gets really expensive when a commit deletes all files in the repo, as these recovery commits do. So for our backend, it is much better to make the recovery commit empty instead. That's what this patch does. It almost doesn't matter functionally what tree we use for it since we don't care much about the current tree when snapshotting the working copy. It does matter in a few cases, however. One case is for conflicts. In that case, it's likely better to use the recovery commit's parent as base tree (as we do by making the recovery commit empty) than to use an empty tree, as that would guarantee that all conflicts would be considered resolved. (Side note: perhaps we should start looking at the current commit's parent instead of looking at the current commit when snapshotting, but that's a topic for another day.)
yuja
approved these changes
Feb 27, 2024
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.
The recovery commit we create when we run into a stale working copy with a missing operation currently has an empty tree. Our commit backend at Google creates an index of which files changed in each commit. That gets really expensive when a commit deletes all files in the repo, as these recovery commits do. So for our backend, it is much better to make the recovery commit empty instead. That's what this patch does.
It almost doesn't matter functionally what tree we use for it since we don't care much about the current tree when snapshotting the working copy. It does matter in a few cases, however. One case is for conflicts. In that case, it's likely better to use the recovery commit's parent as base tree (as we do by making the recovery commit empty) than to use an empty tree, as that would guarantee that all conflicts would be considered resolved. (Side note: perhaps we should start looking at the current commit's parent instead of looking at the current commit when snapshotting, but that's a topic for another day.)
Checklist
If applicable:
CHANGELOG.md