Skip to content

Commit c7ce913

Browse files
gitstergitster
committed
Merge branch 'ds/branch-checked-out' into seen
Introduce a helper to see if a branch is already being worked on (hence should not be newly checked out in a working tree), which performs much better than the existing find_shared_symref() to replace many uses of the latter. * ds/branch-checked-out: branch: fix branch_checked_out() leaks branch: use branch_checked_out() when deleting refs fetch: use new branch_checked_out() and add tests branch: check for bisects and rebases branch: add branch_checked_out() helper
2 parents d17cea5 + b5137cc commit c7ce913

File tree

5 files changed

+217
-26
lines changed

5 files changed

+217
-26
lines changed

‎branch.c

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "worktree.h"
1111
#include "submodule-config.h"
1212
#include "run-command.h"
13+
#include "strmap.h"
1314

1415
struct tracking {
1516
struct refspec_item spec;
@@ -369,6 +370,76 @@ int validate_branchname(const char *name, struct strbuf *ref)
369370
return ref_exists(ref->buf);
370371
}
371372

373+
static int initialized_checked_out_branches;
374+
static struct strmap current_checked_out_branches = STRMAP_INIT;
375+
376+
static void prepare_checked_out_branches(void)
377+
{
378+
int i = 0;
379+
struct worktree **worktrees;
380+
381+
if (initialized_checked_out_branches)
382+
return;
383+
initialized_checked_out_branches = 1;
384+
385+
worktrees = get_worktrees();
386+
387+
while (worktrees[i]) {
388+
char *old;
389+
struct wt_status_state state = { 0 };
390+
struct worktree *wt = worktrees[i++];
391+
392+
if (wt->is_bare)
393+
continue;
394+
395+
if (wt->head_ref) {
396+
old = strmap_put(&current_checked_out_branches,
397+
wt->head_ref,
398+
xstrdup(wt->path));
399+
free(old);
400+
}
401+
402+
if (wt_status_check_rebase(wt, &state) &&
403+
(state.rebase_in_progress || state.rebase_interactive_in_progress) &&
404+
state.branch) {
405+
struct strbuf ref = STRBUF_INIT;
406+
strbuf_addf(&ref, "refs/heads/%s", state.branch);
407+
old = strmap_put(&current_checked_out_branches,
408+
ref.buf,
409+
xstrdup(wt->path));
410+
free(old);
411+
strbuf_release(&ref);
412+
}
413+
wt_status_state_free_buffers(&state);
414+
415+
if (wt_status_check_bisect(wt, &state) &&
416+
state.branch) {
417+
struct strbuf ref = STRBUF_INIT;
418+
strbuf_addf(&ref, "refs/heads/%s", state.branch);
419+
old = strmap_put(&current_checked_out_branches,
420+
ref.buf,
421+
xstrdup(wt->path));
422+
free(old);
423+
strbuf_release(&ref);
424+
}
425+
wt_status_state_free_buffers(&state);
426+
}
427+
428+
free_worktrees(worktrees);
429+
}
430+
431+
int branch_checked_out(const char *refname, char **path)
432+
{
433+
const char *path_in_set;
434+
prepare_checked_out_branches();
435+
436+
path_in_set = strmap_get(&current_checked_out_branches, refname);
437+
if (path_in_set && path)
438+
*path = xstrdup(path_in_set);
439+
440+
return !!path_in_set;
441+
}
442+
372443
/*
373444
* Check if a branch 'name' can be created as a new branch; die otherwise.
374445
* 'force' can be used when it is OK for the named branch already exists.
@@ -377,23 +448,18 @@ int validate_branchname(const char *name, struct strbuf *ref)
377448
*/
378449
int validate_new_branchname(const char *name, struct strbuf *ref, int force)
379450
{
380-
struct worktree **worktrees;
381-
const struct worktree *wt;
382-
451+
char *path;
383452
if (!validate_branchname(name, ref))
384453
return 0;
385454

386455
if (!force)
387456
die(_("a branch named '%s' already exists"),
388457
ref->buf + strlen("refs/heads/"));
389458

390-
worktrees = get_worktrees();
391-
wt = find_shared_symref(worktrees, "HEAD", ref->buf);
392-
if (wt && !wt->is_bare)
459+
if (branch_checked_out(ref->buf, &path))
393460
die(_("cannot force update the branch '%s' "
394461
"checked out at '%s'"),
395-
ref->buf + strlen("refs/heads/"), wt->path);
396-
free_worktrees(worktrees);
462+
ref->buf + strlen("refs/heads/"), path);
397463

398464
return 1;
399465
}

