On Sat, Dec 12, 2020 at 05:32:13PM +0100, Stefan Sperling wrote:
> 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?

ok jrick

histedit -m and -f are behaving properly now too with the other patch
applied, so that is also 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) {