From: Stefan Sperling <stsp@stsp.name>
Subject: fix histedit -m with filemode-only changes
To: gameoftrees@openbsd.org
Date: Sat, 28 Jan 2023 21:57:39 +0100

This fixes an issue I stumbled across receently.

The bug is in merge_file_cb() so it probably affects more than
just histedit -m. But histedit -m provides a nice test case.

ok?

 
 fix histedit -m on a commit which only changes filemode bits
 
 The commit was being miscategorized as a no-op change and dropped.
 Now the commit is retained and its log message is updated as expected.
 
diff 8bf76af3f358c0ad73a45348c76647955efc78e8 d1f87d740d63380dfef5ca5dd501362616abc817
commit - 8bf76af3f358c0ad73a45348c76647955efc78e8
commit + d1f87d740d63380dfef5ca5dd501362616abc817
blob - 9bc22d9248efae066b64b995c6b3c61b7ddac0b8
blob + ae0d5f56dbc189ecc9aeeda06b9ac17ee688f34c
--- TODO
+++ TODO
@@ -18,8 +18,6 @@ got:
   'got fetch' is supposed to work. To make this easier, if the HEAD symref
   points to a non-existent reference it should be updated by 'got fetch'
   to match the HEAD symref sent by the server.
-- got histedit -m on a commit which only changes filemode bits results
-  in the commit being miscategorized as a no-op change and be dropped.
 
 network protocol:
 - add http(s) transport with libtls, speaking the two Git HTTP protocols
blob - 68190506ee35314d0cd4d2e3db28b27330f1f887
blob + 158fcd73c1c9600a23bfabea6968f055ea3e6f0f
--- lib/worktree.c
+++ lib/worktree.c
@@ -2897,7 +2897,7 @@ merge_file_cb(void *arg, struct got_blob_object *blob1
 			}
 			err = merge_file(&local_changes_subsumed, a->worktree,
 			    f_orig, f_deriv, f_deriv2, ondisk_path, path2,
-			    sb.st_mode, a->label_orig, NULL, label_deriv2,
+			    mode2, a->label_orig, NULL, label_deriv2,
 			    GOT_DIFF_ALGORITHM_PATIENCE, repo,
 			    a->progress_cb, a->progress_arg);
 		}
blob - 68ef1ea4051342b29a662d86ab87e8a7f3705e1c
blob + f46da7c9f9539aa4963df3b758671a48f3c53b41
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2213,6 +2213,102 @@ test_parseargs "$@"
 	test_done "$testroot" 0
 }
 
+test_histedit_mesg_filemode_change() {
+	local testroot=`test_init histedit_mode_change`
+
+	local orig_commit=`git_show_head $testroot/repo`
+	local orig_author_time=`git_show_author_time $testroot/repo`
+
+	chmod +x $testroot/repo/alpha
+	git_commit $testroot/repo -m "set x bit on alpha"
+	local old_commit1=`git_show_head $testroot/repo`
+	local old_author_time1=`git_show_author_time $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
+
+	if [ -x $testroot/wt/alpha ]; then
+		echo "file alpha has unexpected executable bit" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	cat > $testroot/editor.sh <<EOF
+#!/bin/sh
+sed -i 's/ x bit / executable bit /' "\$1"
+EOF
+
+	chmod +x $testroot/editor.sh
+
+	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
+		got histedit -m > $testroot/stdout)
+
+	local new_commit1=`git_show_head $testroot/repo`
+	local new_author_time1=`git_show_author_time $testroot/repo`
+
+	local short_old_commit1=`trim_obj_id 28 $old_commit1`
+	local short_new_commit1=`trim_obj_id 28 $new_commit1`
+
+	echo "G  alpha" > $testroot/stdout.expected
+	echo "$short_old_commit1 -> $short_new_commit1: set executable bit on alpha" \
+		>> $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
+
+	echo "alpha" > $testroot/content.expected
+	cat $testroot/wt/alpha > $testroot/content
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if [ ! -x $testroot/wt/alpha ]; then
+		echo "file alpha lost its executable bit" >&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 -l1 | grep ' set executable bit on alpha' \
+		> $testroot/stdout)
+
+	echo ' set executable bit on alpha' > $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
+
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -2236,3 +2332,4 @@ run_test test_histedit_umask
 run_test test_histedit_mesg_invalid
 run_test test_histedit_resets_committer
 run_test test_histedit_umask
+run_test test_histedit_mesg_filemode_change