‎branch.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ void create_branches_recursively(struct repository *r, const char *name,
101101
const char *tracking_name, int force,
102102
int reflog, int quiet, enum branch_track track,
103103
int dry_run);
104+
105+
/*
106+
* Returns true if the branch at 'refname' is checked out at any
107+
* non-bare worktree. The path of the worktree is stored in the
108+
* given 'path', if provided.
109+
*/
110+
int branch_checked_out(const char *refname, char **path);
111+
104112
/*
105113
* Check if 'name' can be a valid name for a branch; die otherwise.
106114
* Return 1 if the named branch already exists; return 0 otherwise.

‎builtin/branch.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,12 +253,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
253253
name = mkpathdup(fmt, bname.buf);
254254

255255
if (kinds == FILTER_REFS_BRANCHES) {
256-
const struct worktree *wt =
257-
find_shared_symref(worktrees, "HEAD", name);
258-
if (wt) {
256+
char *path;
257+
if (branch_checked_out(name, &path)) {
259258
error(_("Cannot delete branch '%s' "
260259
"checked out at '%s'"),
261-
bname.buf, wt->path);
260+
bname.buf, path);
261+
free(path);
262262
ret = 1;
263263
continue;
264264
}

‎builtin/fetch.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,7 @@ static int update_local_ref(struct ref *ref,
890890
struct worktree **worktrees)
891891
{
892892
struct commit *current = NULL, *updated;
893-
const struct worktree *wt;
893+
char *path = NULL;
894894
const char *pretty_ref = prettify_refname(ref->name);
895895
int fast_forward = 0;
896896

@@ -905,17 +905,17 @@ static int update_local_ref(struct ref *ref,
905905
}
906906

907907
if (!update_head_ok &&
908-
(wt = find_shared_symref(worktrees, "HEAD", ref->name)) &&
909-
!wt->is_bare && !is_null_oid(&ref->old_oid)) {
908+
!is_null_oid(&ref->old_oid) &&
909+
branch_checked_out(ref->name, &path)) {
910910
/*
911911
* If this is the head, and it's not okay to update
912912
* the head, and the old value of the head isn't empty...
913913
*/
914914
format_display(display, '!', _("[rejected]"),
915-
wt->is_current ?
916-
_("can't fetch in current branch") :
917-
_("checked out in another worktree"),
915+
path ? _("can't fetch in current branch") :
916+
_("checked out in another worktree"),
918917
remote, pretty_ref, summary_width);
918+
free(path);
919919
return 1;
920920
}
921921

@@ -1439,19 +1439,16 @@ static int prune_refs(struct refspec *rs,
14391439
return result;
14401440
}
14411441

1442-
static void check_not_current_branch(struct ref *ref_map,
1443-
struct worktree **worktrees)
1442+
static void check_not_current_branch(struct ref *ref_map)
14441443
{
1445-
const struct worktree *wt;
1444+
char *path;
14461445
for (; ref_map; ref_map = ref_map->next)
14471446
if (ref_map->peer_ref &&
14481447
starts_with(ref_map->peer_ref->name, "refs/heads/") &&
1449-
(wt = find_shared_symref(worktrees, "HEAD",
1450-
ref_map->peer_ref->name)) &&
1451-
!wt->is_bare)
1448+
branch_checked_out(ref_map->peer_ref->name, &path))
14521449
die(_("refusing to fetch into branch '%s' "
14531450
"checked out at '%s'"),
1454-
ref_map->peer_ref->name, wt->path);
1451+
ref_map->peer_ref->name, path);
14551452
}
14561453

