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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
fix histedit not merging added files correctly
To:
gameoftrees@openbsd.org
Date:
Tue, 16 Sep 2025 17:09:19 +0200

Download raw body.

Thread
fix an issue where histedit would not merge added files correctly

When a file is added during a merge and also already exists on disk as
an unversioned file, we mistakenly did not put the file onto the list
of changed files which histedit includes in the next commit. This could
result in commits which only add new files being dropped as no-op commits.

If future commits depend on the file being present this would result in
errors during the histedit operation. Otherwise, the file would no longer
exist in the edited history.

ok?


M  lib/worktree.c               |  12+  11-
M  regress/cmdline/histedit.sh  |  60+   0-

2 files changed, 72 insertions(+), 11 deletions(-)

commit - 4113e6853e3344ea50288e4cd820f0a88b76e37e
commit + 934cdc3a6c450fb7ccac5f52aa9cc6a5bf9544fe
blob - 628ec31796975901db85d57d58dc1aea8554e73d
blob + 694dfa8eb8ac4166012fcf9da43bd2096817663a
--- lib/worktree.c
+++ lib/worktree.c
@@ -1155,7 +1155,7 @@ install_blob(struct got_worktree *worktree, const char
     const char *path, mode_t te_mode, mode_t st_mode,
     struct got_blob_object *blob, int restoring_missing_file,
     int reverting_versioned_file, int installing_bad_symlink,
-    int path_is_unversioned, int *update_timestamps,
+    int path_is_unversioned, int adding_merged_file, int *update_timestamps,
     struct got_repository *repo,
     got_worktree_checkout_cb progress_cb, void *progress_arg);
 
@@ -1302,7 +1302,7 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 			return install_blob(worktree, ondisk_path, path,
 			    GOT_DEFAULT_FILE_MODE, GOT_DEFAULT_FILE_MODE, blob,
 			    restoring_missing_file, reverting_versioned_file,
-			    1, path_is_unversioned, NULL, repo, progress_cb,
+			    1, path_is_unversioned, 0, NULL, repo, progress_cb,
 			    progress_arg);
 		}
 		if (len > 0) {
@@ -1326,7 +1326,7 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 		err = install_blob(worktree, ondisk_path, path,
 		    GOT_DEFAULT_FILE_MODE, GOT_DEFAULT_FILE_MODE, blob,
 		    restoring_missing_file, reverting_versioned_file, 1,
-		    path_is_unversioned, NULL, repo,
+		    path_is_unversioned, 0, NULL, repo,
 		    progress_cb, progress_arg);
 		return err;
 	}
