mirror of
https://github.com/git/git
synced 2024-08-26 19:19:18 +00:00
fetch: use new branch_checked_out() and add tests
When fetching refs from a remote, it is possible that the refspec will cause use to overwrite a ref that is checked out in a worktree. The existing logic in builtin/fetch.c uses a possibly-slow mechanism. Update those sections to use the new, more efficient branch_checked_out() helper. These uses were not previously tested, so add a test case that can be used for these kinds of collisions. There is only one test now, but more tests will be added as other consumers of branch_checked_out() are added. Note that there are two uses in builtin/fetch.c, but only one of the messages is tested. This is because the tested check is run before completing the fetch, and the untested check is not reachable without concurrent updates to the filesystem. Thus, it is beneficial to keep that extra check for the sake of defense-in-depth. However, we should not attempt to test the check, as the effort required is too complicated to be worth the effort. This use in update_local_ref() also requires a change in the error message because we no longer have access to the worktree struct, only the path of the worktree. This error is so rare that making a distinction between the two is not critical. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
d2ba271aad
commit
12d47e3b1f
|
@ -885,7 +885,6 @@ static int update_local_ref(struct ref *ref,
|
|||
struct worktree **worktrees)
|
||||
{
|
||||
struct commit *current = NULL, *updated;
|
||||
const struct worktree *wt;
|
||||
const char *pretty_ref = prettify_refname(ref->name);
|
||||
int fast_forward = 0;
|
||||
|
||||
|
@ -900,16 +899,14 @@ static int update_local_ref(struct ref *ref,
|
|||
}
|
||||
|
||||
if (!update_head_ok &&
|
||||
(wt = find_shared_symref(worktrees, "HEAD", ref->name)) &&
|
||||
!wt->is_bare && !is_null_oid(&ref->old_oid)) {
|
||||
!is_null_oid(&ref->old_oid) &&
|
||||
branch_checked_out(ref->name)) {
|
||||
/*
|
||||
* If this is the head, and it's not okay to update
|
||||
* the head, and the old value of the head isn't empty...
|
||||
*/
|
||||
format_display(display, '!', _("[rejected]"),
|
||||
wt->is_current ?
|
||||
_("can't fetch in current branch") :
|
||||
_("checked out in another worktree"),
|
||||
_("can't fetch into checked-out branch"),
|
||||
remote, pretty_ref, summary_width);
|
||||
return 1;
|
||||
}
|
||||
|
@ -1434,19 +1431,16 @@ static int prune_refs(struct refspec *rs,
|
|||
return result;
|
||||
}
|
||||
|
||||
static void check_not_current_branch(struct ref *ref_map,
|
||||
struct worktree **worktrees)
|
||||
static void check_not_current_branch(struct ref *ref_map)
|
||||
{
|
||||
const struct worktree *wt;
|
||||
const char *path;
|
||||
for (; ref_map; ref_map = ref_map->next)
|
||||
if (ref_map->peer_ref &&
|
||||
starts_with(ref_map->peer_ref->name, "refs/heads/") &&
|
||||
(wt = find_shared_symref(worktrees, "HEAD",
|
||||
ref_map->peer_ref->name)) &&
|
||||
!wt->is_bare)
|
||||
(path = branch_checked_out(ref_map->peer_ref->name)))
|
||||
die(_("refusing to fetch into branch '%s' "
|
||||
"checked out at '%s'"),
|
||||
ref_map->peer_ref->name, wt->path);
|
||||
ref_map->peer_ref->name, path);
|
||||
}
|
||||
|
||||
static int truncate_fetch_head(void)
|
||||
|
@ -1650,7 +1644,7 @@ static int do_fetch(struct transport *transport,
|
|||
ref_map = get_ref_map(transport->remote, remote_refs, rs,
|
||||
tags, &autotags);
|
||||
if (!update_head_ok)
|
||||
check_not_current_branch(ref_map, worktrees);
|
||||
check_not_current_branch(ref_map);
|
||||
|
||||
retcode = open_fetch_head(&fetch_head);
|
||||
if (retcode)
|
||||
|
|
|
@ -15,6 +15,21 @@ test_expect_success 'setup' '
|
|||
test_commit $i &&
|
||||
git branch wt-$i &&
|
||||
git worktree add wt-$i wt-$i || return 1
|
||||
done &&
|
||||
|
||||
# Create a server that updates each branch by one commit
|
||||
git init server &&
|
||||
test_commit -C server initial &&
|
||||
git remote add server ./server &&
|
||||
for i in 1 2 3 4
|
||||
do
|
||||
git -C server checkout -b wt-$i &&
|
||||
test_commit -C server A-$i || return 1
|
||||
done &&
|
||||
for i in 1 2
|
||||
do
|
||||
git -C server checkout -b fake-$i &&
|
||||
test_commit -C server f-$i || return 1
|
||||
done
|
||||
'
|
||||
|
||||
|
@ -48,4 +63,36 @@ test_expect_success 'refuse to overwrite: worktree in rebase' '
|
|||
grep "cannot force update the branch '\''fake-1'\'' checked out at.*wt-3" err
|
||||
'
|
||||
|
||||
test_expect_success !SANITIZE_LEAK 'refuse to fetch over ref: checked out' '
|
||||
test_must_fail git fetch server +refs/heads/wt-3:refs/heads/wt-3 2>err &&
|
||||
grep "refusing to fetch into branch '\''refs/heads/wt-3'\''" err &&
|
||||
|
||||
# General fetch into refs/heads/ will fail on first ref,
|
||||
# so use a generic error message check.
|
||||
test_must_fail git fetch server +refs/heads/*:refs/heads/* 2>err &&
|
||||
grep "refusing to fetch into branch" err
|
||||
'
|
||||
|
||||
test_expect_success !SANITIZE_LEAK 'refuse to fetch over ref: worktree in bisect' '
|
||||
test_when_finished rm -rf .git/worktrees/wt-*/BISECT_* &&
|
||||
|
||||
touch .git/worktrees/wt-4/BISECT_LOG &&
|
||||
echo refs/heads/fake-2 >.git/worktrees/wt-4/BISECT_START &&
|
||||
|
||||
test_must_fail git fetch server +refs/heads/fake-2:refs/heads/fake-2 2>err &&
|
||||
grep "refusing to fetch into branch" err
|
||||
'
|
||||
|
||||
test_expect_success !SANITIZE_LEAK 'refuse to fetch over ref: worktree in rebase' '
|
||||
test_when_finished rm -rf .git/worktrees/wt-*/rebase-merge &&
|
||||
|
||||
mkdir -p .git/worktrees/wt-4/rebase-merge &&
|
||||
touch .git/worktrees/wt-4/rebase-merge/interactive &&
|
||||
echo refs/heads/fake-1 >.git/worktrees/wt-4/rebase-merge/head-name &&
|
||||
echo refs/heads/fake-2 >.git/worktrees/wt-4/rebase-merge/onto &&
|
||||
|
||||
test_must_fail git fetch server +refs/heads/fake-1:refs/heads/fake-1 2>err &&
|
||||
grep "refusing to fetch into branch" err
|
||||
'
|
||||
|
||||
test_done
|
||||
|
|
Loading…
Reference in a new issue