branch -d: avoid repeated symref resolution
If a repository gets in a broken state with too much symref nesting, it cannot be repaired with "git branch -d": $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense $ git branch -d nonsense error: branch 'nonsense' not found. Worse, "git update-ref --no-deref -d" doesn't work for such repairs either: $ git update-ref -d refs/heads/nonsense error: unable to resolve reference refs/heads/nonsense: Too many levels of symbolic links Fix both by teaching resolve_ref_unsafe a new RESOLVE_REF_NO_RECURSE flag and passing it when appropriate. Callers can still read the value of a symref (for example to print a message about it) with that flag set --- resolve_ref_unsafe will resolve one level of symrefs and stop there. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Nieder committed
Sep 10, 2014 at 18:22 UTC
62a2d52514aed2b684409cb48e40e0cd14335d1b
6 files changed
+68
-3
builtin/branch.c
+2
-1
@@ -236,7 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
236
free(name);
237
238
name = mkpathdup(fmt, bname.buf);
239
- target = resolve_ref_unsafe(name, 0, sha1, &flags);
239
+ target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE,
240
+ sha1, &flags);
241
if (!target ||
242
(!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {
243
error(remote_branch
cache.h
+6
@@ -973,6 +973,11 @@ extern int read_ref(const char *refname, unsigned char *sha1);
973
* "writing" to the ref, the return value is the name of the ref
974
* that will actually be created or changed.
975
*
976
+ * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one
977
+ * level of symbolic reference. The value stored in sha1 for a symbolic
978
+ * reference will always be null_sha1 in this case, and the return
979
+ * value is the reference that the symref refers to directly.
980
+ *
981
* If flags is non-NULL, set the value that it points to the
982
* combination of REF_ISPACKED (if the reference was found among the
983
* packed references), REF_ISSYMREF (if the initial reference was a
@@ -985,6 +990,7 @@ extern int read_ref(const char *refname, unsigned char *sha1);
990
* errno is set to something meaningful on error.
991
*/
992
#define RESOLVE_REF_READING 0x01
993
+#define RESOLVE_REF_NO_RECURSE 0x02
994
extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
995
extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
996
refs.c
+15
-2
@@ -1467,6 +1467,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1467
refname = refname_buffer;
1468
if (flags)
1469
*flags |= REF_ISSYMREF;
1470
+ if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
1471
+ hashclr(sha1);
1472
+ return refname;
1473
+ }
1474
continue;
1475
}
1476
}
@@ -1523,13 +1527,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1527
buf = buffer + 4;
1528
while (isspace(*buf))
1529
buf++;
1530
+ refname = strcpy(refname_buffer, buf);
1531
+ if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
1532
+ hashclr(sha1);
1533
+ return refname;
1534
+ }
1535
if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
1536
if (flags)
1537
*flags |= REF_ISBROKEN;
1538
errno = EINVAL;
1539
return NULL;
1540
}
1532
- refname = strcpy(refname_buffer, buf);
1541
}
1542
}
1543
@@ -2170,6 +2178,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2178
2179
if (mustexist)
2180
resolve_flags |= RESOLVE_REF_READING;
2181
+ if (flags & REF_NODEREF && flags & REF_DELETING)
2182
+ resolve_flags |= RESOLVE_REF_NO_RECURSE;
2183
2184
refname = resolve_ref_unsafe(refname, resolve_flags,
2185
lock->old_sha1, &type);
@@ -3664,13 +3674,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3674
/* Acquire all locks while verifying old values */
3675
for (i = 0; i < n; i++) {
3676
struct ref_update *update = updates[i];
3677
+ int flags = update->flags;
3678
3679
+ if (is_null_sha1(update->new_sha1))
3680
+ flags |= REF_DELETING;
3681
update->lock = lock_ref_sha1_basic(update->refname,
3682
(update->have_old ?
3683
update->old_sha1 :
3684
NULL),
3685
NULL,
3673
- update->flags,
3686
+ flags,
3687
&update->type);
3688
if (!update->lock) {
3689
ret = (errno == ENOTDIR)
refs.h
+2
@@ -177,10 +177,12 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
177
* ref_transaction_create(), etc.
178
* REF_NODEREF: act on the ref directly, instead of dereferencing
179
* symbolic references.
180
+ * REF_DELETING: tolerate broken refs
181
*
182
* Flags >= 0x100 are reserved for internal use.
183
*/
184
#define REF_NODEREF 0x01
185
+#define REF_DELETING 0x02
186
/*
187
* This function sets errno to something meaningful on failure.
188
*/
t/t1400-update-ref.sh
+34
@@ -110,6 +110,40 @@ test_expect_success "delete symref without dereference when the referred ref is
110
cp -f .git/HEAD.orig .git/HEAD
111
git update-ref -d $m
112
113
+test_expect_success 'update-ref -d is not confused by self-reference' '
114
+ git symbolic-ref refs/heads/self refs/heads/self &&
115
+ test_when_finished "rm -f .git/refs/heads/self" &&
116
+ test_path_is_file .git/refs/heads/self &&
117
+ test_must_fail git update-ref -d refs/heads/self &&
118
+ test_path_is_file .git/refs/heads/self
119
+'
120
+
121
+test_expect_success 'update-ref --no-deref -d can delete self-reference' '
122
+ git symbolic-ref refs/heads/self refs/heads/self &&
123
+ test_when_finished "rm -f .git/refs/heads/self" &&
124
+ test_path_is_file .git/refs/heads/self &&
125
+ git update-ref --no-deref -d refs/heads/self &&
126
+ test_path_is_missing .git/refs/heads/self
127
+'
128
+
129
+test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
130
+ >.git/refs/heads/bad &&
131
+ test_when_finished "rm -f .git/refs/heads/bad" &&
132
+ git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
133
+ test_when_finished "rm -f .git/refs/heads/ref-to-bad" &&
134
+ test_path_is_file .git/refs/heads/ref-to-bad &&
135
+ git update-ref --no-deref -d refs/heads/ref-to-bad &&
136
+ test_path_is_missing .git/refs/heads/ref-to-bad
137
+'
138
+
139
+test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
140
+ git symbolic-ref refs/heads/badname refs/heads/broken...ref &&
141
+ test_when_finished "rm -f .git/refs/heads/badname" &&
142
+ test_path_is_file .git/refs/heads/badname &&
143
+ git update-ref --no-deref -d refs/heads/badname &&
144
+ test_path_is_missing .git/refs/heads/badname
145
+'
146
+
147
test_expect_success '(not) create HEAD with old sha1' "
148
test_must_fail git update-ref HEAD $A $B
149
"
t/t3200-branch.sh
+9
@@ -285,6 +285,15 @@ test_expect_success 'deleting a dangling symref' '
285
test_i18ncmp expect actual
286
'
287
288
+test_expect_success 'deleting a self-referential symref' '
289
+ git symbolic-ref refs/heads/self-reference refs/heads/self-reference &&
290
+ test_path_is_file .git/refs/heads/self-reference &&
291
+ echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect &&
292
+ git branch -d self-reference >actual &&
293
+ test_path_is_missing .git/refs/heads/self-reference &&
294
+ test_i18ncmp expect actual
295
+'
296
+
297
test_expect_success 'renaming a symref is not allowed' '
298
git symbolic-ref refs/heads/master2 refs/heads/master &&
299
test_must_fail git branch -m master2 master3 &&