"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
got backout/cherrypick metadata (for commit log messages)
To:
Game of Trees <gameoftrees@openbsd.org>
Date:
Thu, 26 Jan 2023 18:04:43 +1100

Download raw body.

Thread
PSA: Long email and diff incoming! I apologise in advance for the wall
of text :)

The below diff implements stsp's brilliant idea using the algorithm he
laid out in the earlier thread (got backout/cherrypick auto commit).
I basically went with his suggestions across the board as far as the
algorithm and special case handling is concerned.

As a quick recap, when the user runs 'got {backout,cherrypick} commit',
we create a "logmsg" reference pointing to the tree's base commit. What
it points at is largely irrelevant for now, but the reference path takes
the form:

  refs/got/worktree/cherrypick-${WORKTREE_UUID}-$cherry-picked-commit-id}
  refs/got/worktree/backout-${WORKTREE_UUID}-${backed-out-commit-id}

As such, when the user next runs 'got commit', we parse the log
message(s) from the ${backed-out,cherry-picked}-commit-id refs with
which we prepopulate the editor. This way, if the user invokes N 'got
{bo,cy}' commands, all N log messages will appear in the editor. This is
the primary reason I think this idea of stsp's is better than the first.
The other being what op observed: no change is required from the user;
just cherrypick and commit!

We implement two different mechanisms in an effort to keep the editor
populated with relevant log messages only:

First, on 'got revert', we loop through each logmsg ref and if every
path modified in the referenced bo/cy commit no longer has changes in
the work tree, we remove this reference.

This prevents adding irrelevant log messages in the case where a user
does something like: 'got cy commit1' which modifies file1, then 'got rv
file1' followed by 'got cy commit2' which also modifies file1.  Without
the 'got revert' bookkeeping, the editor will be populated with the log
messages of both commit1 and commit2 even though it is now only the log
message of commit2 that is related to the changes being committed in
file1.

Second, when 'got commit' is run, we loop through each logmsg ref and if
at least one changed path in the referenced bo/cy commit has changes in
the work tree that are now being committed, we append its log message to
the temp file opened in the editor.

Upon a successful commit, we remove all references created by previous
backout/cherrypick operations in the current work tree. If a commit is
aborted or otherwise fails, the refs are kept on disk.

This is not foolproof, there are cases where we will inevitably have
messages that aren't quite relevant, but as stsp remarked, it's not
a big deal and the benefit outweights the cost of having to tidy things
up in the editor on the rare occasion this happens.

While no new flags are needed to prepopulate the backed-out and
cherrypicked commit log messages, we need something to list, and as stsp
noted remove, stale refs. For both cases, 'got {bo,cy}' has -l and -X
just like histedit and rebase albeit with two differences: first, if
invoked in a work tree, only those refs created in the current worktree
are displayed/deleted. If not invoked in a work tree, all references
created by previous bo/cy operations are listed/deleted; second, -l also
displays the list of changed paths. The reason for the first difference
is two-fold: "logmsg" refs really only make sense in the context of
a worktree as their purpose is to facilitate commits.  Users may have
multiple worktrees open with the same and/or different backed-out and
cherrypicked commits in several of them. If 'got cy -l' displayed all
refs, it could be confusing and wouldn't help users make decisions
specific to the current worktree. Second, if the same commits are
backed-out/cherrypicked in different work trees, 'got {bo,cy} -X
$commit' will delete the references for $commit created in all the
different work trees, which may not be what the user wanted. I thought
providing this distinction would help in this respect. I considered two
flags: -LX and -lx for all refs in the repository and worktree,
respectively. But decided against it. Lastly, displaying the paths
changed in each commit is so users can more readily map
cherrypicked/backed-out commits to local changes in the work tree.

Sorry again for the long email! I wanted to provide a brief explanation
of the changes, and I'm not entirely sure of the -lX interface; for
example, maybe 'got {bo,cy} -lX' should behave exactly the same as
histedit/rebase for the sake of consistency, or perhaps another flag to
list/delete all references irrespective of the work tree in which they
were created should be considered? Admittedly, I think the chosen
approach is the way to go, but I'm definitely open to alternatives.
I also felt it was important to explain what we do to keep irrelevant
and potentially confusing log messages from appearing in the editor as
this is a primary concern of this approach over -a.

I've not written any tests yet; assuming this is the preferred approach
and is ok, I'll follow-up with regress. Thanks!

diffstat /home/mark/src/got
 M  got/got.1               |   88+   4-
 M  got/got.c               |  689+  22-
 M  include/got_worktree.h  |   15+   0-
 M  lib/worktree.c          |    7+   0-

4 files changed, 799 insertions(+), 26 deletions(-)

diff /home/mark/src/got
commit - 87332a0e6e71182f1fb148d7f42e78105713285b
path + /home/mark/src/got
blob - ccdc977d9180a5385e27d7658840d68175702374
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -1970,7 +1970,11 @@ The maximum is 3.
 The maximum is 3.
 .El
 .Tg cy
