On Sat, Dec 12, 2020 at 04:01:56PM +0100, Stefan Sperling wrote:
> For now, this removes any attempts to clean up after histedit runs into an
> error. The user will now see why the histedit operation has failed, and 
> may decide whether to fix things up and continue or abort the operation.
> 
> Our tests seem happy with this.

Testing by jrick uncovered a related issue: While preparing a histedit
operation, we don't create a fully complete histedit tracking state.
The histedit "commit reference", which points at the commit currently
being edited, is not created during preparation. It is currently created
only once the histedit operation creates a new commit.

This leaves users unable to abort/continue the histedit operation after
saving an empty log message because the code which attempts to resume
the histedit operation complains about the missing reference.

This patch includes a fix for that problem as well. I plan to split
these changes into two commits.

ok?

diff 448d2d7f0de229dc970fbb6bb61a32615877ef08 /home/stsp/src/got
blob - c8dbc08063b2590cb230bbb89b7864078f5b1a4d
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -8587,6 +8587,7 @@ cmd_histedit(int argc, char *argv[])
 	const char *edit_script_path = NULL;
 	unsigned char rebase_status = GOT_STATUS_NO_CHANGE;
 	struct got_object_id_queue commits;
+	struct got_object_qid *qid;
 	struct got_pathlist_head merged_paths;
 	const struct got_object_id_queue *parent_ids;
 	struct got_object_qid *pid;
@@ -8812,21 +8813,17 @@ cmd_histedit(int argc, char *argv[])
 			goto done;
 		}
 
+		qid = SIMPLEQ_FIRST(&commits);
 		error = got_worktree_histedit_prepare(&tmp_branch, &branch,
-		    &base_commit_id, &fileindex, worktree, repo);
+		    &base_commit_id, &fileindex, qid->id, worktree, repo);
 		if (error)
 			goto done;
 
 		if (edit_script_path) {
 			error = histedit_load_list(&histedit_cmds,
 			    edit_script_path, repo);
-			if (error) {
-				got_worktree_histedit_abort(worktree, fileindex,
-				    repo, branch, base_commit_id,
-				    update_progress, &upa);
-				print_update_progress_stats(&upa);
+			if (error)
 				goto done;
-			}
 		} else {
 			const char *branch_name;
 			branch_name = got_ref_get_symref_target(branch);
@@ -8834,25 +8831,14 @@ cmd_histedit(int argc, char *argv[])
 				branch_name += 11;
 			error = histedit_edit_script(&histedit_cmds, &commits,
 			    branch_name, edit_logmsg_only, fold_only, repo);
-			if (error) {
-				got_worktree_histedit_abort(worktree, fileindex,
-				    repo, branch, base_commit_id,
-				    update_progress, &upa);
-				print_update_progress_stats(&upa);
+			if (error)
 				goto done;
-			}
 
 		}
 
-		error = histedit_save_list(&histedit_cmds, worktree,
-		    repo);
-		if (error) {
-			got_worktree_histedit_abort(worktree, fileindex,
-			    repo, branch, base_commit_id,
-			    update_progress, &upa);
-			print_update_progress_stats(&upa);
+		error = histedit_save_list(&histedit_cmds, worktree, repo);
+		if (error)
 			goto done;
-		}
 
 	}
 
blob - 24fddd52e81d96ff7b1800a61de1f80319326a3e
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -337,7 +337,7 @@ const struct got_error *got_worktree_rebase_abort(stru
  */
 const struct got_error *got_worktree_histedit_prepare(struct got_reference **,
     struct got_reference **, struct got_object_id **, struct got_fileindex **,
-    struct got_worktree *, struct got_repository *);
+    struct got_object_id *, struct got_worktree *, struct got_repository *);
 
 /*
  * Continue an interrupted histedit operation.
blob - 91a87ba60ac2ebc61140d6778df16fc1e2f39016
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -6554,13 +6554,14 @@ done:
 const struct got_error *
 got_worktree_histedit_prepare(struct got_reference **tmp_branch,
     struct got_reference **branch_ref, struct got_object_id **base_commit_id,
-    struct got_fileindex **fileindex, struct got_worktree *worktree,
-    struct got_repository *repo)
+    struct got_fileindex **fileindex, struct got_object_id *commit_id,
+    struct got_worktree *worktree, struct got_repository *repo)
 {
 	const struct got_error *err = NULL;
 	char *tmp_branch_name = NULL;
 	char *branch_ref_name = NULL;
 	char *base_commit_ref_name = NULL;
+	char *commit_ref_name = NULL;
 	char *fileindex_path = NULL;
 	struct check_rebase_ok_arg ok_arg;
 	struct got_reference *wt_branch = NULL;
@@ -6625,6 +6626,14 @@ got_worktree_histedit_prepare(struct got_reference **t
 		goto done;
 	}
 
+	err = get_histedit_commit_ref_name(&commit_ref_name, worktree);
+	if (err)
+		return err;
+
+	err = store_commit_id(commit_ref_name, commit_id, 0, repo);
+	if (err)
+		goto done;
+
 	err = got_ref_alloc(tmp_branch, tmp_branch_name,
 	    worktree->base_commit_id);
 	if (err)
@@ -6641,6 +6650,7 @@ done:
 	free(tmp_branch_name);
 	free(branch_ref_name);
 	free(base_commit_ref_name);
+	free(commit_ref_name);
 	if (wt_branch)
 		got_ref_close(wt_branch);
 	if (err) {