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

From:
Mark Jamsek <mark@jamsek.com>
Subject:
backout/cherrypick -l output tweaked
To:
Game of Trees <gameoftrees@openbsd.org>
Date:
Tue, 31 Jan 2023 01:40:56 +1100

Download raw body.

Thread 
As per discussion with stsp on irc, the below diff tweaks the output of
'got {backout/cherrypick} -l' in two ways, the first of which I was not
too sure about, but I actually think it makes sense:

  1. replace "commit" in the header with "backout" or "cherrypick"
     depending on what command was run/which ref type is displayed
  2. display "worktree: WORKTREE-UUID" after the logmsg and changeset if
     invoked from the repository

The reason I was unsure about (1) is because the user already knows
whether it's a logmsg ref from a backout or cherrypick because of the
command they must have just run (i.e., 'got bo -l' or 'got cy -l'). But
making this connection is good as a form of confirmation for the user.
Plus, although the refs are really all about log messages, it's good to
have something distinguish the output between different commands.

(2) is a definite, imo; this information needs to be displayed in
some way when invoked outside a work tree as it's the only way the user
can connect refs to the tree they belong. I initially displayed both the
worktree UUID and ref like so:

-----------------------------------------------
cherrypick 862b68b7d40a3aec7cc411c275359a19da6fae73
from: Mark Jamsek <mark@jamsek.dev>
date: Thu Jan 26 06:36:17 2023 UTC

 foo

 M  node

work tree: 6a68c142-68ca-4237-8fd3-96de08cc7306
refs/got/worktree/cherrypick-6a68c142-68ca-4237-8fd3-96de08cc7306-862b68b7d40a3aec7cc411c275359a19da6fae73
0

But I'm not sure the full ref path adds much. It can be surmised by
appending the type (i.e., "backout" or "cherrypick"), which is now
displayed in the header, and the UUID and commit hash--both now also in
the output--to the namespace (i.e., "refs/got/worktree"), which got(1)
mentions. So all the information is present already. Nonetheless, I'm
not sure what the full path adds.

That said, I'm completely fine with displaying it if we think it's
a good idea!

The diff is really simple, but there's a bit of churn in regress to
account for the header change. I've not amended regress further to test
for the "work tree: UUID" output till we're settled on the extent of the
output.

diffstat /home/mark/src/got
 M  got/got.c                      |  25+  9-
 M  regress/cmdline/backout.sh     |   9+  9-
 M  regress/cmdline/cherrypick.sh  |  10+  9-
 M  regress/cmdline/commit.sh      |   1+  1-

4 files changed, 45 insertions(+), 28 deletions(-)

diff /home/mark/src/got
commit - 2ad4494321ae7d90e16d4add6a429ca0979cf546
path + /home/mark/src/got
blob - 60c761947d9b8dfb848269d6bba3e25b341bf7d6
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -4198,7 +4198,8 @@ print_commit(struct got_commit_object *commit, struct 
     struct got_repository *repo, const char *path,
     struct got_pathlist_head *changed_paths,
     struct got_diffstat_cb_arg *diffstat, int show_patch, int diff_context,
-    struct got_reflist_object_id_map *refs_idmap, const char *custom_refs_str)
+    struct got_reflist_object_id_map *refs_idmap, const char *custom_refs_str,
+    const char *prefix)
 {
 	const struct got_error *err = NULL;
 	FILE *f = NULL;
@@ -4224,10 +4225,12 @@ print_commit(struct got_commit_object *commit, struct 
 
 	printf(GOT_COMMIT_SEP_STR);
 	if (custom_refs_str)
-		printf("commit %s (%s)\n", id_str, custom_refs_str);
+		printf("%s %s (%s)\n", prefix ? prefix : "commit", id_str,
+		    custom_refs_str);
 	else
-		printf("commit %s%s%s%s\n", id_str, refs_str ? " (" : "",
-		    refs_str ? refs_str : "", refs_str ? ")" : "");
+		printf("%s %s%s%s%s\n", prefix ? prefix : "commit", id_str,
+		    refs_str ? " (" : "", refs_str ? refs_str : "",
+		    refs_str ? ")" : "");
 	free(id_str);
 	id_str = NULL;
 	free(refs_str);
@@ -4412,7 +4415,7 @@ print_commits(struct got_object_id *root_id, struct go
 				    (show_changed_paths || show_diffstat) ?
 				    &changed_paths : NULL,
 				    show_diffstat ? &dsa : NULL, show_patch,
-				    diff_context, refs_idmap, NULL);
+				    diff_context, refs_idmap, NULL, NULL);
 			got_object_commit_close(commit);
 			if (err)
 				break;
@@ -4447,7 +4450,7 @@ print_commits(struct got_object_id *root_id, struct go
 				    (show_changed_paths || show_diffstat) ?
 				    &changed_paths : NULL,
 				    show_diffstat ? &dsa : NULL, show_patch,
-				    diff_context, refs_idmap, NULL);
+				    diff_context, refs_idmap, NULL, NULL);
 			got_object_commit_close(commit);
 			if (err)
 				break;
