Skip to content

Commit 4e40fb3

Browse files
gitstergitster
committed
Merge branch 'mh/ref-locking-fix'
Transactions to update multiple references that involves a deletion was quite broken in an error codepath and did not abort everything correctly. * mh/ref-locking-fix: files_transaction_prepare(): fix handling of ref lock failure t1404: add a bunch of tests of D/F conflicts
2 parents ba78f39 + da5267f commit 4e40fb3

File tree

2 files changed

+142
-1
lines changed

2 files changed

+142
-1
lines changed

‎refs/files-backend.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2570,7 +2570,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
25702570
ret = lock_ref_for_update(refs, update, transaction,
25712571
head_ref, &affected_refnames, err);
25722572
if (ret)
2573-
break;
2573+
goto cleanup;
25742574

25752575
if (update->flags & REF_DELETING &&
25762576
!(update->flags & REF_LOG_ONLY) &&

‎t/t1404-update-ref-errors.sh

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,81 @@ test_update_rejected () {
3434

3535
Q="'"
3636

37+
# Test adding and deleting D/F-conflicting references in a single
38+
# transaction.
39+
df_test() {
40+
prefix="$1"
41+
pack=: symadd=false symdel=false add_del=false addref= delref=
42+
shift
43+
while test $# -gt 0
44+
do
45+
case "$1" in
46+
--pack)
47+
pack="git pack-refs --all"
48+
shift
49+
;;
50+
--sym-add)
51+
# Perform the add via a symbolic reference
52+
symadd=true
53+
shift
54+
;;
55+
--sym-del)
56+
# Perform the del via a symbolic reference
57+
symdel=true
58+
shift
59+
;;
60+
--del-add)
61+
# Delete first reference then add second
62+
add_del=false
63+
delref="$prefix/r/$2"
64+
addref="$prefix/r/$3"
65+
shift 3
66+
;;
67+
--add-del)
68+
# Add first reference then delete second
69+
add_del=true
70+
addref="$prefix/r/$2"
71+
delref="$prefix/r/$3"
72+
shift 3
73+
;;
74+
*)
75+
echo 1>&2 "Extra args to df_test: $*"
76+
return 1
77+
;;
78+
esac
79+
done
80+
git update-ref "$delref" $C &&
81+
if $symadd
82+
then
83+
addname="$prefix/s/symadd" &&
84+
git symbolic-ref "$addname" "$addref"
85+
else
86+
addname="$addref"
87+
fi &&
88+
if $symdel
89+
then
90+
delname="$prefix/s/symdel" &&
91+
git symbolic-ref "$delname" "$delref"
92+
else
93+
delname="$delref"
94+
fi &&
95+
cat >expected-err <<-EOF &&
96+
fatal: cannot lock ref $Q$addname$Q: $Q$delref$Q exists; cannot create $Q$addref$Q
97+
EOF
98+
$pack &&
99+
if $add_del
100+
then
101+
printf "%s\n" "create $addname $D" "delete $delname"
102+
else
103+
printf "%s\n" "delete $delname" "create $addname $D"
104+
fi >commands &&
105+
test_must_fail git update-ref --stdin <commands 2>output.err &&
106+
test_cmp expected-err output.err &&
107+
printf "%s\n" "$C $delref" >expected-refs &&
108+
git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
109+
test_cmp expected-refs actual-refs
110+
}
111+
37112
test_expect_success 'setup' '
38113
39114
git commit --allow-empty -m Initial &&
@@ -188,6 +263,72 @@ test_expect_success 'empty directory should not fool 1-arg delete' '
188263
git update-ref --stdin
189264
'
190265

266+
test_expect_success 'D/F conflict prevents add long + delete short' '
267+
df_test refs/df-al-ds --add-del foo/bar foo
268+
'
269+
270+
test_expect_success 'D/F conflict prevents add short + delete long' '
271+
df_test refs/df-as-dl --add-del foo foo/bar
272+
'
273+
274+
test_expect_success 'D/F conflict prevents delete long + add short' '
275+
df_test refs/df-dl-as --del-add foo/bar foo
276+
'
277+
278+
test_expect_success 'D/F conflict prevents delete short + add long' '
279+
df_test refs/df-ds-al --del-add foo foo/bar
280+
'
281+
282+
test_expect_success 'D/F conflict prevents add long + delete short packed' '
283+
df_test refs/df-al-dsp --pack --add-del foo/bar foo
284+
'
285+
286+
test_expect_success 'D/F conflict prevents add short + delete long packed' '
287+
df_test refs/df-as-dlp --pack --add-del foo foo/bar
288+
'
289+
290+
test_expect_success 'D/F conflict prevents delete long packed + add short' '
291+
df_test refs/df-dlp-as --pack --del-add foo/bar foo
292+
'
293+
294+
test_expect_success 'D/F conflict prevents delete short packed + add long' '
295+
df_test refs/df-dsp-al --pack --del-add foo foo/bar
296+
'
297+
298+
# Try some combinations involving symbolic refs...
299+
300+
test_expect_success 'D/F conflict prevents indirect add long + delete short' '
301+
df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
302+
'
303+
304+
test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '
305+
df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
306+
'
307+
308+
test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '
309+
df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
310+
'
311+
312+
test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '
313+
df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
314+
'
315+
316+
test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '
317+
df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
318+
'
319+
320+
test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '
321+
df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
322+
'
323+
324+
test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '
325+
df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
326+
'
327+
328+
test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '
329+
df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
330+
'
331+
191332
# Test various errors when reading the old values of references...
192333

193334
test_expect_success 'missing old value blocks update' '

0 commit comments

Comments
 (0)