From 62a2d52514aed2b684409cb48e40e0cd14335d1b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 10 Sep 2014 18:22:48 -0700 Subject: [PATCH] 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 Reviewed-by: Ronnie Sahlberg Signed-off-by: Junio C Hamano --- builtin/branch.c | 3 ++- cache.h | 6 ++++++ refs.c | 17 +++++++++++++++-- refs.h | 2 ++ t/t1400-update-ref.sh | 34 ++++++++++++++++++++++++++++++++++ t/t3200-branch.sh | 9 +++++++++ 6 files changed, 68 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6491bacd69..2ad2d0b44a 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -236,7 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(name); name = mkpathdup(fmt, bname.buf); - target = resolve_ref_unsafe(name, 0, sha1, &flags); + target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE, + sha1, &flags); if (!target || (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) { error(remote_branch diff --git a/cache.h b/cache.h index 720063968b..b735f3f1e3 100644 --- a/cache.h +++ b/cache.h @@ -973,6 +973,11 @@ extern int read_ref(const char *refname, unsigned char *sha1); * "writing" to the ref, the return value is the name of the ref * that will actually be created or changed. * + * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one + * level of symbolic reference. The value stored in sha1 for a symbolic + * reference will always be null_sha1 in this case, and the return + * value is the reference that the symref refers to directly. + * * If flags is non-NULL, set the value that it points to the * combination of REF_ISPACKED (if the reference was found among the * packed references), REF_ISSYMREF (if the initial reference was a @@ -985,6 +990,7 @@ extern int read_ref(const char *refname, unsigned char *sha1); * errno is set to something meaningful on error. */ #define RESOLVE_REF_READING 0x01 +#define RESOLVE_REF_NO_RECURSE 0x02 extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags); extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags); diff --git a/refs.c b/refs.c index f8d59ab7f0..4fe263ef9a 100644 --- a/refs.c +++ b/refs.c @@ -1467,6 +1467,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned refname = refname_buffer; if (flags) *flags |= REF_ISSYMREF; + if (resolve_flags & RESOLVE_REF_NO_RECURSE) { + hashclr(sha1); + return refname; + } continue; } } @@ -1523,13 +1527,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned buf = buffer + 4; while (isspace(*buf)) buf++; + refname = strcpy(refname_buffer, buf); + if (resolve_flags & RESOLVE_REF_NO_RECURSE) { + hashclr(sha1); + return refname; + } if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) { if (flags) *flags |= REF_ISBROKEN; errno = EINVAL; return NULL; } - refname = strcpy(refname_buffer, buf); } } @@ -2170,6 +2178,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, if (mustexist) resolve_flags |= RESOLVE_REF_READING; + if (flags & REF_NODEREF && flags & REF_DELETING) + resolve_flags |= RESOLVE_REF_NO_RECURSE; refname = resolve_ref_unsafe(refname, resolve_flags, lock->old_sha1, &type); @@ -3664,13 +3674,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Acquire all locks while verifying old values */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; + int flags = update->flags; + if (is_null_sha1(update->new_sha1)) + flags |= REF_DELETING; update->lock = lock_ref_sha1_basic(update->refname, (update->have_old ? update->old_sha1 : NULL), NULL, - update->flags, + flags, &update->type); if (!update->lock) { ret = (errno == ENOTDIR) diff --git a/refs.h b/refs.h index eea104478f..79802d79c0 100644 --- a/refs.h +++ b/refs.h @@ -177,10 +177,12 @@ extern int peel_ref(const char *refname, unsigned char *sha1); * ref_transaction_create(), etc. * REF_NODEREF: act on the ref directly, instead of dereferencing * symbolic references. + * REF_DELETING: tolerate broken refs * * Flags >= 0x100 are reserved for internal use. */ #define REF_NODEREF 0x01 +#define REF_DELETING 0x02 /* * This function sets errno to something meaningful on failure. */ diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 0218e96366..7c8c41a1a0 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -110,6 +110,40 @@ test_expect_success "delete symref without dereference when the referred ref is cp -f .git/HEAD.orig .git/HEAD git update-ref -d $m +test_expect_success 'update-ref -d is not confused by self-reference' ' + git symbolic-ref refs/heads/self refs/heads/self && + test_when_finished "rm -f .git/refs/heads/self" && + test_path_is_file .git/refs/heads/self && + test_must_fail git update-ref -d refs/heads/self && + test_path_is_file .git/refs/heads/self +' + +test_expect_success 'update-ref --no-deref -d can delete self-reference' ' + git symbolic-ref refs/heads/self refs/heads/self && + test_when_finished "rm -f .git/refs/heads/self" && + test_path_is_file .git/refs/heads/self && + git update-ref --no-deref -d refs/heads/self && + test_path_is_missing .git/refs/heads/self +' + +test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' ' + >.git/refs/heads/bad && + test_when_finished "rm -f .git/refs/heads/bad" && + git symbolic-ref refs/heads/ref-to-bad refs/heads/bad && + test_when_finished "rm -f .git/refs/heads/ref-to-bad" && + test_path_is_file .git/refs/heads/ref-to-bad && + git update-ref --no-deref -d refs/heads/ref-to-bad && + test_path_is_missing .git/refs/heads/ref-to-bad +' + +test_expect_success 'update-ref --no-deref -d can delete reference to broken name' ' + git symbolic-ref refs/heads/badname refs/heads/broken...ref && + test_when_finished "rm -f .git/refs/heads/badname" && + test_path_is_file .git/refs/heads/badname && + git update-ref --no-deref -d refs/heads/badname && + test_path_is_missing .git/refs/heads/badname +' + test_expect_success '(not) create HEAD with old sha1' " test_must_fail git update-ref HEAD $A $B " diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index ac31b711f2..432921b6b8 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -285,6 +285,15 @@ test_expect_success 'deleting a dangling symref' ' test_i18ncmp expect actual ' +test_expect_success 'deleting a self-referential symref' ' + git symbolic-ref refs/heads/self-reference refs/heads/self-reference && + test_path_is_file .git/refs/heads/self-reference && + echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect && + git branch -d self-reference >actual && + test_path_is_missing .git/refs/heads/self-reference && + test_i18ncmp expect actual +' + test_expect_success 'renaming a symref is not allowed' ' git symbolic-ref refs/heads/master2 refs/heads/master && test_must_fail git branch -m master2 master3 && -- 2.11.4.GIT