-.It Cm cherrypick Ar commit
+.It Xo
+.Cm cherrypick
+.Op Fl lX
+.Ar commit
+.Xc
 .Dl Pq alias: Cm cy
 Merge changes from a single
 .Ar commit
@@ -2001,7 +2005,8 @@ committed with
 .Cm got cherrypick
 commands,
 committed with
-.Cm got commit ,
+.Cm got commit
+where the log message of the cherrypicked commit will appear in the editor,
 or discarded again with
 .Cm got revert .
 .Pp
@@ -2012,8 +2017,49 @@ conflicts must be resolved first.
 .Cm got update .
 If any relevant files already contain merge conflicts, these
 conflicts must be resolved first.
+.Pp
+The options for
+.Nm
+.Cm cherrypick
+are as follows:
+.Bl -tag -width Ds
+.It Fl l
+Display a list of references created by previous cherrypick operations
+If a
+.Ar commit
+is specified, only show the reference identified by the specified commit.
+If invoked in a work tree, only references created in the current work tree
+will be displayed.
+Otherwise, all references will be displayed irrespective of the work tree
+in which they were created.
+This option cannot be used with
+.Cm X .
+.It Fl X
+Delete references created by previous cherrypick operations, represented by
+references in the
+.Dq refs/got/worktree/cherrypick
+reference namespace.
+.Pp
+If a
+.Ar commit
+is specified, only delete the reference identified by this commit.
+Otherwise, delete all references matching the
+.Dq refs/got/worktree/cherrypick
+namespace.
+If invoked in a work tree, matching references created in the current
+work tree will be deleted.
+Otherwise, all matching references will be deleted irrespective of the
+work tree in which they were created.
+This option cannot be used with
+.Cm l .
+.El
+.Pp
 .Tg bo
-.It Cm backout Ar commit
+.It Xo
+.Cm backout
+.Op Fl lX
+.Ar commit
+.Xc
 .Dl Pq alias: Cm bo
 Reverse-merge changes from a single
 .Ar commit
@@ -2044,7 +2090,8 @@ committed with
 .Cm got backout
 commands,
 committed with
-.Cm got commit ,
+.Cm got commit
+where the log message of the backed-out commit will appear in the editor,
 or discarded again with
 .Cm got revert .
 .Pp
@@ -2055,6 +2102,43 @@ conflicts must be resolved first.
 .Cm got update .
 If any relevant files already contain merge conflicts, these
 conflicts must be resolved first.
+.Pp
+The options for
+.Nm
+.Cm backout
+are as follows:
+.Bl -tag -width Ds
+.It Fl l
+Display a list of references created by previous backout operations
+If a
+.Ar commit
+is specified, only show the reference identified by the specified commit.
+If invoked in a work tree, only references created in the current work tree
+will be displayed.
+Otherwise, all references will be displayed irrespective of the work tree
+in which they were created.
+This option cannot be used with
+.Cm X .
+.It Fl X
+Delete references created by previous backout operations, represented by
+references in the
+.Dq refs/got/worktree/backout
+reference namespace.
+.Pp
+If a
+.Ar commit
+is specified, only delete the reference identified by this commit.
+Otherwise, delete all references matching the
+.Dq refs/got/worktree/backout
+namespace.
+If invoked in a work tree, matching references created in the current
+work tree will be deleted.
+Otherwise, all matching references will be deleted irrespective of the
+work tree in which they were created.
+This option cannot be used with
+.Cm l .
+.El
+.Pp
 .Tg rb
 .It Xo
 .Cm rebase
blob - 87ce79c90db5ebd79d068d2738dfb439ab4da56a
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -8386,7 +8386,179 @@ static const struct got_error *
 	return NULL;
 }
 
+/*
+ * Shortcut work tree status callback to determine if the set of
+ * paths scanned has at least one versioned path that is modified.
+ * Set arg and return GOT_ERR_FILE_MODIFIED as soon as a path is
+ * passed with a status that is neither unchanged nor unversioned.
+ */
 static const struct got_error *
