commit 6b5246e4d4f6c6cbd079a43d14042d998fbc902f from: James Cook via: Stefan Sperling date: Mon Jun 05 08:14:56 2023 UTC make 'got merge -c' fail even if new changes only affect unrelated paths Otherwise, 'got merge -c' can silently revert already committed changes. Also fix GOT_ERR_MERGE_COMMIT_OUT_OF_DATE by giving it a value distinct from GOT_ERR_MERGE_STAGED_PATHS. Patch by James Cook commit - 427615296bc79cda685540d7fc241444b7447980 commit + 6b5246e4d4f6c6cbd079a43d14042d998fbc902f blob - f4666b0aa2b87365b32db745d93c6feae807d6a4 blob + d853c6a2eec0e5067f684d2bdf43979a898c26bb --- got/got.c +++ got/got.c @@ -13265,10 +13265,24 @@ cmd_merge(int argc, char *argv[]) } error = get_author(&author, repo, worktree); + if (error) + goto done; + + error = got_ref_open(&wt_branch, repo, + got_worktree_get_head_ref_name(worktree), 0); + if (error) + goto done; + error = got_ref_resolve(&wt_branch_tip, repo, wt_branch); if (error) goto done; if (continue_merge) { + struct got_object_id *base_commit_id; + base_commit_id = got_worktree_get_base_commit_id(worktree); + if (got_object_id_cmp(wt_branch_tip, base_commit_id) != 0) { + error = got_error(GOT_ERR_MERGE_COMMIT_OUT_OF_DATE); + goto done; + } error = got_worktree_merge_continue(&branch_name, &branch_tip, &fileindex, worktree, repo); if (error) @@ -13287,13 +13301,6 @@ cmd_merge(int argc, char *argv[]) goto done; } - error = got_ref_open(&wt_branch, repo, - got_worktree_get_head_ref_name(worktree), 0); - if (error) - goto done; - error = got_ref_resolve(&wt_branch_tip, repo, wt_branch); - if (error) - goto done; error = got_commit_graph_find_youngest_common_ancestor(&yca_id, wt_branch_tip, branch_tip, 0, repo, check_cancelled, NULL); blob - 3b047e438a7e4fff23c4e9f6c1ada623765a80dc blob + 0582e35816efd0e75172f961b9d274e6c249d83d --- include/got_error.h +++ include/got_error.h @@ -158,7 +158,6 @@ #define GOT_ERR_NOT_MERGING 141 #define GOT_ERR_MERGE_OUT_OF_DATE 142 #define GOT_ERR_MERGE_STAGED_PATHS 143 -#define GOT_ERR_MERGE_COMMIT_OUT_OF_DATE 143 #define GOT_ERR_MERGE_BUSY 144 #define GOT_ERR_MERGE_PATH 145 #define GOT_ERR_FILE_BINARY 146 @@ -185,6 +184,7 @@ #define GOT_ERR_UID 167 #define GOT_ERR_GID 168 #define GOT_ERR_NO_PROG 169 +#define GOT_ERR_MERGE_COMMIT_OUT_OF_DATE 170 struct got_error { int code; blob - c3b2bdd9b1622256235092145718299216397766 blob + feebeac691a9e648011aa46d28905422aae9db1a --- lib/error.c +++ lib/error.c @@ -203,9 +203,6 @@ static const struct got_error got_errors[] = { { GOT_ERR_MERGE_STAGED_PATHS, "work tree contains files with staged " "changes; these changes must be unstaged before merging can " "proceed" }, - { GOT_ERR_MERGE_COMMIT_OUT_OF_DATE, "merging cannot proceed because " - "the work tree is no longer up-to-date; merge must be aborted " - "and retried" }, { GOT_ERR_MERGE_BUSY,"a merge operation is in progress in this " "work tree and must be continued or aborted first" }, { GOT_ERR_MERGE_PATH, "cannot merge branch which contains " @@ -235,6 +232,9 @@ static const struct got_error got_errors[] = { { GOT_ERR_UID, "bad user ID" }, { GOT_ERR_GID, "bad group ID" }, { GOT_ERR_NO_PROG, "command not found or not accessible" }, + { GOT_ERR_MERGE_COMMIT_OUT_OF_DATE, "merging cannot proceed because " + "the work tree is no longer up-to-date; merge must be aborted " + "and retried" }, }; static struct got_custom_error { blob - 9d45bcc89fba03349491b41c260b4fc0c9606531 blob + e97f10e73cd0df0521ce4e36b40897121837e8d5 --- lib/worktree.c +++ lib/worktree.c @@ -8408,20 +8408,6 @@ got_worktree_merge_commit(struct got_object_id **new_c collect_commitables, &cc_arg, NULL, NULL, 1, 0); if (err) goto done; - - TAILQ_FOREACH(pe, &commitable_paths, entry) { - struct got_commitable *ct = pe->data; - const char *ct_path = ct->in_repo_path; - - while (ct_path[0] == '/') - ct_path++; - err = check_out_of_date(ct_path, ct->status, - ct->staged_status, ct->base_blob_id, ct->base_commit_id, - head_commit_id, repo, GOT_ERR_MERGE_COMMIT_OUT_OF_DATE); - if (err) - goto done; - - } mcm_arg.worktree = worktree; mcm_arg.branch_name = branch_name; blob - df6f2f9edf54a23ff9b1e9f05267a77673a4fbcf blob + ca27e157f924c0abac516cda4f26581c04e3cc21 --- regress/cmdline/merge.sh +++ regress/cmdline/merge.sh @@ -582,10 +582,71 @@ test_merge_continue() { (cd $testroot/wt && got log -l1 | grep ^parent > $testroot/stdout) echo "parent 1: $master_commit" > $testroot/stdout.expected echo "parent 2: $branch_commit3" >> $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + test_done "$testroot" "$ret" +} + +test_merge_continue_new_commit() { + # "got merge -c" should refuse to run if the current branch tip has + # changed since the merge was started, to avoid clobbering the changes. + local testroot=`test_init merge_continue_new_commit` + + (cd $testroot/repo && git checkout -q -b newbranch) + echo "modified delta on branch" > $testroot/repo/gamma/delta + git_commit $testroot/repo -m "committing to delta on newbranch" + + (cd $testroot/repo && git checkout -q master) + echo "modified alpha on master" > $testroot/repo/alpha + git_commit $testroot/repo -m "committing to alpha on master" + + got checkout -b master $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + echo "got checkout failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got merge -n newbranch >/dev/null) + ret=$? + if [ $ret -ne 0 ]; then + echo "got merge failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha again on master" > $testroot/repo/alpha + git_commit $testroot/repo -m "committing to alpha on master again" + + (cd $testroot/wt && got merge -c > $testroot/stdout 2> $testroot/stderr) + ret=$? + if [ $ret -eq 0 ]; then + echo "got merge succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + + 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 + + echo -n "got: merging cannot proceed because the work tree is no " \ + > $testroot/stderr.expected + echo "longer up-to-date; merge must be aborted and retried" \ + >> $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr fi test_done "$testroot" "$ret" } @@ -1506,6 +1567,7 @@ test_merge_gitconfig_author() { test_parseargs "$@" run_test test_merge_basic run_test test_merge_continue +run_test test_merge_continue_new_commit run_test test_merge_abort run_test test_merge_in_progress run_test test_merge_path_prefix