@@ -9815,6 +9818,7 @@ process_logmsg_refs(const char *ref_prefix, size_t pre
 	struct got_reflist_object_id_map	*refs_idmap = NULL;
 	struct got_commit_object		*commit = NULL;
 	struct got_object_id			*id = NULL;
+	const char				*header_prefix;
 	char					*uuidstr = NULL;
 	int					 found = 0;
 
@@ -9840,8 +9844,13 @@ process_logmsg_refs(const char *ref_prefix, size_t pre
 			wanted_ref += 11;
 	}
 
+	if (strcmp(ref_prefix, GOT_WORKTREE_BACKOUT_REF_PREFIX) == 0)
+		header_prefix = "backout";
+	else
+		header_prefix = "cherrypick";
+
 	TAILQ_FOREACH(re, &refs, entry) {
-		const char *refname;
+		const char *refname, *wt;
 
 		refname = got_ref_get_name(re->ref);
 
@@ -9854,6 +9863,8 @@ process_logmsg_refs(const char *ref_prefix, size_t pre
 		else
 			continue;
 
+		wt = refname;
+
 		if (worktree == NULL || strncmp(refname, uuidstr,
 		    GOT_WORKTREE_UUID_STRLEN) == 0)
 			refname += GOT_WORKTREE_UUID_STRLEN + 1; /* skip '-' */
@@ -9921,9 +9932,14 @@ process_logmsg_refs(const char *ref_prefix, size_t pre
 					goto done;
 
 				err = print_commit(commit, id, repo, NULL,
-				    &paths, NULL, 0, 0, refs_idmap, NULL);
+				    &paths, NULL, 0, 0, refs_idmap, NULL,
+				    header_prefix);
 				got_pathlist_free(&paths,
 				    GOT_PATHLIST_FREE_ALL);
+
+				if (worktree == NULL)
+					printf("work tree: %.*s\n\n",
+					    GOT_WORKTREE_UUID_STRLEN, wt);
 			}
 			if (err || found)
 				goto done;
@@ -10657,7 +10673,7 @@ print_backup_ref(const char *branch_name, const char *
 		return got_error_from_errno("asprintf");
 
 	err = print_commit(old_commit, old_commit_id, repo, NULL, NULL, NULL,
-	    0, 0, refs_idmap, custom_refs_str);
+	    0, 0, refs_idmap, custom_refs_str, NULL);
 	if (err)
 		goto done;
 
blob - bf66824996f21369543f644f760e178198b0d78a
file + regress/cmdline/backout.sh
--- regress/cmdline/backout.sh
+++ regress/cmdline/backout.sh
@@ -290,7 +290,7 @@ test_backout_logmsg_ref() {
 	for r in $sorted; do
 		echo $sep >> $testroot/stdout.expected
 		if [ $r == $branch_rev ]; then
-			echo "commit $r" >> $testroot/stdout.expected
+			echo "backout $r" >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date" >> $testroot/stdout.expected
 			printf " \n $logmsg\n \n" >> $testroot/stdout.expected
@@ -300,7 +300,7 @@ test_backout_logmsg_ref() {
 			echo "Deleted: $ymd $short_id $logmsg" >> \
 			    $testroot/stdout.wt_deleted
 		else
-			echo "commit $r (newbranch)" \
+			echo "backout $r (newbranch)" \
 			    >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date2" >> $testroot/stdout.expected
@@ -325,7 +325,7 @@ test_backout_logmsg_ref() {
 
 	# only show log message ref of the specified commit id
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch_rev" >> $testroot/stdout.expected
+	echo "backout $branch_rev" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date" >> $testroot/stdout.expected
 	printf " \n $logmsg\n \n" >> $testroot/stdout.expected
@@ -343,7 +343,7 @@ test_backout_logmsg_ref() {
 
 	# only show log message ref of the specified symref
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch_rev2 (newbranch)" >> $testroot/stdout.expected
+	echo "backout $branch_rev2 (newbranch)" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date2" >> $testroot/stdout.expected
 	printf " \n $logmsg2\n \n" >> $testroot/stdout.expected
@@ -400,7 +400,7 @@ test_backout_logmsg_ref() {
 	for r in $sorted; do
 		echo $sep >> $testroot/stdout.expected
 		if [ $r == $branch2_rev ]; then
-			echo "commit $r" >> $testroot/stdout.expected
+			echo "backout $r" >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date" >> $testroot/stdout.expected
 			printf " \n $b2_logmsg\n \n" >> \
@@ -408,7 +408,7 @@ test_backout_logmsg_ref() {
 			printf "$b2_changeset\n\n" >> \
 			    $testroot/stdout.expected
 		else
-			echo "commit $r (newbranch2)" \
+			echo "backout $r (newbranch2)" \
 			    >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date2" >> $testroot/stdout.expected
@@ -435,10 +435,10 @@ test_backout_logmsg_ref() {
 
 	echo -n > $testroot/stdout.expected
 	for r in $sorted; do
-		echo "commit $r" >> $testroot/stdout.expected
+		echo "backout $r" >> $testroot/stdout.expected
 	done
 
-	(cd $testroot/repo && got backout -l | grep ^commit | \
+	(cd $testroot/repo && got backout -l | grep ^backout | \
 	    sort | cut -f1,2 -d' ' > $testroot/stdout)
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
@@ -490,7 +490,7 @@ test_backout_logmsg_ref() {
 
 	# make sure the remaining ref in work tree 2 was not also deleted
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch2_rev2 (newbranch2)" >> $testroot/stdout.expected
+	echo "backout $branch2_rev2 (newbranch2)" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date2" >> $testroot/stdout.expected
 	printf " \n $b2_logmsg2\n \n" >> $testroot/stdout.expected
blob - 2b2eb4582ce3a130bac8aedaf3901a84ce31458a
file + regress/cmdline/cherrypick.sh
--- regress/cmdline/cherrypick.sh
+++ regress/cmdline/cherrypick.sh
@@ -1773,7 +1773,7 @@ test_cherrypick_logmsg_ref() {
 	for r in $sorted; do
 		echo $sep >> $testroot/stdout.expected
 		if [ $r == $branch_rev ]; then
-			echo "commit $r" >> $testroot/stdout.expected
+			echo "cherrypick $r" >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date" >> $testroot/stdout.expected
 			printf " \n $logmsg\n \n" >> $testroot/stdout.expected
@@ -1783,7 +1783,7 @@ test_cherrypick_logmsg_ref() {
 			echo "Deleted: $ymd $short_id $logmsg" >> \
 			    $testroot/stdout.wt_deleted
 		else
-			echo "commit $r (newbranch)" \
+			echo "cherrypick $r (newbranch)" \
 			    >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date2" >> $testroot/stdout.expected
@@ -1808,7 +1808,7 @@ test_cherrypick_logmsg_ref() {
 
 	# only show log message ref of the specified commit id
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch_rev" >> $testroot/stdout.expected
+	echo "cherrypick $branch_rev" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date" >> $testroot/stdout.expected
 	printf " \n $logmsg\n \n" >> $testroot/stdout.expected
@@ -1826,7 +1826,7 @@ test_cherrypick_logmsg_ref() {
 
 	# only show log message ref of the specified symref
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch_rev2 (newbranch)" >> $testroot/stdout.expected
+	echo "cherrypick $branch_rev2 (newbranch)" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date2" >> $testroot/stdout.expected
 	printf " \n $logmsg2\n \n" >> $testroot/stdout.expected
@@ -1883,7 +1883,7 @@ test_cherrypick_logmsg_ref() {
 	for r in $sorted; do
 		echo $sep >> $testroot/stdout.expected
 		if [ $r == $branch2_rev ]; then
-			echo "commit $r" >> $testroot/stdout.expected
+			echo "cherrypick $r" >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date" >> $testroot/stdout.expected
 			printf " \n $b2_logmsg\n \n" >> \
@@ -1891,7 +1891,7 @@ test_cherrypick_logmsg_ref() {
 			printf "$b2_changeset\n\n" >> \
 			    $testroot/stdout.expected
 		else
-			echo "commit $r (newbranch2)" \
+			echo "cherrypick $r (newbranch2)" \
 			    >> $testroot/stdout.expected
 			echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 			echo "date: $date2" >> $testroot/stdout.expected
@@ -1918,10 +1918,10 @@ test_cherrypick_logmsg_ref() {
 
 	echo -n > $testroot/stdout.expected
 	for r in $sorted; do
-		echo "commit $r" >> $testroot/stdout.expected
+		echo "cherrypick $r" >> $testroot/stdout.expected
 	done
 
-	(cd $testroot/repo && got cherrypick -l | grep ^commit | \
+	(cd $testroot/repo && got cherrypick -l | grep ^cherrypick | \
 	    sort | cut -f1,2 -d' ' > $testroot/stdout)
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
@@ -1973,7 +1973,8 @@ test_cherrypick_logmsg_ref() {
 
 	# make sure the remaining ref in work tree 2 was not also deleted
 	echo $sep > $testroot/stdout.expected
-	echo "commit $branch2_rev2 (newbranch2)" >> $testroot/stdout.expected
+	echo "cherrypick $branch2_rev2 (newbranch2)" \
+	    >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $date2" >> $testroot/stdout.expected
 	printf " \n $b2_logmsg2\n \n" >> $testroot/stdout.expected
blob - 9d6cc96be651e682ca185e858f8dfff8eaca919a
file + regress/cmdline/commit.sh
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -1907,7 +1907,7 @@ EOF
 	# confirm logmsg ref was not deleted with got cherrypick -l
 	echo "-----------------------------------------------" \
 	    > $testroot/stdout.expected
-	echo "commit $branch_rev (newbranch)" >> $testroot/stdout.expected
+	echo "cherrypick $branch_rev (newbranch)" >> $testroot/stdout.expected
 	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
 	echo "date: $d" >> $testroot/stdout.expected
 	echo " " >> $testroot/stdout.expected

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