+worktree_has_changed_path(void *arg, unsigned char status,
+    unsigned char staged_status, const char *path,
+    struct got_object_id *blob_id, struct got_object_id *staged_blob_id,
+    struct got_object_id *commit_id, int dirfd, const char *de_name)
+{
+	int *has_changes = arg;
+
+	if (status == staged_status && (status == GOT_STATUS_DELETE))
+		status = GOT_STATUS_NO_CHANGE;
+
+	if (!(status == GOT_STATUS_NO_CHANGE ||
+	    status == GOT_STATUS_UNVERSIONED) ||
+	    staged_status != GOT_STATUS_NO_CHANGE) {
+		*has_changes = 1;
+		return got_error(GOT_ERR_FILE_MODIFIED);
+	}
+
+	return NULL;
+}
+
+/*
+ * Check that the changeset of the commit identified by id is
+ * comprised of at least one path that is modified in the work tree.
+ */
+static const struct got_error *
+commit_path_changed_in_worktree(int *add_logmsg, struct got_object_id *id,
+    struct got_worktree *worktree, struct got_repository *repo)
+{
+	const struct got_error		*err;
+	struct got_pathlist_head	 paths;
+	struct got_commit_object	*commit = NULL, *pcommit = NULL;
+	struct got_tree_object		*tree = NULL, *ptree = NULL;
+	struct got_object_qid		*pid;
+
+	TAILQ_INIT(&paths);
+
+	err = got_object_open_as_commit(&commit, repo, id);
+	if (err)
+		goto done;
+
+	pid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
+
+	err = got_object_open_as_commit(&pcommit, repo, &pid->id);
+	if (err)
+		goto done;
+
+	err = got_object_open_as_tree(&tree, repo,
+	    got_object_commit_get_tree_id(commit));
+	if (err)
+		goto done;
+
+	err = got_object_open_as_tree(&ptree, repo,
+	    got_object_commit_get_tree_id(pcommit));
+	if (err)
+		goto done;
+
+	err = got_diff_tree(ptree, tree, NULL, NULL, -1, -1, "", "", repo,
+	    got_diff_tree_collect_changed_paths, &paths, 0);
+	if (err)
+		goto done;
+
+	err = got_worktree_status(worktree, &paths, repo, 0,
+	    worktree_has_changed_path, add_logmsg, check_cancelled, NULL);
+	if (err && err->code == GOT_ERR_FILE_MODIFIED) {
+		/*
+		 * At least one changed path in the referenced commit is
+		 * modified in the work tree, that's all we need to know!
+		 */
+		err = NULL;
+	}
+
+done:
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_ALL);
+	if (commit)
+		got_object_commit_close(commit);
+	if (pcommit)
+		got_object_commit_close(pcommit);
+	if (tree)
+		got_object_tree_close(tree);
+	if (ptree)
+		got_object_tree_close(ptree);
+	return err;
+}
+
+/*
+ * Remove any "logmsg" reference comprised entirely of paths that have
+ * been reverted in this work tree. If any path in the logmsg ref changeset
+ * remains in a changed state in the worktree, do not remove the reference.
+ */
+static const struct got_error *
+rm_logmsg_ref(struct got_worktree *worktree, struct got_repository *repo)
+{
+	const struct got_error		*err;
+	struct got_reflist_head		 refs;
+	struct got_reflist_entry	*re;
+	struct got_commit_object	*commit = NULL;
+	struct got_object_id		*commit_id = NULL;
+	char				*uuidstr = NULL;
+
+	TAILQ_INIT(&refs);
+
+	err = got_worktree_get_uuid(&uuidstr, worktree);
+	if (err)
+		goto done;
+
+	err = got_ref_list(&refs, repo, "refs/got/worktree",
+	    got_ref_cmp_by_name, repo);
+	if (err)
+		goto done;
+
+	TAILQ_FOREACH(re, &refs, entry) {
+		const char	*refname;
+		int		 has_changes = 0;
+
+		refname = got_ref_get_name(re->ref);
+
+		if (!strncmp(refname, GOT_WORKTREE_CHERRYPICK_REF_PREFIX,
+		    GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN))
+			refname += GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN + 1;
+		else if (!strncmp(refname, GOT_WORKTREE_BACKOUT_REF_PREFIX,
+		    GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN))
+			refname += GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN + 1;
+		else
+			continue;
+
+		if (strncmp(refname, uuidstr, GOT_WORKTREE_UUID_STRLEN) == 0)
+			refname += GOT_WORKTREE_UUID_STRLEN + 1; /* skip '-' */
+		else
+			continue;
+
+		err = got_repo_match_object_id(&commit_id, NULL, refname,
+		    GOT_OBJ_TYPE_COMMIT, NULL, repo);
+		if (err)
+			goto done;
+
+		err = got_object_open_as_commit(&commit, repo, commit_id);
+		if (err)
+			goto done;
+
+		err = commit_path_changed_in_worktree(&has_changes, commit_id,
+		    worktree, repo);
+		if (err)
+			goto done;
+
+		if (!has_changes) {
+			err = got_ref_delete(re->ref, repo);
+			if (err)
+				goto done;
+		}
+
+		got_object_commit_close(commit);
+		commit = NULL;
+		free(commit_id);
+		commit_id = NULL;
+	}
+
+done:
+	free(uuidstr);
+	free(commit_id);
+	got_ref_list_free(&refs);
+	if (commit)
+		got_object_commit_close(commit);
+	return err;
+}
+
+static const struct got_error *
 cmd_revert(int argc, char *argv[])
 {
 	const struct got_error *error = NULL;
@@ -8463,7 +8635,11 @@ cmd_revert(int argc, char *argv[])
 			goto done;
 		}
 	}
