Download raw body.
prevent 'got merge' from modifying arbitrary references
It is possible to switch the work tree to an arbitrary reference and then run: got merge some/other/reference This will cause a merge commit to be created, and the work tree's current reference will be changed to point at this merge commit. There is an awful trap. Imagine someone did this: got update -b origin/main got merge main This sequence matches the 'got rebase' workflow so it may seem like a reasonable thing to do. But the outcome of this sequence is that origin/main no longer matches the state last fetched by 'got fetch', which is very bad. I am adding regression tests below which cover this situation, and also test the more desirable sequence: got update -b main got merge origin/main Limiting 'got merge' to first parents in the "refs/heads/" namespace is a solution that aligns with the restrictions imposed by 'got commit'. I don't believe this change negatively impacts any workflows that we should be supporting -- is this correct? ok? ----------------------------------------------- prevent 'got merge' from creating commits on branches outside of "refs/heads/" diff b88936d3f94e26ab32d9ef5d893b39fe633c6485 73097a92238a552f665580cf41942cda976a0a81 commit - b88936d3f94e26ab32d9ef5d893b39fe633c6485 commit + 73097a92238a552f665580cf41942cda976a0a81 blob - 8122fe9273cc37091dc13c83016ef03a2a436be5 blob + b6da5695eaab874ad02b4ba5ad0801b9748b60fb --- got/got.1 +++ got/got.1 @@ -2837,7 +2837,10 @@ The tip commit of the work tree's current branch, whic .Cm got import . .Pp Merge commits are commits based on multiple parent commits. -The tip commit of the work tree's current branch, which must be set with +The tip commit of the work tree's current branch, which must be +must be in the +.Dq refs/heads/ +reference namespace and must be set with .Cm got update -b before starting the .Cm merge @@ -2882,6 +2885,13 @@ If the work tree is not yet fully updated to the tip c .Pp .Cm got merge will refuse to run if certain preconditions are not met. +If the work tree's current branch is not in the +.Dq refs/heads/ +reference namespace then the work tree must first be switched to a +branch in the +.Dq refs/heads/ +namespace with +.Cm got update -b . If the work tree is not yet fully updated to the tip commit of its branch, then the work tree must first be updated with .Cm got update . blob - ea8a08baed68affba73cafa577c3125e9756466a blob + 4c6ca7ff2f76eaa431b055ab0a6912772b492b38 --- got/got.c +++ got/got.c @@ -13264,6 +13264,16 @@ cmd_merge(int argc, char *argv[]) goto done; /* nothing else to do */ } + if (strncmp(got_worktree_get_head_ref_name(worktree), + "refs/heads/", 11) != 0) { + error = got_error_fmt(GOT_ERR_COMMIT_BRANCH, + "work tree's current branch %s is outside the " + "\"refs/heads/\" reference namespace; " + "update -b required", + got_worktree_get_head_ref_name(worktree)); + goto done; + } + error = get_author(&author, repo, worktree); if (error) goto done; blob - bdd7571df06f3c7d45127433eabfcdc76e0e94ed blob + 55823c98f07c65faeafacd31ad7723d7cbae0af1 --- regress/cmdline/fetch.sh +++ regress/cmdline/fetch.sh @@ -1935,6 +1935,171 @@ test_parseargs "$@" } +test_fetch_branch_and_merge() { + local testroot=`test_init fetch_branch_and_merge` + local testurl=ssh://127.0.0.1/$testroot + local commit_id=`git_show_head $testroot/repo` + + got clone -q $testurl/repo $testroot/repo-clone + ret=$? + if [ $ret -ne 0 ]; then + echo "got clone command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/repo/alpha + git_commit $testroot/repo -m "modified alpha" + local commit_id2=`git_show_head $testroot/repo` + + got fetch -q -r $testroot/repo-clone > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + echo "got fetch command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got ref -l -r $testroot/repo-clone > $testroot/stdout + + echo "HEAD: refs/heads/master" > $testroot/stdout.expected + echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected + echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \ + >> $testroot/stdout.expected + echo "refs/remotes/origin/master: $commit_id2" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got checkout $testroot/repo-clone $testroot/wt > /dev/null + + echo "modified beta" > $testroot/wt/beta + (cd $testroot/wt && got commit -m "modified beta" > /dev/null) + local commit_id3=`git_show_head $testroot/repo-clone` + + (cd $testroot/wt && got update > /dev/null) + (cd $testroot/wt && got merge origin/master > $testroot/stdout) + local merge_commit_id=`git_show_head $testroot/repo-clone` + + cat > $testroot/stdout.expected <<EOF +G alpha +Merged refs/remotes/origin/master into refs/heads/master: $merge_commit_id +EOF + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + test_done "$testroot" "$ret" +} + +test_fetch_branch_and_merge_remote() { + local testroot=`test_init fetch_branch_and_merge` + local testurl=ssh://127.0.0.1/$testroot + local commit_id=`git_show_head $testroot/repo` + + got clone -q $testurl/repo $testroot/repo-clone + ret=$? + if [ $ret -ne 0 ]; then + echo "got clone command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/repo/alpha + git_commit $testroot/repo -m "modified alpha" + local commit_id2=`git_show_head $testroot/repo` + + got fetch -q -r $testroot/repo-clone > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + echo "got fetch command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got ref -l -r $testroot/repo-clone > $testroot/stdout + + echo "HEAD: refs/heads/master" > $testroot/stdout.expected + echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected + echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \ + >> $testroot/stdout.expected + echo "refs/remotes/origin/master: $commit_id2" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got checkout $testroot/repo-clone $testroot/wt > /dev/null + + echo "modified beta" > $testroot/wt/beta + (cd $testroot/wt && got commit -m "modified beta" > /dev/null) + local commit_id3=`git_show_head $testroot/repo-clone` + + (cd $testroot/wt && got update -b origin/master > /dev/null) + (cd $testroot/wt && got merge master > \ + $testroot/stdout 2> $testroot/stderr) + local merge_commit_id=`git_show_head $testroot/repo-clone` + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + echo -n "got: work tree's current branch refs/remotes/origin/master " \ + > $testroot/stderr.expected + echo -n 'is outside the "refs/heads/" reference namespace; ' \ + >> $testroot/stderr.expected + echo -n "update -b required: will not commit to a branch " \ + >> $testroot/stderr.expected + echo 'outside the "refs/heads/" reference namespace' \ + >> $testroot/stderr.expected + + cmp -s $testroot/stderr $testroot/stderr.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr + fi + test_done "$testroot" "$ret" +} + test_parseargs "$@" run_test test_fetch_basic run_test test_fetch_list @@ -1952,3 +2117,5 @@ run_test test_fetch_from_out_of_date_remote run_test test_fetch_delete_remote_refs run_test test_fetch_honor_wt_conf_bflag run_test test_fetch_from_out_of_date_remote +run_test test_fetch_branch_and_merge +run_test test_fetch_branch_and_merge_remote
prevent 'got merge' from modifying arbitrary references