@@ -1390,7 +1390,7 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 			err = install_blob(worktree, ondisk_path, path,
 			    GOT_DEFAULT_FILE_MODE, GOT_DEFAULT_FILE_MODE, blob,
 			    restoring_missing_file, reverting_versioned_file, 1,
-			    path_is_unversioned, NULL, repo,
+			    path_is_unversioned, 0, NULL, repo,
 			    progress_cb, progress_arg);
 		} else if (errno == ENOTDIR) {
 			err = got_error_path(ondisk_path,
@@ -1410,8 +1410,8 @@ install_blob(struct got_worktree *worktree, const char
     const char *path, mode_t te_mode, mode_t st_mode,
     struct got_blob_object *blob, int restoring_missing_file,
     int reverting_versioned_file, int installing_bad_symlink,
-    int path_is_unversioned, int *update_timestamps,
-    struct got_repository *repo,
+    int path_is_unversioned, int adding_merged_file,
+    int *update_timestamps, struct got_repository *repo,
     got_worktree_checkout_cb progress_cb, void *progress_arg)
 {
 	const struct got_error *err = NULL;
@@ -1450,7 +1450,8 @@ install_blob(struct got_worktree *worktree, const char
 				if (update_timestamps)
 					*update_timestamps = 0;
 				err = (*progress_cb)(progress_arg,
-				    GOT_STATUS_EXISTS, path);
+				    adding_merged_file ?
+				    GOT_STATUS_ADD : GOT_STATUS_EXISTS, path);
 				goto done;
 			}
 			if (!(S_ISLNK(st_mode) && S_ISREG(te_mode)) &&
@@ -2150,7 +2151,7 @@ update_blob(struct got_worktree *worktree,
 			err = install_blob(worktree, ondisk_path, path,
 			    te->mode, sb.st_mode, blob,
 			    status == GOT_STATUS_MISSING, 0, 0,
-			    status == GOT_STATUS_UNVERSIONED,
+			    status == GOT_STATUS_UNVERSIONED, 0,
 			    &update_timestamps,
 			    repo, progress_cb, progress_arg);
 		}
@@ -2966,7 +2967,7 @@ add_file(struct got_worktree *worktree, struct got_fil
 		err = install_blob(worktree, ondisk_path, path2,
 		    mode2, GOT_DEFAULT_FILE_MODE, blob2,
 		    restoring_missing_file, reverting_versioned_file, 0,
-		    path_is_unversioned, NULL, repo,
+		    path_is_unversioned, 1, NULL, repo,
 		    progress_cb, progress_arg);
 	}
 	if (err)
@@ -5365,7 +5366,7 @@ revert_file(void *arg, unsigned char status, unsigned 
 				    ie->path,
 				    te ? te->mode : GOT_DEFAULT_FILE_MODE,
 				    got_fileindex_perms_to_st(ie), blob,
-				    0, 1, 0, 0, NULL, a->repo,
+				    0, 1, 0, 0, 0, NULL, a->repo,
 				    a->progress_cb, a->progress_arg);
 				if (err != NULL)
 					goto done;
@@ -5416,7 +5417,7 @@ revert_file(void *arg, unsigned char status, unsigned 
 				    ie->path,
 				    te ? te->mode : GOT_DEFAULT_FILE_MODE,
 				    got_fileindex_perms_to_st(ie), blob,
-				    0, 1, 0, 0, NULL, a->repo,
+				    0, 1, 0, 0, 0, NULL, a->repo,
 				    a->progress_cb, a->progress_arg);
 			}
 			if (err)
blob - 26b2c3e0885bfb34a45095898ff615b88dd7db10
blob + 05ee4bf13161b6e7c469839cb4978119bc2875da
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2994,6 +2994,65 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
+test_histedit_added_file() {
+	local testroot=`test_init histedit_added_file`
+	local orig_commit=`git_show_head $testroot/repo`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got checkout failed unexpectedly"
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo new > $testroot/wt/new
+	(cd $testroot/wt && got add new > /dev/null)
+
+	(cd $testroot/wt && got commit -m 'add new file' \
+		> $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+	local commit2=`git_show_head $testroot/repo`
+
+	(cd $testroot/wt && got up -q -c $orig_commit > /dev/null)
+
+	# Place an unversioned file into the work tree where the new
+	# file was added. This triggered a bug where histedit would
+	# assume the file was already present in earlier commits and then
+	# treat the commit which actually adds this file as a no-op change.
+	# This test then failed with:
+	# -2f81c7988242 -> c53428a3640e: add new file
+	# +2f81c7988242 -> no-op change: add new file
+	echo new > $testroot/wt/new
+
+	(cd $testroot/wt && got histedit -f > $testroot/stdout)
+	local new_commit=`git_show_head $testroot/repo`
+
+	local short_commit2=`trim_obj_id 12 $commit2`
+	local short_new_commit=`trim_obj_id 12 $new_commit`
+
+	echo "A  new" > $testroot/stdout.expected
+	echo "$short_commit2 -> $short_new_commit: add new file" \
+		>> $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
+
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -3024,3 +3083,4 @@ run_test test_histedit_drop_only
 run_test test_histedit_conflict_revert
 run_test test_histedit_no_eof_newline
 run_test test_histedit_preserve_bad_link
+run_test test_histedit_added_file