From: Stefan Sperling Subject: Re: histedit: How to fold M, D? To: Christian Weisgerber Cc: gameoftrees@openbsd.org Date: Tue, 25 Jul 2023 15:24:16 +0200 On Tue, Jul 25, 2023 at 02:35:45PM +0200, Christian Weisgerber wrote: > Stefan Sperling: > > > The "different content" check for deletions is supposed to be a > > safe-guard in cases where comnands such as rebase, merge, cherrypick, > > and backout delete files. If file content is not 100% the same then > > we cannot assume that the file being deleted is the "same". > > Absolutely. If I have a branch that deletes files and I rebase it, > I want a conflict if the contents of those files are different. > > In fact that is my situation. The files have changed, and I add a > commit that reverts those changes, and then rebase the branch on > top. Which works fine. But afterwards, I would like to fold the > revert commit into the rebased branch. > > > However, your histedit situation is a different use case. > > Yes. How doas the "different content" check even apply to a folding > operation?? It is because histedit uses the same code as rebase to do merging. Folding is implemented as a series of cherrypick merges without creating commits in-between. Below is a relatively simple patch you could try. Consider the deletion-rule we have in place for locally added files: If the file's content can be found in the repository then we can delete it. Applying this logic to files in modified status should make your histedit folding case "just work" since the intermediate merged content should by definition appear in the repository, unless you have make other edits. Because this approach affects all merges, not just histedit, it might lead to other surprises in other edge cases. But any files deleted here should always remain recoverable, provided the corresponding blob isn't garbaged-collected during gotadmin cleanup. So I would consider this approach safe for now. Surprisingly, the test suite keeps passing with this (ignoring the unrelated tog/log.sh test failure which regres builders have been telling this mailing list about already). An alternative to this patch would be to special-case the folding case by passing down a flag which tweaks the behaviour of this code path just for folding. But I don't have enough spare time to write such a diff now. diff /home/stsp/src/got commit - 48bfa43b34c38fcf5e33c1e63dbe5dbd8c5865ce path + /home/stsp/src/got blob - c2e52f2b6af12e7f6d173ad130885b820fa1bd5e file + lib/worktree.c --- lib/worktree.c +++ lib/worktree.c @@ -3124,12 +3124,13 @@ merge_file_cb(void *arg, struct got_blob_object *blob1 if (ie) got_fileindex_entry_mark_deleted_from_disk(ie); break; + case GOT_STATUS_MODIFY: case GOT_STATUS_ADD: { struct got_object_id *id; FILE *blob1_f; off_t blob1_size; /* - * Delete the added file only if its content already + * Delete the file only if its content already * exists in the repository. */ err = got_object_blob_file_create(&id, &blob1_f, @@ -3159,7 +3160,6 @@ merge_file_cb(void *arg, struct got_blob_object *blob1 goto done; break; } - case GOT_STATUS_MODIFY: case GOT_STATUS_CONFLICT: err = (*a->progress_cb)(a->progress_arg, GOT_STATUS_CANNOT_DELETE, path1);