Re: [PATCH 3/4] fetch: use new branch_checked_out() and add tests
Hi Stolee
On 08/06/2022 21:09, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@github.com>
> [...]
Show 31 quoted lines
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index ac29c2b1ae3..1ba56240312 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -885,7 +885,7 @@ static int update_local_ref(struct ref *ref,
> struct worktree **worktrees)
> {
> struct commit *current = NULL, *updated;
> - const struct worktree *wt;
> + char *path = NULL;
> const char *pretty_ref = prettify_refname(ref->name);
> int fast_forward = 0;
>
> @@ -900,17 +900,17 @@ 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, &path)) {
> /*
> * 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"),
> + path ? _("can't fetch in current branch") :
> + _("checked out in another worktree"),I'm confused by this, isn't path always non-null?
Best Wishes
Phillip
Show 99 quoted lines
> remote, pretty_ref, summary_width);
> + free(path);
> return 1;
> }
>
> @@ -1434,19 +1434,16 @@ cleanup:
> 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;
> + 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)
> + branch_checked_out(ref_map->peer_ref->name, &path))
> 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 +1647,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)
> diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
> index 12faca7f655..f3f8b0b2b79 100755
> --- a/t/t2407-worktree-heads.sh
> +++ b/t/t2407-worktree-heads.sh
> @@ -10,6 +10,15 @@ 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 clone . server &&
> + git remote add server ./server &&
> + for i in 1 2 3 4
> + do
> + git -C server checkout wt-$i &&
> + test_commit -C server A-$i || return 1
> done
> '
>
> @@ -21,6 +30,16 @@ test_expect_success 'refuse to overwrite: checked out in worktree' '
> done
> '
>
> +test_expect_success 'refuse to overwrite during fetch' '
> + 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 'refuse to overwrite: worktree in bisect' '
> test_when_finished test_might_fail git -C wt-4 bisect reset &&
>
> @@ -31,7 +50,10 @@ test_expect_success 'refuse to overwrite: worktree in bisect' '
> ) &&
>
> test_must_fail git branch -f wt-4 HEAD 2>err &&
> - grep "cannot force update the branch '\''wt-4'\'' checked out at" err
> + grep "cannot force update the branch '\''wt-4'\'' checked out at" err &&
> +
> + test_must_fail git fetch server +refs/heads/wt-4:refs/heads/wt-4 2>err &&
> + grep "refusing to fetch into branch '\''refs/heads/wt-4'\''" err
> '
>
> . "$TEST_DIRECTORY"/lib-rebase.sh
> @@ -47,7 +69,10 @@ test_expect_success 'refuse to overwrite: worktree in rebase' '
> ) &&
>
> test_must_fail git branch -f wt-4 HEAD 2>err &&
> - grep "cannot force update the branch '\''wt-4'\'' checked out at" err
> + grep "cannot force update the branch '\''wt-4'\'' checked out at" err &&
> +
> + test_must_fail git fetch server +refs/heads/wt-4:refs/heads/wt-4 2>err &&
> + grep "refusing to fetch into branch '\''refs/heads/wt-4'\''" err
> '
>
> test_done