refs: don't dereference on rename
When renaming refs, don't dereference either the origin or the destination before renaming. The origin does not need to be dereferenced because it is presently forbidden to rename symbolic refs. Not dereferencing the destination fixes a bug where renaming on top of a broken symref would use the pointed-to ref name for the moved reflog. Add a test for the reflog bug. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
David Turner committed
Feb 24, 2016 at 17:58 UTC
12fd3496d19c33c6401c5fdc7558944d46124a0f
2 files changed
+25
-5
refs/files-backend.c
+16
-5
@@ -2333,7 +2333,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2333
if (log && S_ISLNK(loginfo.st_mode))
2334
return error("reflog for %s is a symlink", oldrefname);
2335
2336
- if (!resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING, orig_sha1, &flag))
2336
+ if (!resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
2337
+ orig_sha1, &flag))
2338
return error("refname %s not found", oldrefname);
2339
2340
if (flag & REF_ISSYMREF)
@@ -2351,8 +2352,16 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2352
goto rollback;
2353
}
2354
2354
- if (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL) &&
2355
- delete_ref(newrefname, sha1, REF_NODEREF)) {
2355
+ /*
2356
+ * Since we are doing a shallow lookup, sha1 is not the
2357
+ * correct value to pass to delete_ref as old_sha1. But that
2358
+ * doesn't matter, because an old_sha1 check wouldn't add to
2359
+ * the safety anyway; we want to delete the reference whatever
2360
+ * its current value.
2361
+ */
2362
+ if (!read_ref_full(newrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
2363
+ sha1, NULL) &&
2364
+ delete_ref(newrefname, NULL, REF_NODEREF)) {
2365
if (errno==EISDIR) {
2366
struct strbuf path = STRBUF_INIT;
2367
int result;
@@ -2376,7 +2385,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2385
2386
logmoved = log;
2387
2379
- lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, 0, NULL, &err);
2388
+ lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, REF_NODEREF,
2389
+ NULL, &err);
2390
if (!lock) {
2391
error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
2392
strbuf_release(&err);
@@ -2394,7 +2404,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2404
return 0;
2405
2406
rollback:
2397
- lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, 0, NULL, &err);
2407
+ lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, REF_NODEREF,
2408
+ NULL, &err);
2409
if (!lock) {
2410
error("unable to lock %s for rollback: %s", oldrefname, err.buf);
2411
strbuf_release(&err);
t/t3200-branch.sh
+9
@@ -79,6 +79,15 @@ test_expect_success 'git branch -m dumps usage' '
79
test_i18ngrep "branch name required" err
80
'
81
82
+test_expect_success 'git branch -m m broken_symref should work' '
83
+ test_when_finished "git branch -D broken_symref" &&
84
+ git branch -l m &&
85
+ git symbolic-ref refs/heads/broken_symref refs/heads/i_am_broken &&
86
+ git branch -m m broken_symref &&
87
+ git reflog exists refs/heads/broken_symref &&
88
+ test_must_fail git reflog exists refs/heads/i_am_broken
89
+'
90
+
91
test_expect_success 'git branch -m m m/m should work' '
92
git branch -l m &&
93
git branch -m m m/m &&