From: Stefan Sperling Subject: Re: histedit: How to fold M, D? To: Christian Weisgerber Cc: gameoftrees@openbsd.org Date: Wed, 26 Jul 2023 18:22:18 +0200 On Wed, Jul 26, 2023 at 05:23:04PM +0200, Christian Weisgerber wrote: > Stefan Sperling: > > > > Oh sorry this diff won't work as-is since it assumes that the > > > blob ID passed in already matches the file on disk. I will have > > > to revisit this, adding an object ID lookup. > > > > Try this one please. Also fixes small leaks in unlikely error paths. > > That doesn't quite work: > > 81474c680f54 -> fold commit: revert: Implement PAC support. > ... > 4b1696fbab55 -> 3790f01fbf94: llvm: revert to 13.0.0 > ... > 0d86ca54faea -> 31c8452f490b: llvm: update to 15.0.7 > ... > got: duplicate entry in tree object > > > Also, aborting the operation: > > $ got he -a > ... > got: gnu/llvm/lldb/source/Plugins/Process/OpenBSD/CMakeLists.txt: duplicate file index entry > > > Hmpf. FWIW, "got up" recovers from this. Sorry about that. I missed that removing modified files and locally added files require different operations on the file index. Here is a fixed version, with a test to confirm that it is now working as intended. ----------------------------------------------- allow modified files to be deleted during merges if content is found the repo Problem reported by naddy@ diff 7c67cf56d3e4bab38eebe16a51f0ffb790694738 5f5af15cd633829e1362bbf7843315248d3c41c0 commit - 7c67cf56d3e4bab38eebe16a51f0ffb790694738 commit + 5f5af15cd633829e1362bbf7843315248d3c41c0 blob - c2e52f2b6af12e7f6d173ad130885b820fa1bd5e blob + 2cdafab5a89da7e31a55bc887af42b14226637d8 --- lib/worktree.c +++ lib/worktree.c @@ -2999,6 +2999,7 @@ merge_file_cb(void *arg, struct got_blob_object *blob1 int local_changes_subsumed; FILE *f_orig = NULL, *f_deriv = NULL, *f_deriv2 = NULL; char *id_str = NULL, *label_deriv2 = NULL; + struct got_object_id *id = NULL; if (blob1 && blob2) { ie = got_fileindex_entry_get(a->fileindex, path2, @@ -3124,18 +3125,63 @@ 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: { + FILE *blob1_f; + off_t blob1_size; + int obj_type; + /* + * Delete the file only if its content already + * exists in the repository. + */ + err = got_object_blob_file_create(&id, &blob1_f, + &blob1_size, path1); + if (err) + goto done; + if (fclose(blob1_f) == EOF) { + err = got_error_from_errno("fclose"); + goto done; + } + + /* Implied existence check. */ + err = got_object_get_type(&obj_type, repo, id); + if (err) { + if (err->code != GOT_ERR_NO_OBJ) + goto done; + err = (*a->progress_cb)(a->progress_arg, + GOT_STATUS_CANNOT_DELETE, path1); + goto done; + } else if (obj_type != GOT_OBJ_TYPE_BLOB) { + err = (*a->progress_cb)(a->progress_arg, + GOT_STATUS_CANNOT_DELETE, path1); + goto done; + } + err = (*a->progress_cb)(a->progress_arg, + GOT_STATUS_DELETE, path1); + if (err) + goto done; + err = remove_ondisk_file(a->worktree->root_path, + path1); + if (err) + goto done; + if (ie) + got_fileindex_entry_mark_deleted_from_disk(ie); + break; + } 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, &blob1_size, path1); if (err) goto done; + if (fclose(blob1_f) == EOF) { + err = got_error_from_errno("fclose"); + goto done; + } if (got_object_id_cmp(id, id1) == 0) { err = (*a->progress_cb)(a->progress_arg, GOT_STATUS_DELETE, path1); @@ -3152,14 +3198,10 @@ merge_file_cb(void *arg, struct got_blob_object *blob1 err = (*a->progress_cb)(a->progress_arg, GOT_STATUS_CANNOT_DELETE, path1); } - if (fclose(blob1_f) == EOF && err == NULL) - err = got_error_from_errno("fclose"); - free(id); if (err) goto done; break; } - case GOT_STATUS_MODIFY: case GOT_STATUS_CONFLICT: err = (*a->progress_cb)(a->progress_arg, GOT_STATUS_CANNOT_DELETE, path1); @@ -3243,6 +3285,7 @@ done: if (f_deriv2 && fclose(f_deriv2) == EOF && err == NULL) err = got_error_from_errno("fclose"); free(id_str); + free(id); free(label_deriv2); free(ondisk_path); return err; blob - 2850c77ea9f07c4d604211acb189455b07692252 blob + 5ef42917a9efb422c5a96479122b03533f9019bc --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -1542,6 +1542,88 @@ test_histedit_fold_delete_add() { test_done "$testroot" "$ret" } +# if a previous commit edits a file, and it is folded into a commit +# that deletes the same file, the file will be deleted by histedit +test_histedit_fold_edit_delete() { + local testroot=`test_init histedit_fold_edit_delete` + + local orig_commit=`git_show_head $testroot/repo` + + echo "modify alpha" > $testroot/repo/alpha + (cd $testroot/repo && git add alpha) + git_commit $testroot/repo -m "modified alpha" + local old_commit1=`git_show_head $testroot/repo` + + git_rm $testroot/repo alpha + git_commit $testroot/repo -m "deleted alpha" + local old_commit2=`git_show_head $testroot/repo` + + got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" "$ret" + return 1 + fi + + echo "fold $old_commit1" > $testroot/histedit-script + echo "pick $old_commit2" >> $testroot/histedit-script + echo "mesg folded changes" >> $testroot/histedit-script + + (cd $testroot/wt && got histedit -F $testroot/histedit-script \ + > $testroot/stdout) + + local new_commit1=`git_show_head $testroot/repo` + + local short_old_commit1=`trim_obj_id 28 $old_commit1` + local short_old_commit2=`trim_obj_id 28 $old_commit2` + local short_new_commit1=`trim_obj_id 28 $new_commit1` + + echo "G alpha" >> $testroot/stdout.expected + echo "$short_old_commit1 -> fold commit: modified alpha" \ + >> $testroot/stdout.expected + echo "D alpha" >> $testroot/stdout.expected + echo "$short_old_commit2 -> $short_new_commit1: folded changes" \ + >> $testroot/stdout.expected + echo "Switching work tree to refs/heads/master" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + if [ -e $testroot/wt/alpha ]; then + echo "removed file alpha still exists on disk" >&2 + test_done "$testroot" "1" + return 1 + fi + + (cd $testroot/wt && got status > $testroot/stdout) + + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got log -l2 | grep ^commit > $testroot/stdout) + echo "commit $new_commit1 (master)" > $testroot/stdout.expected + echo "commit $orig_commit" >> $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + + test_done "$testroot" "$ret" +} + test_histedit_fold_delete_add() { local testroot=`test_init histedit_fold_delete_add` @@ -2531,6 +2613,7 @@ run_test test_histedit_fold_delete_add run_test test_histedit_split_commit run_test test_histedit_duplicate_commit_in_script run_test test_histedit_fold_add_delete +run_test test_histedit_fold_edit_delete run_test test_histedit_fold_delete_add run_test test_histedit_fold_only run_test test_histedit_fold_only_empty_logmsg