-	error = apply_unveil(got_repo_get_path(repo), 1,
+
+	/*
+	 * XXX "c" perm needed on repo dir to delete merge references.
+	 */
+	error = apply_unveil(got_repo_get_path(repo), 0,
 	    got_worktree_get_root_path(worktree));
 	if (error)
 		goto done;
@@ -8505,6 +8681,8 @@ done:
 	cpa.action = "revert";
 	error = got_worktree_revert(worktree, &paths, revert_progress, NULL,
 	    pflag ? choose_patch : NULL, &cpa, repo);
+
+	error = rm_logmsg_ref(worktree, repo);
 done:
 	if (patch_script_file && fclose(patch_script_file) == EOF &&
 	    error == NULL)
@@ -8686,6 +8864,132 @@ cmd_commit(int argc, char *argv[])
 }
 
 static const struct got_error *
+cat_logmsg(FILE *f, struct got_commit_object *commit, const char *idstr,
+    const char *type, int has_content)
+{
+	const struct got_error	*err = NULL;
+	char			*logmsg = NULL;
+
+	err = got_object_commit_get_logmsg(&logmsg, commit);
+	if (err)
+		return err;
+
+	if (fprintf(f, "%s# log message of %s commit %s:%s",
+	    has_content ? "\n" : "", type, idstr, logmsg) < 0)
+		err = got_ferror(f, GOT_ERR_IO);
+
+	free(logmsg);
+	return err;
+}
+
+/*
+ * Lookup "logmsg" references of backed-out and cherrypicked commits
+ * belonging to the current work tree. If found, and the worktree has
+ * at least one modified file that was changed in the referenced commit,
+ * add its log message to *logmsg. Add all refs found to matched_refs
+ * to be scheduled for removal on successful commit.
+ */
+static const struct got_error *
+lookup_logmsg_ref(char **logmsg, struct got_reflist_head *matched_refs,
+    struct got_worktree *worktree, struct got_repository *repo)
+{
+	const struct got_error		*err;
+	struct got_commit_object	*commit = NULL;
+	struct got_object_id		*id = NULL;
+	struct got_reflist_head		 refs;
+	struct got_reflist_entry	*re, *re_match;
+	FILE				*f = NULL;
+	char				*uuidstr = NULL;
+	int				 added_logmsg = 0;
+
+	TAILQ_INIT(&refs);
+
+	err = got_opentemp_named(logmsg, &f, "got-commit-logmsg", "");
+	if (err)
+		goto done;
+
+	err = got_worktree_get_uuid(&uuidstr, worktree);
+	if (err)
+		goto done;
+
+	err = got_ref_list(&refs, repo, "refs/got/worktree",
+	    got_ref_cmp_by_name, repo);
+	if (err)
+		goto done;
+
+	TAILQ_FOREACH(re, &refs, entry) {
+		const char	*refname, *type;
+		int		 add_logmsg = 0;
+
+		refname = got_ref_get_name(re->ref);
+
+		if (strncmp(refname, GOT_WORKTREE_CHERRYPICK_REF_PREFIX,
+		    GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN) == 0) {
+			refname += GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN + 1;
+			type = "cherrypicked";
+		} else if (strncmp(refname, GOT_WORKTREE_BACKOUT_REF_PREFIX,
+		    GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN) == 0) {
+			refname += GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN + 1;
+			type = "backed-out";
+		} else
+			continue;
+
+		if (strncmp(refname, uuidstr, GOT_WORKTREE_UUID_STRLEN) == 0)
+			refname += GOT_WORKTREE_UUID_STRLEN + 1; /* skip '-' */
+		else
+			continue;
+
+		err = got_repo_match_object_id(&id, NULL, refname,
+		    GOT_OBJ_TYPE_COMMIT, NULL, repo);
+		if (err)
+			goto done;
+
+		err = got_object_open_as_commit(&commit, repo, id);
+		if (err)
+			goto done;
+
+		err = commit_path_changed_in_worktree(&add_logmsg, id,
+		    worktree, repo);
+		if (err)
+			goto done;
+
+		if (add_logmsg) {
+			err = cat_logmsg(f, commit, refname, type,
+			    added_logmsg);
+			if (err)
+				goto done;
+			if (!added_logmsg)
+				++added_logmsg;
+		}
+
+		err = got_reflist_entry_dup(&re_match, re);
+		if (err)
+			goto done;
+		TAILQ_INSERT_HEAD(matched_refs, re_match, entry);
+
+		got_object_commit_close(commit);
+		commit = NULL;
+		free(id);
+		id = NULL;
+	}
+
+done:
+	free(id);
+	free(uuidstr);
+	got_ref_list_free(&refs);
+	if (commit)
+		got_object_commit_close(commit);
+	if (f && fclose(f) == EOF && err == NULL)
+		err = got_error_from_errno("fclose");
+	if (!added_logmsg) {
+		if (*logmsg && unlink(*logmsg) != 0 && err == NULL)
+			err = got_error_from_errno2("unlink", *logmsg);
+		*logmsg = NULL;
+	}
+	return err;
+}
+
+static const struct got_error *
 cmd_commit(int argc, char *argv[])
 {
 	const struct got_error *error = NULL;
@@ -8702,8 +9006,11 @@ cmd_commit(int argc, char *argv[])
 	int allow_bad_symlinks = 0, non_interactive = 0, merge_in_progress = 0;
 	int show_diff = 1;
 	struct got_pathlist_head paths;
+	struct got_reflist_head refs;
+	struct got_reflist_entry *re;
 	int *pack_fds = NULL;
 
+	TAILQ_INIT(&refs);
 	TAILQ_INIT(&paths);
 	cl_arg.logmsg_path = NULL;
 
@@ -8825,6 +9132,13 @@ cmd_commit(int argc, char *argv[])
 	if (error)
 		goto done;
 
+	if (prepared_logmsg == NULL) {
+		error = lookup_logmsg_ref(&prepared_logmsg, &refs,
+		    worktree, repo);
+		if (error)
+			goto done;
+	}
+
 	cl_arg.editor = editor;
 	cl_arg.cmdline_log = logmsg;
 	cl_arg.prepared_log = prepared_logmsg;
@@ -8853,6 +9167,13 @@ done:
 	if (error)
 		goto done;
 	printf("Created commit %s\n", id_str);
+
+	TAILQ_FOREACH(re, &refs, entry) {
+		error = got_ref_delete(re->ref, repo);
+		if (error)
+			goto done;
+	}
+
 done:
 	if (preserve_logmsg) {
 		fprintf(stderr, "%s: log message preserved in %s\n",
@@ -8874,6 +9195,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
+	got_ref_list_free(&refs);
 	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	free(id_str);
@@ -9453,10 +9775,222 @@ __dead static void
 	return error;
 }
 
+/*
+ * Print and if delete is set delete all ref_prefix references.
+ * If wanted_ref is not NULL, only print or delete this reference.
+ */
+static const struct got_error *
+process_logmsg_refs(const char *ref_prefix, size_t prefix_len,
+    const char *wanted_ref, int delete, struct got_worktree *worktree,
+    struct got_repository *repo)
+{
+	const struct got_error			*err;
+	struct got_pathlist_head		 paths;
+	struct got_reflist_head			 refs;
+	struct got_reflist_entry		*re;
+	struct got_reflist_object_id_map	*refs_idmap = NULL;
+	struct got_commit_object		*commit = NULL;
+	struct got_object_id			*id = NULL;
+	char					*uuidstr = NULL;
+	int					 found = 0;
+
+	TAILQ_INIT(&refs);
+	TAILQ_INIT(&paths);
+
+	err = got_ref_list(&refs, repo, "refs/got/worktree",
+	    got_ref_cmp_by_name, repo);
+	if (err)
+		goto done;
+
+	err = got_reflist_object_id_map_create(&refs_idmap, &refs, repo);
+	if (err)
+		goto done;
+
+	if (worktree != NULL) {
+		err = got_worktree_get_uuid(&uuidstr, worktree);
+		if (err)
+			goto done;
+	}
+
+	if (wanted_ref) {
+		if (strncmp(wanted_ref, "refs/heads/", 11) == 0)
+			wanted_ref += 11;
+	}
+
+	TAILQ_FOREACH(re, &refs, entry) {
+		const char *refname;
+
+		refname = got_ref_get_name(re->ref);
+
+		err = check_cancelled(NULL);
+		if (err)
+			goto done;
+
+		if (strncmp(refname, ref_prefix, prefix_len) == 0)
+			refname += prefix_len + 1;  /* skip '-' delimiter */
+		else
+			continue;
+
+		if (worktree == NULL || strncmp(refname, uuidstr,
+		    GOT_WORKTREE_UUID_STRLEN) == 0)
+			refname += GOT_WORKTREE_UUID_STRLEN + 1; /* skip '-' */
+		else
+			continue;
+
+		err = got_repo_match_object_id(&id, NULL, refname,
+		    GOT_OBJ_TYPE_COMMIT, NULL, repo);
+		if (err)
+			goto done;
+
+		err = got_object_open_as_commit(&commit, repo, id);
+		if (err)
+			goto done;
+
+		if (wanted_ref)
+			found = strncmp(wanted_ref, refname,
+			    strlen(wanted_ref)) == 0;
+		if (wanted_ref && !found) {
+			struct got_reflist_head	*ci_refs;
+
+			ci_refs = got_reflist_object_id_map_lookup(refs_idmap,
+			    id);
+
+			if (ci_refs) {
+				char		*refs_str = NULL;
+				char const	*r = NULL;
+
+				err = build_refs_str(&refs_str, ci_refs, id,
+				    repo, 1);
+				if (err)
+					goto done;
+
+				r = refs_str;
+				while (r) {
+					if (strncmp(r, wanted_ref,
+					    strlen(wanted_ref)) == 0) {
+						found = 1;
+						break;
+					}
+					r = strchr(r, ' ');
+					if (r)
+						++r;
+				}
+				free(refs_str);
+			}
+		}
+
+		if (wanted_ref == NULL || found) {
+			if (delete) {
+				err = got_ref_delete(re->ref, repo);
+				if (err)
+					goto done;
+				printf("deleted: ");
+				err = print_commit_oneline(commit, id, repo,
+				    refs_idmap);
+			} else {
+				/*
+				 * Print paths modified by commit to help
+				 * associate commits with worktree changes.
+				 */
+				err = get_changed_paths(&paths, commit,
+				    repo, NULL);
+				if (err)
+					goto done;
+
+				err = print_commit(commit, id, repo, NULL,
+				    &paths, NULL, 0, 0, refs_idmap, NULL);
+				got_pathlist_free(&paths,
+				    GOT_PATHLIST_FREE_ALL);
+			}
+			if (err || found)
+				goto done;
+		}
+
+		got_object_commit_close(commit);
+		commit = NULL;
+		free(id);
+		id = NULL;
+	}
+
+	if (wanted_ref != NULL && !found)
+		err = got_error_fmt(GOT_ERR_NOT_REF, "%s", wanted_ref);
+
+done:
+	free(id);
+	free(uuidstr);
+	got_ref_list_free(&refs);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_ALL);
+	if (refs_idmap)
+		got_reflist_object_id_map_free(refs_idmap);
+	if (commit)
+		got_object_commit_close(commit);
+	return err;
+}
+
+/*
+ * Create new temp "logmsg" ref of the backed-out or cherrypicked commit
+ * identified by id for log messages to prepopulate the editor on commit.
+ */
+static const struct got_error *
+logmsg_ref(struct got_object_id *id, const char *prefix,
+    struct got_worktree *worktree, struct got_repository *repo)
+{
+	const struct got_error	*err = NULL;
+	char			*idstr, *ref = NULL, *refname = NULL;
+	int			 histedit_in_progress;
+	int			 rebase_in_progress, merge_in_progress;
+
+	/*
+	 * Silenty refuse to create merge reference if any histedit, merge,
+	 * or rebase operation is in progress.
+	 */
+	err = got_worktree_histedit_in_progress(&histedit_in_progress,
+	    worktree);
+	if (err)
+		return err;
+	if (histedit_in_progress)
+		return NULL;
+
+	err = got_worktree_rebase_in_progress(&rebase_in_progress, worktree);
+	if (err)
+		return err;
+	if (rebase_in_progress)
+		return NULL;
+
+	err = got_worktree_merge_in_progress(&merge_in_progress, worktree,
+	    repo);
+	if (err)
+		return err;
+	if (merge_in_progress)
+		return NULL;
+
+	err = got_object_id_str(&idstr, id);
+	if (err)
+		return err;
+
+	err = got_worktree_get_logmsg_ref_name(&refname, worktree, prefix);
+	if (err)
+		goto done;
+
+	if (asprintf(&ref, "%s-%s", refname, idstr) == -1) {
+		err = got_error_from_errno("asprintf");
+		goto done;
+	}
+
+	err = create_ref(ref, got_worktree_get_base_commit_id(worktree),
+	    -1, repo);
+done:
+	free(ref);
+	free(idstr);
+	free(refname);
+	return err;
+}
+
 __dead static void
 usage_cherrypick(void)
 {
-	fprintf(stderr, "usage: %s cherrypick commit-id\n", getprogname());
+	fprintf(stderr, "usage: %s cherrypick [-lX] [commit-id]\n",
+	    getprogname());
 	exit(1);
 }
 
@@ -9470,12 +10004,18 @@ cmd_cherrypick(int argc, char *argv[])
 	struct got_object_id *commit_id = NULL;
 	struct got_commit_object *commit = NULL;
 	struct got_object_qid *pid;
-	int ch;
+	int ch, list_refs = 0, remove_refs = 0;
 	struct got_update_progress_arg upa;
 	int *pack_fds = NULL;
 
-	while ((ch = getopt(argc, argv, "")) != -1) {
+	while ((ch = getopt(argc, argv, "lX")) != -1) {
 		switch (ch) {
+		case 'l':
+			list_refs = 1;
+			break;
+		case 'X':
+			remove_refs = 1;
+			break;
 		default:
 			usage_cherrypick();
 			/* NOTREACHED */
@@ -9490,8 +10030,13 @@ cmd_cherrypick(int argc, char *argv[])
 	    "unveil", NULL) == -1)
 		err(1, "pledge");
 #endif
-	if (argc != 1)
+	if (list_refs || remove_refs) {
+		if (argc != 0 && argc != 1)
+			usage_cherrypick();
+	} else if (argc != 1)
 		usage_cherrypick();
+	if (list_refs && remove_refs)
+		option_conflict('l', 'X');
 
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
@@ -9505,22 +10050,35 @@ cmd_cherrypick(int argc, char *argv[])
 
 	error = got_worktree_open(&worktree, cwd);
 	if (error) {
-		if (error->code == GOT_ERR_NOT_WORKTREE)
-			error = wrap_not_worktree_error(error, "cherrypick",
-			    cwd);
-		goto done;
+		if (list_refs || remove_refs) {
+			if (error->code != GOT_ERR_NOT_WORKTREE)
+				goto done;
+		} else {
+			if (error->code == GOT_ERR_NOT_WORKTREE)
+				error = wrap_not_worktree_error(error,
+				    "cherrypick", cwd);
+			goto done;
+		}
 	}
 
-	error = got_repo_open(&repo, got_worktree_get_repo_path(worktree),
+	error = got_repo_open(&repo,
+	    worktree ? got_worktree_get_repo_path(worktree) : cwd,
 	    NULL, pack_fds);
 	if (error != NULL)
 		goto done;
 
 	error = apply_unveil(got_repo_get_path(repo), 0,
-	    got_worktree_get_root_path(worktree));
+	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
 		goto done;
 
+	if (list_refs || remove_refs) {
+		error = process_logmsg_refs(GOT_WORKTREE_CHERRYPICK_REF_PREFIX,
+		    GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN,
+		    argc == 1 ? argv[0] : NULL, remove_refs, worktree, repo);
+		goto done;
+	}
+
 	error = got_repo_match_object_id(&commit_id, NULL, argv[0],
 	    GOT_OBJ_TYPE_COMMIT, NULL, repo);
 	if (error)
@@ -9540,8 +10098,13 @@ cmd_cherrypick(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
-	if (upa.did_something)
+	if (upa.did_something) {
+		error = logmsg_ref(commit_id,
+		    GOT_WORKTREE_CHERRYPICK_REF_PREFIX, worktree, repo);
+		if (error)
+			goto done;
 		printf("Merged commit %s\n", commit_id_str);
+	}
 	print_merge_progress_stats(&upa);
 done:
 	if (commit)
@@ -9567,7 +10130,7 @@ usage_backout(void)
 __dead static void
 usage_backout(void)
 {
-	fprintf(stderr, "usage: %s backout commit-id\n", getprogname());
+	fprintf(stderr, "usage: %s backout [-lX] [commit-id]\n", getprogname());
 	exit(1);
 }
 
@@ -9581,12 +10144,18 @@ cmd_backout(int argc, char *argv[])
 	struct got_object_id *commit_id = NULL;
 	struct got_commit_object *commit = NULL;
 	struct got_object_qid *pid;
-	int ch;
+	int ch, list_refs = 0, remove_refs = 0;
 	struct got_update_progress_arg upa;
 	int *pack_fds = NULL;
 
-	while ((ch = getopt(argc, argv, "")) != -1) {
+	while ((ch = getopt(argc, argv, "lX")) != -1) {
 		switch (ch) {
+		case 'l':
+			list_refs = 1;
+			break;
+		case 'X':
+			remove_refs = 1;
+			break;
 		default:
 			usage_backout();
 			/* NOTREACHED */
@@ -9601,8 +10170,13 @@ cmd_backout(int argc, char *argv[])
 	    "unveil", NULL) == -1)
 		err(1, "pledge");
 #endif
-	if (argc != 1)
+	if (list_refs || remove_refs) {
+		if (argc != 0 && argc != 1)
+			usage_backout();
+	} else if (argc != 1)
 		usage_backout();
+	if (list_refs && remove_refs)
+		option_conflict('l', 'X');
 
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
@@ -9616,21 +10190,35 @@ cmd_backout(int argc, char *argv[])
 
 	error = got_worktree_open(&worktree, cwd);
 	if (error) {
-		if (error->code == GOT_ERR_NOT_WORKTREE)
-			error = wrap_not_worktree_error(error, "backout", cwd);
-		goto done;
+		if (list_refs || remove_refs) {
+			if (error->code != GOT_ERR_NOT_WORKTREE)
+				goto done;
+		} else {
+			if (error->code == GOT_ERR_NOT_WORKTREE)
+				error = wrap_not_worktree_error(error,
+				    "backout", cwd);
+			goto done;
+		}
 	}
 
-	error = got_repo_open(&repo, got_worktree_get_repo_path(worktree),
+	error = got_repo_open(&repo,
+	    worktree ? got_worktree_get_repo_path(worktree) : cwd,
 	    NULL, pack_fds);
 	if (error != NULL)
 		goto done;
 
 	error = apply_unveil(got_repo_get_path(repo), 0,
-	    got_worktree_get_root_path(worktree));
+	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
 		goto done;
 
+	if (list_refs || remove_refs) {
+		error = process_logmsg_refs(GOT_WORKTREE_BACKOUT_REF_PREFIX,
+		    GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN,
+		    argc == 1 ? argv[0] : NULL, remove_refs, worktree, repo);
+		goto done;
+	}
+
 	error = got_repo_match_object_id(&commit_id, NULL, argv[0],
 	    GOT_OBJ_TYPE_COMMIT, NULL, repo);
 	if (error)
@@ -9654,8 +10242,13 @@ cmd_backout(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
-	if (upa.did_something)
+	if (upa.did_something) {
+		error = logmsg_ref(commit_id, GOT_WORKTREE_BACKOUT_REF_PREFIX,
+		    worktree, repo);
+		if (error)
+			goto done;
 		printf("Backed out commit %s\n", commit_id_str);
+	}
 	print_merge_progress_stats(&upa);
 done:
 	if (commit)
@@ -10116,6 +10709,62 @@ process_backup_refs(const char *backup_ref_prefix,
 }
 
 static const struct got_error *
+worktree_has_logmsg_ref(const char *caller, struct got_worktree *worktree,
+    struct got_repository *repo)
+{
+	const struct got_error		*err;
+	struct got_reflist_head		 refs;
+	struct got_reflist_entry	*re;
+	char				*uuidstr = NULL;
+	static char			 msg[160];
+
+	TAILQ_INIT(&refs);
+
+	err = got_worktree_get_uuid(&uuidstr, worktree);
+	if (err)
+		goto done;
+
+	err = got_ref_list(&refs, repo, "refs/got/worktree",
+	    got_ref_cmp_by_name, repo);
+	if (err)
+		goto done;
+
+	TAILQ_FOREACH(re, &refs, entry) {
+		const char *cmd, *refname, *type;
+
+		refname = got_ref_get_name(re->ref);
+
+		if (strncmp(refname, GOT_WORKTREE_CHERRYPICK_REF_PREFIX,
+		    GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN) == 0) {
+			refname += GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN + 1;
+			cmd = "cherrypick";
+			type = "cherrypicked";
+		} else if (strncmp(refname, GOT_WORKTREE_BACKOUT_REF_PREFIX,
+		    GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN) == 0) {
+			refname += GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN + 1;
+			cmd = "backout";
+			type = "backed-out";
+		} else
+			continue;
+
+		if (strncmp(refname, uuidstr, GOT_WORKTREE_UUID_STRLEN) != 0)
+			continue;
+
+		snprintf(msg, sizeof(msg),
+		    "work tree has references created by %s commits which "
+		    "must be removed with 'got %s -X' before running the %s "
+		    "command", type, cmd, caller);
+		err = got_error_msg(GOT_ERR_WORKTREE_META, msg);
+		goto done;
+	}
+
+done:
+	free(uuidstr);
+	got_ref_list_free(&refs);
+	return err;
+}
+
+static const struct got_error *
 process_backup_refs(const char *backup_ref_prefix,
     const char *wanted_branch_name,
     int delete, struct got_repository *repo)
@@ -10356,6 +11005,12 @@ cmd_rebase(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+	if (worktree != NULL && !list_backups && !delete_backups) {
+		error = worktree_has_logmsg_ref("rebase", worktree, repo);
+		if (error)
+			goto done;
+	}
+
 	error = get_author(&committer, repo, worktree);
 	if (error && error->code != GOT_ERR_COMMIT_NO_AUTHOR)
 		goto done;
@@ -11693,6 +12348,12 @@ cmd_histedit(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+	if (worktree != NULL && !list_backups && !delete_backups) {
+		error = worktree_has_logmsg_ref("histedit", worktree, repo);
+		if (error)
+			goto done;
+	}
+
 	error = got_worktree_rebase_in_progress(&rebase_in_progress, worktree);
 	if (error)
 		goto done;
@@ -12326,6 +12987,12 @@ cmd_merge(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+	if (worktree != NULL) {
+		error = worktree_has_logmsg_ref("merge", worktree, repo);
+		if (error)
+			goto done;
+	}
+
 	error = apply_unveil(got_repo_get_path(repo), 0,
 	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
blob - d3c26b9a044948f5b93e8f2f6adf673b8dd7e599
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -87,6 +87,21 @@ const struct got_error *got_worktree_match_path_prefix
     struct got_worktree *, const char *);
 
 /*
+ * Prefix for references pointing at base commit of backout/cherrypick commits.
+ * Reference path takes the form: PREFIX-WORKTREE_UUID-COMMIT_ID
+ */
+#define GOT_WORKTREE_CHERRYPICK_REF_PREFIX	"refs/got/worktree/cherrypick"
+#define GOT_WORKTREE_BACKOUT_REF_PREFIX		"refs/got/worktree/backout"
+
+#define GOT_WORKTREE_CHERRYPICK_REF_PREFIX_LEN		\
+	sizeof(GOT_WORKTREE_CHERRYPICK_REF_PREFIX) - 1
+#define GOT_WORKTREE_BACKOUT_REF_PREFIX_LEN		\
+	sizeof(GOT_WORKTREE_BACKOUT_REF_PREFIX) - 1
+#define GOT_WORKTREE_UUID_STRLEN	36
+
+const struct got_error *got_worktree_get_logmsg_ref_name(char **,
+    struct got_worktree *, const char *);
+/*
  * Get the name of a work tree's HEAD reference.
  */
 const char *got_worktree_get_head_ref_name(struct got_worktree *);
blob - febf807fc9a1e5b9c2cdab208adc8217a492980e
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -2242,6 +2242,13 @@ got_worktree_get_base_ref_name(char **refname, struct 
 }
 
 const struct got_error *
+got_worktree_get_logmsg_ref_name(char **refname, struct got_worktree *worktree,
+    const char *prefix)
+{
+	return get_ref_name(refname, worktree, prefix);
+}
+
+const struct got_error *
 got_worktree_get_base_ref_name(char **refname, struct got_worktree *worktree)
 {
 	return get_ref_name(refname, worktree, GOT_WORKTREE_BASE_REF_PREFIX);

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68