14571454
static int truncate_fetch_head(void)
@@ -1655,7 +1652,7 @@ static int do_fetch(struct transport *transport,
16551652
ref_map = get_ref_map(transport->remote, remote_refs, rs,
16561653
tags, &autotags);
16571654
if (!update_head_ok)
1658-
check_not_current_branch(ref_map, worktrees);
1655+
check_not_current_branch(ref_map);
16591656

16601657
retcode = open_fetch_head(&fetch_head);
16611658
if (retcode)

‎t/t2407-worktree-heads.sh

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
#!/bin/sh
2+
3+
test_description='test operations trying to overwrite refs at worktree HEAD'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success 'setup' '
8+
for i in 1 2 3 4
9+
do
10+
test_commit $i &&
11+
git branch wt-$i &&
12+
git worktree add wt-$i wt-$i || return 1
13+
done &&
14+
15+
# Create a server that updates each branch by one commit
16+
git clone . server &&
17+
git remote add server ./server &&
18+
for i in 1 2 3 4
19+
do
20+
git -C server checkout wt-$i &&
21+
test_commit -C server A-$i || return 1
22+
done
23+
'
24+
25+
test_expect_success 'refuse to overwrite: checked out in worktree' '
26+
for i in 1 2 3 4
27+
do
28+
test_must_fail git branch -f wt-$i HEAD 2>err
29+
grep "cannot force update the branch" err &&
30+
31+
test_must_fail git branch -D wt-$i 2>err
32+
grep "Cannot delete branch" err || return 1
33+
done
34+
'
35+
36+
test_expect_success 'refuse to overwrite during fetch' '
37+
test_must_fail git fetch server +refs/heads/wt-3:refs/heads/wt-3 2>err &&
38+
grep "refusing to fetch into branch '\''refs/heads/wt-3'\''" err &&
39+
40+
# General fetch into refs/heads/ will fail on first ref,
41+
# so use a generic error message check.
42+
test_must_fail git fetch server +refs/heads/*:refs/heads/* 2>err &&
43+
grep "refusing to fetch into branch" err
44+
'
45+
46+
test_expect_success 'refuse to overwrite: worktree in bisect' '
47+
test_when_finished test_might_fail git -C wt-4 bisect reset &&
48+
49+
(
50+
git -C wt-4 bisect start &&
51+
git -C wt-4 bisect bad HEAD &&
52+
git -C wt-4 bisect good HEAD~3
53+
) &&
54+
55+
test_must_fail git branch -f wt-4 HEAD 2>err &&
56+
grep "cannot force update the branch '\''wt-4'\'' checked out at" err &&
57+
58+
test_must_fail git fetch server +refs/heads/wt-4:refs/heads/wt-4 2>err &&
59+
grep "refusing to fetch into branch '\''refs/heads/wt-4'\''" err
60+
'
61+
62+
. "$TEST_DIRECTORY"/lib-rebase.sh
63+
64+
test_expect_success 'refuse to overwrite: worktree in rebase' '
65+
test_when_finished test_might_fail git -C wt-4 rebase --abort &&
66+
67+
(
68+
set_fake_editor &&
69+
FAKE_LINES="edit 1 2 3" \
70+
git -C wt-4 rebase -i HEAD~3 >rebase &&
71+
git -C wt-4 status
72+
) &&
73+
74+
test_must_fail git branch -f wt-4 HEAD 2>err &&
75+
grep "cannot force update the branch '\''wt-4'\'' checked out at" err &&
76+
77+
test_must_fail git fetch server +refs/heads/wt-4:refs/heads/wt-4 2>err &&
78+
grep "refusing to fetch into branch '\''refs/heads/wt-4'\''" err
79+
'
80+
81+
test_expect_success 'refuse to overwrite when in error states' '
82+
test_when_finished rm -rf .git/worktrees/wt-*/rebase-merge &&
83+
test_when_finished rm -rf .git/worktrees/wt-*/BISECT_* &&
84+
85+
git branch -f fake1 &&
86+
mkdir -p .git/worktrees/wt-3/rebase-merge &&
87+
touch .git/worktrees/wt-3/rebase-merge/interactive &&
88+
echo refs/heads/fake1 >.git/worktrees/wt-3/rebase-merge/head-name &&
89+
echo refs/heads/fake2 >.git/worktrees/wt-3/rebase-merge/onto &&
90+
91+
git branch -f fake2 &&
92+
touch .git/worktrees/wt-4/BISECT_LOG &&
93+
echo refs/heads/fake2 >.git/worktrees/wt-4/BISECT_START &&
94+
95+
# First, ensure we prevent writing when only one reason to fail.
96+
for i in 1 2
97+
do
98+
test_must_fail git branch -f fake$i HEAD 2>err &&
99+
grep "cannot force update the branch '\''fake$i'\'' checked out at" err ||
100+
return 1
101+
done &&
102+
103+
# Second, set up duplicate values.
104+
mkdir -p .git/worktrees/wt-4/rebase-merge &&
105+
touch .git/worktrees/wt-4/rebase-merge/interactive &&
106+
echo refs/heads/fake2 >.git/worktrees/wt-4/rebase-merge/head-name &&
107+
echo refs/heads/fake1 >.git/worktrees/wt-4/rebase-merge/onto &&
108+
109+
touch .git/worktrees/wt-1/BISECT_LOG &&
110+
echo refs/heads/fake1 >.git/worktrees/wt-1/BISECT_START &&
111+
112+
for i in 1 2
113+
do
114+
test_must_fail git branch -f fake$i HEAD 2>err &&
115+
grep "cannot force update the branch '\''fake$i'\'' checked out at" err ||
116+
return 1
117+
done
118+
'
119+
120+
test_done

0 commit comments

Comments
 (0)