From 2bf68ed5aa79fcccba6ea4c021b15e51c019a85e Mon Sep 17 00:00:00 2001
From: David Turner <dturner@twopensource.com>
Date: Thu, 7 Apr 2016 15:02:48 -0400
Subject: [PATCH 01/24] refs: move head_ref{,_submodule} to the common code
These don't use any backend-specific functions. These were previously
defined in terms of the do_head_ref helper function, but since they
are otherwise identical, we don't need that function.
Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.c | 23 +++++++++++++++++++++++
refs/files-backend.c | 28 ----------------------------
2 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/refs.c b/refs.c
index b0e6ece6f4..6b8c16cdd8 100644
--- a/refs.c
+++ b/refs.c
@@ -1080,3 +1080,26 @@ int rename_ref_available(const char *oldname, const char *newname)
strbuf_release(&err);
return ret;
}
+
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+ struct object_id oid;
+ int flag;
+
+ if (submodule) {
+ if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
+ return fn("HEAD", &oid, 0, cb_data);
+
+ return 0;
+ }
+
+ if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
+ return fn("HEAD", &oid, flag, cb_data);
+
+ return 0;
+}
+
+int head_ref(each_ref_fn fn, void *cb_data)
+{
+ return head_ref_submodule(NULL, fn, cb_data);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 81f68f846b..c07dc418d6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1745,34 +1745,6 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
return do_for_each_entry(refs, base, do_one_ref, &data);
}
-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- struct object_id oid;
- int flag;
-
- if (submodule) {
- if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
- return fn("HEAD", &oid, 0, cb_data);
-
- return 0;
- }
-
- if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
- return fn("HEAD", &oid, flag, cb_data);
-
- return 0;
-}
-
-int head_ref(each_ref_fn fn, void *cb_data)
-{
- return do_head_ref(NULL, fn, cb_data);
-}
-
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return do_head_ref(submodule, fn, cb_data);
-}
-
int for_each_ref(each_ref_fn fn, void *cb_data)
{
return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
From 937705901b83737a35996f62dbfbfefd8e5c8d7f Mon Sep 17 00:00:00 2001
From: David Turner <dturner@twopensource.com>
Date: Thu, 7 Apr 2016 15:02:49 -0400
Subject: [PATCH 02/24] refs: move for_each_*ref* functions into common code
Make do_for_each_ref take a submodule as an argument instead of a
ref_cache. Since all for_each_*ref* functions are defined in terms of
do_for_each_ref, we can then move them into the common code.
Later, we can simply make do_for_each_ref into a backend function.
Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.c | 52 +++++++++++++++++++++++++++++++++++++
refs/files-backend.c | 62 ++++----------------------------------------
refs/refs-internal.h | 9 +++++++
3 files changed, 66 insertions(+), 57 deletions(-)
diff --git a/refs.c b/refs.c
index 6b8c16cdd8..f0feff72da 100644
--- a/refs.c
+++ b/refs.c
@@ -1103,3 +1103,55 @@ int head_ref(each_ref_fn fn, void *cb_data)
{
return head_ref_submodule(NULL, fn, cb_data);
}
+
+int for_each_ref(each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(NULL, "", fn, 0, 0, cb_data);
+}
+
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(submodule, "", fn, 0, 0, cb_data);
+}
+
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data);
+}
+
+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
+{
+ unsigned int flag = 0;
+
+ if (broken)
+ flag = DO_FOR_EACH_INCLUDE_BROKEN;
+ return do_for_each_ref(NULL, prefix, fn, 0, flag, cb_data);
+}
+
+int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(submodule, prefix, fn, strlen(prefix), 0, cb_data);
+}
+
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(NULL, git_replace_ref_base, fn,
+ strlen(git_replace_ref_base), 0, cb_data);
+}
+
+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int ret;
+ strbuf_addf(&buf, "%srefs/", get_git_namespace());
+ ret = do_for_each_ref(NULL, buf.buf, fn, 0, 0, cb_data);
+ strbuf_release(&buf);
+ return ret;
+}
+
+int for_each_rawref(each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref(NULL, "", fn, 0,
+ DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c07dc418d6..9676ec2fd1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -513,9 +513,6 @@ static void sort_ref_dir(struct ref_dir *dir)
dir->sorted = dir->nr = i;
}
-/* Include broken references in a do_for_each_ref*() iteration: */
-#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
-
/*
* Return true iff the reference described by entry can be resolved to
* an object in the database. Emit a warning if the referred-to
@@ -1727,10 +1724,13 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
* value, stop the iteration and return that value; otherwise, return
* 0.
*/
-static int do_for_each_ref(struct ref_cache *refs, const char *base,
- each_ref_fn fn, int trim, int flags, void *cb_data)
+int do_for_each_ref(const char *submodule, const char *base,
+ each_ref_fn fn, int trim, int flags, void *cb_data)
{
struct ref_entry_cb data;
+ struct ref_cache *refs;
+
+ refs = get_ref_cache(submodule);
data.base = base;
data.trim = trim;
data.flags = flags;
@@ -1745,58 +1745,6 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
return do_for_each_entry(refs, base, do_one_ref, &data);
}
-int for_each_ref(each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
-}
-
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
-}
-
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
-}
-
-int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
-{
- unsigned int flag = 0;
-
- if (broken)
- flag = DO_FOR_EACH_INCLUDE_BROKEN;
- return do_for_each_ref(&ref_cache, prefix, fn, 0, flag, cb_data);
-}
-
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
-}
-
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(&ref_cache, git_replace_ref_base, fn,
- strlen(git_replace_ref_base), 0, cb_data);
-}
-
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
-{
- struct strbuf buf = STRBUF_INIT;
- int ret;
- strbuf_addf(&buf, "%srefs/", get_git_namespace());
- ret = do_for_each_ref(&ref_cache, buf.buf, fn, 0, 0, cb_data);
- strbuf_release(&buf);
- return ret;
-}
-
-int for_each_rawref(each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(&ref_cache, "", fn, 0,
- DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
-}
-
static void unlock_ref(struct ref_lock *lock)
{
/* Do not free lock->lk -- atexit() still looks at them */
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index c7dded35f4..92aae80c21 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -197,4 +197,13 @@ const char *find_descendant_ref(const char *dirname,
int rename_ref_available(const char *oldname, const char *newname);
+
+/* Include broken references in a do_for_each_ref*() iteration: */
+#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+
+/*
+ * The common backend for the for_each_*ref* functions
+ */
+int do_for_each_ref(const char *submodule, const char *base,
+ each_ref_fn fn, int trim, int flags, void *cb_data);
#endif /* REFS_REFS_INTERNAL_H */
From f86d8350c8c72851dc9c250dbf0a6f1254e59282 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:50 -0400
Subject: [PATCH 03/24] t1430: test the output and error of some commands more
carefully
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1430-bad-ref-name.sh | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index c465abe8e3..005e2b1756 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -42,7 +42,7 @@ test_expect_success 'git branch shows badly named ref as warning' '
cp .git/refs/heads/master .git/refs/heads/broken...ref &&
test_when_finished "rm -f .git/refs/heads/broken...ref" &&
git branch >output 2>error &&
- grep -e "broken\.\.\.ref" error &&
+ test_i18ngrep -e "ignoring ref with broken name refs/heads/broken\.\.\.ref" error &&
! grep -e "broken\.\.\.ref" output
'
@@ -152,21 +152,25 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
git rev-parse --verify one >expect &&
git rev-parse --verify shadow >actual 2>err &&
test_cmp expect actual &&
- test_i18ngrep "ignoring.*refs/tags/shadow" err
+ test_i18ngrep "ignoring dangling symref refs/tags/shadow" err
'
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
+ git update-ref --no-deref -d refs/heads/badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/badname &&
+ test_must_be_empty output &&
+ test_must_be_empty error
'
test_expect_success 'update-ref -d can delete broken name' '
cp .git/refs/heads/master .git/refs/heads/broken...ref &&
test_when_finished "rm -f .git/refs/heads/broken...ref" &&
- git update-ref -d refs/heads/broken...ref &&
+ git update-ref -d refs/heads/broken...ref >output 2>error &&
+ test_must_be_empty output &&
+ test_must_be_empty error &&
git branch >output 2>error &&
! grep -e "broken\.\.\.ref" error &&
! grep -e "broken\.\.\.ref" output
@@ -175,7 +179,9 @@ test_expect_success 'update-ref -d can delete broken name' '
test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '
echo precious >.git/my-private-file &&
echo precious >expect &&
- test_must_fail git update-ref -d my-private-file &&
+ test_must_fail git update-ref -d my-private-file >output 2>error &&
+ test_must_be_empty output &&
+ test_i18ngrep -e "cannot lock .*: unable to resolve reference" error &&
test_cmp expect .git/my-private-file
'
From 45669a79b1459dddf19fd5334411dd4998ce1668 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:51 -0400
Subject: [PATCH 04/24] t1430: clean up broken refs/tags/shadow
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1430-bad-ref-name.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 005e2b1756..cb815ab6c3 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -148,7 +148,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
git branch shadow one &&
cp .git/refs/heads/master .git/refs/heads/broken...ref &&
git symbolic-ref refs/tags/shadow refs/heads/broken...ref &&
-
+ test_when_finished "rm -f .git/refs/tags/shadow" &&
git rev-parse --verify one >expect &&
git rev-parse --verify shadow >actual 2>err &&
test_cmp expect actual &&
From 6141a6dcdce6b925b3670a995ffc59e78658521b Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:52 -0400
Subject: [PATCH 05/24] t1430: don't rely on symbolic-ref for creating broken
symrefs
It's questionable whether it should even work.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1430-bad-ref-name.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index cb815ab6c3..a9639519f0 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -147,7 +147,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
test_when_finished "rm -f .git/refs/heads/broken...ref" &&
git branch shadow one &&
cp .git/refs/heads/master .git/refs/heads/broken...ref &&
- git symbolic-ref refs/tags/shadow refs/heads/broken...ref &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow &&
test_when_finished "rm -f .git/refs/tags/shadow" &&
git rev-parse --verify one >expect &&
git rev-parse --verify shadow >actual 2>err &&
@@ -156,7 +156,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
'
test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
- git symbolic-ref refs/heads/badname refs/heads/broken...ref &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
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 >output 2>error &&
From b78ceced0ce0a8c098fad9d979b47ea98e27d22a Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:53 -0400
Subject: [PATCH 06/24] t1430: test for-each-ref in the presence of badly-named
refs
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1430-bad-ref-name.sh | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index a9639519f0..612cc32bfc 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -155,6 +155,22 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
test_i18ngrep "ignoring dangling symref refs/tags/shadow" err
'
+test_expect_success 'for-each-ref emits warnings for broken names' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ printf "ref: refs/heads/master\n" >.git/refs/heads/broken...symref &&
+ test_when_finished "rm -f .git/refs/heads/broken...symref" &&
+ git for-each-ref >output 2>error &&
+ ! grep -e "broken\.\.\.ref" output &&
+ ! grep -e "badname" output &&
+ ! grep -e "broken\.\.\.symref" output &&
+ test_i18ngrep "ignoring ref with broken name refs/heads/broken\.\.\.ref" error &&
+ test_i18ngrep "ignoring broken ref refs/heads/badname" error &&
+ test_i18ngrep "ignoring ref with broken name refs/heads/broken\.\.\.symref" error
+'
+
test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
test_when_finished "rm -f .git/refs/heads/badname" &&
From 757552db5794056cdeccab827745f3de1fb82f4b Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:54 -0400
Subject: [PATCH 07/24] t1430: improve test coverage of deletion of badly-named
refs
Check "branch -d broken...ref"
Check various combinations of
* Deleting using "update-ref -d"
* Deleting using "update-ref --no-deref -d"
* Deleting using "branch -d"
in the following combinations of symref -> ref:
* badname -> broken...ref
* badname -> broken...ref (dangling)
* broken...symref -> master
* broken...symref -> idonotexist (dangling)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1430-bad-ref-name.sh | 108 ++++++++++++++++++++++++++++++++++++----
1 file changed, 98 insertions(+), 10 deletions(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 612cc32bfc..25ddab4e98 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -171,16 +171,6 @@ test_expect_success 'for-each-ref emits warnings for broken names' '
test_i18ngrep "ignoring ref with broken name refs/heads/broken\.\.\.symref" error
'
-test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- 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 >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
- test_must_be_empty output &&
- test_must_be_empty error
-'
-
test_expect_success 'update-ref -d can delete broken name' '
cp .git/refs/heads/master .git/refs/heads/broken...ref &&
test_when_finished "rm -f .git/refs/heads/broken...ref" &&
@@ -192,6 +182,104 @@ test_expect_success 'update-ref -d can delete broken name' '
! grep -e "broken\.\.\.ref" output
'
+test_expect_success 'branch -d can delete broken name' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ git branch -d broken...ref >output 2>error &&
+ test_i18ngrep "Deleted branch broken...ref (was broken)" output &&
+ test_must_be_empty error &&
+ git branch >output 2>error &&
+ ! grep -e "broken\.\.\.ref" error &&
+ ! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'update-ref --no-deref -d can delete symref to broken name' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ git update-ref --no-deref -d refs/heads/badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/badname &&
+ test_must_be_empty output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'branch -d can delete symref to broken name' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ git branch -d badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/badname &&
+ test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ git update-ref --no-deref -d refs/heads/badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/badname &&
+ test_must_be_empty output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'branch -d can delete dangling symref to broken name' '
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ git branch -d badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/badname &&
+ test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'update-ref -d can delete broken name through symref' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test_when_finished "rm -f .git/refs/heads/badname" &&
+ git update-ref -d refs/heads/badname >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...ref &&
+ test_must_be_empty output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
+ printf "ref: refs/heads/master\n" >.git/refs/heads/broken...symref &&
+ test_when_finished "rm -f .git/refs/heads/broken...symref" &&
+ git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
+ test_must_be_empty output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'branch -d can delete symref with broken name' '
+ printf "ref: refs/heads/master\n" >.git/refs/heads/broken...symref &&
+ test_when_finished "rm -f .git/refs/heads/broken...symref" &&
+ git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
+ test_i18ngrep "Deleted branch broken...symref (was refs/heads/master)" output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
+ printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test_when_finished "rm -f .git/refs/heads/broken...symref" &&
+ git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
+ test_must_be_empty output &&
+ test_must_be_empty error
+'
+
+test_expect_success 'branch -d can delete dangling symref with broken name' '
+ printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test_when_finished "rm -f .git/refs/heads/broken...symref" &&
+ git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
+ test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
+ test_must_be_empty error
+'
+
test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '
echo precious >.git/my-private-file &&
echo precious >expect &&
From 419c6f4c76c5950a33ed2cedbce9c6f7d66773f6 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:55 -0400
Subject: [PATCH 08/24] resolve_missing_loose_ref(): simplify semantics
Make resolve_missing_loose_ref() only responsible for looking up a
packed reference, without worrying about whether we want to read or
write the reference and without setting errno on failure. Move the other
logic to the caller.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9676ec2fd1..c0cf6fd6c0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1368,11 +1368,9 @@ static struct ref_entry *get_packed_ref(const char *refname)
}
/*
- * A loose ref file doesn't exist; check for a packed ref. The
- * options are forwarded from resolve_safe_unsafe().
+ * A loose ref file doesn't exist; check for a packed ref.
*/
static int resolve_missing_loose_ref(const char *refname,
- int resolve_flags,
unsigned char *sha1,
int *flags)
{
@@ -1389,14 +1387,8 @@ static int resolve_missing_loose_ref(const char *refname,
*flags |= REF_ISPACKED;
return 0;
}
- /* The reference is not a packed reference, either. */
- if (resolve_flags & RESOLVE_REF_READING) {
- errno = ENOENT;
- return -1;
- } else {
- hashclr(sha1);
- return 0;
- }
+ /* refname is not a packed reference. */
+ return -1;
}
/* This function needs to return a meaningful errno on failure */
@@ -1461,9 +1453,13 @@ static const char *resolve_ref_1(const char *refname,
if (lstat(path, &st) < 0) {
if (errno != ENOENT)
return NULL;
- if (resolve_missing_loose_ref(refname, resolve_flags,
- sha1, flags))
- return NULL;
+ if (resolve_missing_loose_ref(refname, sha1, flags)) {
+ if (resolve_flags & RESOLVE_REF_READING) {
+ errno = ENOENT;
+ return NULL;
+ }
+ hashclr(sha1);
+ }
if (bad_name) {
hashclr(sha1);
if (flags)
From 37da4227b27771cd9947b7393851975a311e6eb9 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:56 -0400
Subject: [PATCH 09/24] resolve_ref_unsafe(): use for loop to count up to
MAXDEPTH
The loop's there anyway; we might as well use it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c0cf6fd6c0..101abba8d1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1400,8 +1400,8 @@ static const char *resolve_ref_1(const char *refname,
struct strbuf *sb_path,
struct strbuf *sb_contents)
{
- int depth = MAXDEPTH;
int bad_name = 0;
+ int symref_count;
if (flags)
*flags = 0;
@@ -1425,17 +1425,13 @@ static const char *resolve_ref_1(const char *refname,
*/
bad_name = 1;
}
- for (;;) {
+
+ for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
const char *path;
struct stat st;
char *buf;
int fd;
- if (--depth < 0) {
- errno = ELOOP;
- return NULL;
- }
-
strbuf_reset(sb_path);
strbuf_git_path(sb_path, "%s", refname);
path = sb_path->buf;
@@ -1566,6 +1562,9 @@ static const char *resolve_ref_1(const char *refname,
bad_name = 1;
}
}
+
+ errno = ELOOP;
+ return NULL;
}
const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
From a70a93b7947d3aa6a202342256e616460a2b6e5b Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:57 -0400
Subject: [PATCH 10/24] resolve_ref_unsafe(): ensure flags is always set
If the caller passes flags==NULL, then set it to point at a local
scratch variable. This removes the need for a lot of "if (flags)"
guards in resolve_ref_1() and resolve_missing_loose_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 101abba8d1..067ce1c39e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1383,8 +1383,7 @@ static int resolve_missing_loose_ref(const char *refname,
entry = get_packed_ref(refname);
if (entry) {
hashcpy(sha1, entry->u.value.oid.hash);
- if (flags)
- *flags |= REF_ISPACKED;
+ *flags |= REF_ISPACKED;
return 0;
}
/* refname is not a packed reference. */
@@ -1403,12 +1402,10 @@ static const char *resolve_ref_1(const char *refname,
int bad_name = 0;
int symref_count;
- if (flags)
- *flags = 0;
+ *flags = 0;
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- if (flags)
- *flags |= REF_BAD_NAME;
+ *flags |= REF_BAD_NAME;
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
@@ -1458,8 +1455,7 @@ static const char *resolve_ref_1(const char *refname,
}
if (bad_name) {
hashclr(sha1);
- if (flags)
- *flags |= REF_ISBROKEN;
+ *flags |= REF_ISBROKEN;
}
return refname;
}
@@ -1478,8 +1474,7 @@ static const char *resolve_ref_1(const char *refname,
!check_refname_format(sb_contents->buf, 0)) {
strbuf_swap(sb_refname, sb_contents);
refname = sb_refname->buf;
- if (flags)
- *flags |= REF_ISSYMREF;
+ *flags |= REF_ISSYMREF;
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
hashclr(sha1);
return refname;
@@ -1526,20 +1521,17 @@ static const char *resolve_ref_1(const char *refname,
*/
if (get_sha1_hex(sb_contents->buf, sha1) ||
(sb_contents->buf[40] != '\0' && !isspace(sb_contents->buf[40]))) {
- if (flags)
- *flags |= REF_ISBROKEN;
+ *flags |= REF_ISBROKEN;
errno = EINVAL;
return NULL;
}
if (bad_name) {
hashclr(sha1);
- if (flags)
- *flags |= REF_ISBROKEN;
+ *flags |= REF_ISBROKEN;
}
return refname;
}
- if (flags)
- *flags |= REF_ISSYMREF;
+ *flags |= REF_ISSYMREF;
buf = sb_contents->buf + 4;
while (isspace(*buf))
buf++;
@@ -1551,8 +1543,7 @@ static const char *resolve_ref_1(const char *refname,
return refname;
}
if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
- if (flags)
- *flags |= REF_ISBROKEN;
+ *flags |= REF_ISBROKEN;
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(buf)) {
@@ -1573,8 +1564,12 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
static struct strbuf sb_refname = STRBUF_INIT;
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
+ int unused_flags;
const char *ret;
+ if (!flags)
+ flags = &unused_flags;
+
ret = resolve_ref_1(refname, resolve_flags, sha1, flags,
&sb_refname, &sb_path, &sb_contents);
strbuf_release(&sb_path);
From 90c28ae11c5a9d8386606c727be1d80699c9aa92 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:58 -0400
Subject: [PATCH 11/24] resolve_ref_1(): eliminate local variable
In place of `buf`, use `refname`, which is anyway a better description
of what is being pointed at.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 067ce1c39e..69ec9036e0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1426,7 +1426,6 @@ static const char *resolve_ref_1(const char *refname,
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
const char *path;
struct stat st;
- char *buf;
int fd;
strbuf_reset(sb_path);
@@ -1532,21 +1531,21 @@ static const char *resolve_ref_1(const char *refname,
return refname;
}
*flags |= REF_ISSYMREF;
- buf = sb_contents->buf + 4;
- while (isspace(*buf))
- buf++;
+ refname = sb_contents->buf + 4;
+ while (isspace(*refname))
+ refname++;
strbuf_reset(sb_refname);
- strbuf_addstr(sb_refname, buf);
+ strbuf_addstr(sb_refname, refname);
refname = sb_refname->buf;
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
hashclr(sha1);
return refname;
}
- if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
*flags |= REF_ISBROKEN;
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
- !refname_is_safe(buf)) {
+ !refname_is_safe(refname)) {
errno = EINVAL;
return NULL;
}
From e6702e570ba4a8501362594b66b74d0ecff002dd Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:02:59 -0400
Subject: [PATCH 12/24] resolve_ref_1(): reorder code
There is no need to adjust *flags if we're just about to fail.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 69ec9036e0..60f149370a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1542,13 +1542,13 @@ static const char *resolve_ref_1(const char *refname,
return refname;
}
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- *flags |= REF_ISBROKEN;
-
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
errno = EINVAL;
return NULL;
}
+
+ *flags |= REF_ISBROKEN;
bad_name = 1;
}
}
From afbe782fa3ea4278dee2c19d0f3c2dfa50522ff2 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:00 -0400
Subject: [PATCH 13/24] resolve_ref_1(): eliminate local variable "bad_name"
We can use (*flags & REF_BAD_NAME) for that purpose.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 60f149370a..b865ba5221 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1399,19 +1399,17 @@ static const char *resolve_ref_1(const char *refname,
struct strbuf *sb_path,
struct strbuf *sb_contents)
{
- int bad_name = 0;
int symref_count;
*flags = 0;
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- *flags |= REF_BAD_NAME;
-
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
errno = EINVAL;
return NULL;
}
+
/*
* dwim_ref() uses REF_ISBROKEN to distinguish between
* missing refs and refs that were present but invalid,
@@ -1420,7 +1418,7 @@ static const char *resolve_ref_1(const char *refname,
* We don't know whether the ref exists, so don't set
* REF_ISBROKEN yet.
*/
- bad_name = 1;
+ *flags |= REF_BAD_NAME;
}
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
@@ -1452,7 +1450,7 @@ static const char *resolve_ref_1(const char *refname,
}
hashclr(sha1);
}
- if (bad_name) {
+ if (*flags & REF_BAD_NAME) {
hashclr(sha1);
*flags |= REF_ISBROKEN;
}
@@ -1524,7 +1522,7 @@ static const char *resolve_ref_1(const char *refname,
errno = EINVAL;
return NULL;
}
- if (bad_name) {
+ if (*flags & REF_BAD_NAME) {
hashclr(sha1);
*flags |= REF_ISBROKEN;
}
@@ -1548,8 +1546,7 @@ static const char *resolve_ref_1(const char *refname,
return NULL;
}
- *flags |= REF_ISBROKEN;
- bad_name = 1;
+ *flags |= REF_ISBROKEN | REF_BAD_NAME;
}
}
From 7048653a7351df6049e2306a51b0a06b83aa181b Mon Sep 17 00:00:00 2001
From: David Turner <dturner@twopensource.com>
Date: Thu, 7 Apr 2016 15:03:01 -0400
Subject: [PATCH 14/24] files-backend: break out ref reading
Refactor resolve_ref_1 in terms of a new function read_raw_ref, which
is responsible for reading ref data from the ref storage.
Later, we will make read_raw_ref a pluggable backend function, and make
resolve_ref_unsafe common.
Signed-off-by: David Turner <dturner@twopensource.com>
Helped-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 252 +++++++++++++++++++++++++------------------
1 file changed, 149 insertions(+), 103 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b865ba5221..d51e7787f2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1390,6 +1390,141 @@ static int resolve_missing_loose_ref(const char *refname,
return -1;
}
+/*
+ * Read a raw ref from the filesystem or packed refs file.
+ *
+ * If the ref is a sha1, fill in sha1 and return 0.
+ *
+ * If the ref is symbolic, fill in *symref with the referrent
+ * (e.g. "refs/heads/master") and return 0. The caller is responsible
+ * for validating the referrent. Set REF_ISSYMREF in flags.
+ *
+ * If the ref doesn't exist, set errno to ENOENT and return -1.
+ *
+ * If the ref exists but is neither a symbolic ref nor a sha1, it is
+ * broken. Set REF_ISBROKEN in flags, set errno to EINVAL, and return
+ * -1.
+ *
+ * If there is another error reading the ref, set errno appropriately and
+ * return -1.
+ *
+ * Backend-specific flags might be set in flags as well, regardless of
+ * outcome.
+ *
+ * sb_path is workspace: the caller should allocate and free it.
+ *
+ * It is OK for refname to point into symref. In this case:
+ * - if the function succeeds with REF_ISSYMREF, symref will be
+ * overwritten and the memory pointed to by refname might be changed
+ * or even freed.
+ * - in all other cases, symref will be untouched, and therefore
+ * refname will still be valid and unchanged.
+ */
+static int read_raw_ref(const char *refname, unsigned char *sha1,
+ struct strbuf *symref, struct strbuf *sb_path,
+ struct strbuf *sb_contents, int *flags)
+{
+ const char *path;
+ const char *buf;
+ struct stat st;
+ int fd;
+
+ strbuf_reset(sb_path);
+ strbuf_git_path(sb_path, "%s", refname);
+ path = sb_path->buf;
+
+stat_ref:
+ /*
+ * We might have to loop back here to avoid a race
+ * condition: first we lstat() the file, then we try
+ * to read it as a link or as a file. But if somebody
+ * changes the type of the file (file <-> directory
+ * <-> symlink) between the lstat() and reading, then
+ * we don't want to report that as an error but rather
+ * try again starting with the lstat().
+ */
+
+ if (lstat(path, &st) < 0) {
+ if (errno != ENOENT)
+ return -1;
+ if (resolve_missing_loose_ref(refname, sha1, flags)) {
+ errno = ENOENT;
+ return -1;
+ }
+ return 0;
+ }
+
+ /* Follow "normalized" - ie "refs/.." symlinks by hand */
+ if (S_ISLNK(st.st_mode)) {
+ strbuf_reset(sb_contents);
+ if (strbuf_readlink(sb_contents, path, 0) < 0) {
+ if (errno == ENOENT || errno == EINVAL)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ if (starts_with(sb_contents->buf, "refs/") &&
+ !check_refname_format(sb_contents->buf, 0)) {
+ strbuf_swap(sb_contents, symref);
+ *flags |= REF_ISSYMREF;
+ return 0;
+ }
+ }
+
+ /* Is it a directory? */
+ if (S_ISDIR(st.st_mode)) {
+ errno = EISDIR;
+ return -1;
+ }
+
+ /*
+ * Anything else, just open it and try to use it as
+ * a ref
+ */
+ fd = open(path, O_RDONLY);
+ if (fd < 0) {
+ if (errno == ENOENT)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ strbuf_reset(sb_contents);
+ if (strbuf_read(sb_contents, fd, 256) < 0) {
+ int save_errno = errno;
+ close(fd);
+ errno = save_errno;
+ return -1;
+ }
+ close(fd);
+ strbuf_rtrim(sb_contents);
+ buf = sb_contents->buf;
+ if (starts_with(buf, "ref:")) {
+ buf += 4;
+ while (isspace(*buf))
+ buf++;
+
+ strbuf_reset(symref);
+ strbuf_addstr(symref, buf);
+ *flags |= REF_ISSYMREF;
+ return 0;
+ }
+
+ /*
+ * Please note that FETCH_HEAD has additional
+ * data after the sha.
+ */
+ if (get_sha1_hex(buf, sha1) ||
+ (buf[40] != '\0' && !isspace(buf[40]))) {
+ *flags |= REF_ISBROKEN;
+ errno = EINVAL;
+ return -1;
+ }
+
+ return 0;
+}
+
/* This function needs to return a meaningful errno on failure */
static const char *resolve_ref_1(const char *refname,
int resolve_flags,
@@ -1422,34 +1557,22 @@ static const char *resolve_ref_1(const char *refname,
}
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
- const char *path;
- struct stat st;
- int fd;
+ int read_flags = 0;
- strbuf_reset(sb_path);
- strbuf_git_path(sb_path, "%s", refname);
- path = sb_path->buf;
-
- /*
- * We might have to loop back here to avoid a race
- * condition: first we lstat() the file, then we try
- * to read it as a link or as a file. But if somebody
- * changes the type of the file (file <-> directory
- * <-> symlink) between the lstat() and reading, then
- * we don't want to report that as an error but rather
- * try again starting with the lstat().
- */
- stat_ref:
- if (lstat(path, &st) < 0) {
- if (errno != ENOENT)
+ if (read_raw_ref(refname, sha1, sb_refname,
+ sb_path, sb_contents, &read_flags)) {
+ *flags |= read_flags;
+ if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
return NULL;
- if (resolve_missing_loose_ref(refname, sha1, flags)) {
- if (resolve_flags & RESOLVE_REF_READING) {
- errno = ENOENT;
- return NULL;
- }
- hashclr(sha1);
- }
+ hashclr(sha1);
+ if (*flags & REF_BAD_NAME)
+ *flags |= REF_ISBROKEN;
+ return refname;
+ }
+
+ *flags |= read_flags;
+
+ if (!(read_flags & REF_ISSYMREF)) {
if (*flags & REF_BAD_NAME) {
hashclr(sha1);
*flags |= REF_ISBROKEN;
@@ -1457,83 +1580,6 @@ static const char *resolve_ref_1(const char *refname,
return refname;
}
- /* Follow "normalized" - ie "refs/.." symlinks by hand */
- if (S_ISLNK(st.st_mode)) {
- strbuf_reset(sb_contents);
- if (strbuf_readlink(sb_contents, path, 0) < 0) {
- if (errno == ENOENT || errno == EINVAL)
- /* inconsistent with lstat; retry */
- goto stat_ref;
- else
- return NULL;
- }
- if (starts_with(sb_contents->buf, "refs/") &&
- !check_refname_format(sb_contents->buf, 0)) {
- strbuf_swap(sb_refname, sb_contents);
- refname = sb_refname->buf;
- *flags |= REF_ISSYMREF;
- if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
- hashclr(sha1);
- return refname;
- }
- continue;
- }
- }
-
- /* Is it a directory? */
- if (S_ISDIR(st.st_mode)) {
- errno = EISDIR;
- return NULL;
- }
-
- /*
- * Anything else, just open it and try to use it as
- * a ref
- */
- fd = open(path, O_RDONLY);
- if (fd < 0) {
- if (errno == ENOENT)
- /* inconsistent with lstat; retry */
- goto stat_ref;
- else
- return NULL;
- }
- strbuf_reset(sb_contents);
- if (strbuf_read(sb_contents, fd, 256) < 0) {
- int save_errno = errno;
- close(fd);
- errno = save_errno;
- return NULL;
- }
- close(fd);
- strbuf_rtrim(sb_contents);
-
- /*
- * Is it a symbolic ref?
- */
- if (!starts_with(sb_contents->buf, "ref:")) {
- /*
- * Please note that FETCH_HEAD has a second
- * line containing other data.
- */
- if (get_sha1_hex(sb_contents->buf, sha1) ||
- (sb_contents->buf[40] != '\0' && !isspace(sb_contents->buf[40]))) {
- *flags |= REF_ISBROKEN;
- errno = EINVAL;
- return NULL;
- }
- if (*flags & REF_BAD_NAME) {
- hashclr(sha1);
- *flags |= REF_ISBROKEN;
- }
- return refname;
- }
- *flags |= REF_ISSYMREF;
- refname = sb_contents->buf + 4;
- while (isspace(*refname))
- refname++;
- strbuf_reset(sb_refname);
- strbuf_addstr(sb_refname, refname);
refname = sb_refname->buf;
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
hashclr(sha1);
From 42a38cf788428bf39db1729ee8c79c64252a2849 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:02 -0400
Subject: [PATCH 15/24] read_raw_ref(): manage own scratch space
Instead of creating scratch space in resolve_ref_unsafe() and passing
it down through resolve_ref_1 to read_raw_ref(), teach read_raw_ref()
to manage its own scratch space. This reduces coupling across the
functions at the cost of some extra allocations.
Also, when read_raw_ref() is implemented for different reference
backends, the other implementations might have different scratch
space requirements.
Note that we now preserve errno across the calls to strbuf_release(),
which calls free() and can thus theoretically overwrite errno.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 76 ++++++++++++++++++++++++--------------------
1 file changed, 41 insertions(+), 35 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d51e7787f2..f75256882c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1421,17 +1421,20 @@ static int resolve_missing_loose_ref(const char *refname,
* refname will still be valid and unchanged.
*/
static int read_raw_ref(const char *refname, unsigned char *sha1,
- struct strbuf *symref, struct strbuf *sb_path,
- struct strbuf *sb_contents, int *flags)
+ struct strbuf *symref, int *flags)
{
+ struct strbuf sb_contents = STRBUF_INIT;
+ struct strbuf sb_path = STRBUF_INIT;
const char *path;
const char *buf;
struct stat st;
int fd;
+ int ret = -1;
+ int save_errno;
- strbuf_reset(sb_path);
- strbuf_git_path(sb_path, "%s", refname);
- path = sb_path->buf;
+ strbuf_reset(&sb_path);
+ strbuf_git_path(&sb_path, "%s", refname);
+ path = sb_path.buf;
stat_ref:
/*
@@ -1446,36 +1449,38 @@ stat_ref:
if (lstat(path, &st) < 0) {
if (errno != ENOENT)
- return -1;
+ goto out;
if (resolve_missing_loose_ref(refname, sha1, flags)) {
errno = ENOENT;
- return -1;
+ goto out;
}
- return 0;
+ ret = 0;
+ goto out;
}
/* Follow "normalized" - ie "refs/.." symlinks by hand */
if (S_ISLNK(st.st_mode)) {
- strbuf_reset(sb_contents);
- if (strbuf_readlink(sb_contents, path, 0) < 0) {
+ strbuf_reset(&sb_contents);
+ if (strbuf_readlink(&sb_contents, path, 0) < 0) {
if (errno == ENOENT || errno == EINVAL)
/* inconsistent with lstat; retry */
goto stat_ref;
else
- return -1;
+ goto out;
}
- if (starts_with(sb_contents->buf, "refs/") &&
- !check_refname_format(sb_contents->buf, 0)) {
- strbuf_swap(sb_contents, symref);
+ if (starts_with(sb_contents.buf, "refs/") &&
+ !check_refname_format(sb_contents.buf, 0)) {
+ strbuf_swap(&sb_contents, symref);
*flags |= REF_ISSYMREF;
- return 0;
+ ret = 0;
+ goto out;
}
}
/* Is it a directory? */
if (S_ISDIR(st.st_mode)) {
errno = EISDIR;
- return -1;
+ goto out;
}
/*
@@ -1488,18 +1493,18 @@ stat_ref:
/* inconsistent with lstat; retry */
goto stat_ref;
else
- return -1;
+ goto out;
}
- strbuf_reset(sb_contents);
- if (strbuf_read(sb_contents, fd, 256) < 0) {
+ strbuf_reset(&sb_contents);
+ if (strbuf_read(&sb_contents, fd, 256) < 0) {
int save_errno = errno;
close(fd);
errno = save_errno;
- return -1;
+ goto out;
}
close(fd);
- strbuf_rtrim(sb_contents);
- buf = sb_contents->buf;
+ strbuf_rtrim(&sb_contents);
+ buf = sb_contents.buf;
if (starts_with(buf, "ref:")) {
buf += 4;
while (isspace(*buf))
@@ -1508,7 +1513,8 @@ stat_ref:
strbuf_reset(symref);
strbuf_addstr(symref, buf);
*flags |= REF_ISSYMREF;
- return 0;
+ ret = 0;
+ goto out;
}
/*
@@ -1519,10 +1525,17 @@ stat_ref:
(buf[40] != '\0' && !isspace(buf[40]))) {
*flags |= REF_ISBROKEN;
errno = EINVAL;
- return -1;
+ goto out;
}
- return 0;
+ ret = 0;
+
+out:
+ save_errno = errno;
+ strbuf_release(&sb_path);
+ strbuf_release(&sb_contents);
+ errno = save_errno;
+ return ret;
}
/* This function needs to return a meaningful errno on failure */
@@ -1530,9 +1543,7 @@ static const char *resolve_ref_1(const char *refname,
int resolve_flags,
unsigned char *sha1,
int *flags,
- struct strbuf *sb_refname,
- struct strbuf *sb_path,
- struct strbuf *sb_contents)
+ struct strbuf *sb_refname)
{
int symref_count;
@@ -1559,8 +1570,7 @@ static const char *resolve_ref_1(const char *refname,
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
int read_flags = 0;
- if (read_raw_ref(refname, sha1, sb_refname,
- sb_path, sb_contents, &read_flags)) {
+ if (read_raw_ref(refname, sha1, sb_refname, &read_flags)) {
*flags |= read_flags;
if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
return NULL;
@@ -1604,8 +1614,6 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
unsigned char *sha1, int *flags)
{
static struct strbuf sb_refname = STRBUF_INIT;
- struct strbuf sb_contents = STRBUF_INIT;
- struct strbuf sb_path = STRBUF_INIT;
int unused_flags;
const char *ret;
@@ -1613,9 +1621,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
flags = &unused_flags;
ret = resolve_ref_1(refname, resolve_flags, sha1, flags,
- &sb_refname, &sb_path, &sb_contents);
- strbuf_release(&sb_path);
- strbuf_release(&sb_contents);
+ &sb_refname);
return ret;
}
From 8c346fb1d791f388b307d1832aebc577477ed6fe Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:03 -0400
Subject: [PATCH 16/24] files-backend: inline resolve_ref_1() into
resolve_ref_unsafe()
resolve_ref_unsafe() wasn't doing anything useful anymore.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 31 +++++++++----------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f75256882c..120b2dddf8 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1539,14 +1539,16 @@ out:
}
/* This function needs to return a meaningful errno on failure */
-static const char *resolve_ref_1(const char *refname,
- int resolve_flags,
- unsigned char *sha1,
- int *flags,
- struct strbuf *sb_refname)
+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
+ unsigned char *sha1, int *flags)
{
+ static struct strbuf sb_refname = STRBUF_INIT;
+ int unused_flags;
int symref_count;
+ if (!flags)
+ flags = &unused_flags;
+
*flags = 0;
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
@@ -1570,7 +1572,7 @@ static const char *resolve_ref_1(const char *refname,
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
int read_flags = 0;
- if (read_raw_ref(refname, sha1, sb_refname, &read_flags)) {
+ if (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {
*flags |= read_flags;
if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
return NULL;
@@ -1590,7 +1592,7 @@ static const char *resolve_ref_1(const char *refname,
return refname;
}
- refname = sb_refname->buf;
+ refname = sb_refname.buf;
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
hashclr(sha1);
return refname;
@@ -1610,21 +1612,6 @@ static const char *resolve_ref_1(const char *refname,
return NULL;
}
-const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
- unsigned char *sha1, int *flags)
-{
- static struct strbuf sb_refname = STRBUF_INIT;
- int unused_flags;
- const char *ret;
-
- if (!flags)
- flags = &unused_flags;
-
- ret = resolve_ref_1(refname, resolve_flags, sha1, flags,
- &sb_refname);
- return ret;
-}
-
/*
* Peel the entry (if possible) and return its new peel_status. If
* repeel is true, re-peel the entry even if there is an old peeled
From 89e82389655e3792bebad311d74541c3c0c6cc86 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:04 -0400
Subject: [PATCH 17/24] read_raw_ref(): change flags parameter to unsigned int
read_raw_ref() is going to be part of the vtable for reference backends,
so clean up its interface to use "unsigned int flags" rather than "int
flags". Its caller still uses signed int for its flags arguments. But
changing that would touch a lot of code, so leave it for now.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 120b2dddf8..a15986ccc2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1372,7 +1372,7 @@ static struct ref_entry *get_packed_ref(const char *refname)
*/
static int resolve_missing_loose_ref(const char *refname,
unsigned char *sha1,
- int *flags)
+ unsigned int *flags)
{
struct ref_entry *entry;
@@ -1421,7 +1421,7 @@ static int resolve_missing_loose_ref(const char *refname,
* refname will still be valid and unchanged.
*/
static int read_raw_ref(const char *refname, unsigned char *sha1,
- struct strbuf *symref, int *flags)
+ struct strbuf *symref, unsigned int *flags)
{
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
@@ -1570,7 +1570,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
}
for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
- int read_flags = 0;
+ unsigned int read_flags = 0;
if (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {
*flags |= read_flags;
From 95ae2c7490fe241e6fbdc3e26789b3cee8ec9914 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:05 -0400
Subject: [PATCH 18/24] fsck_head_link(): remove unneeded flag variable
It is never read, so we can pass NULL to resolve_ref_unsafe().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/fsck.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 55eac756f7..3f27456883 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -493,13 +493,12 @@ static void fsck_object_dir(const char *path)
static int fsck_head_link(void)
{
- int flag;
int null_is_error = 0;
if (verbose)
fprintf(stderr, "Checking HEAD link\n");
- head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid.hash, &flag);
+ head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid.hash, NULL);
if (!head_points_at) {
errors_found |= ERROR_REFS;
return error("Invalid HEAD");
From 17377b62520fc12bd179514c8510d68e25dfb686 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:06 -0400
Subject: [PATCH 19/24] cmd_merge(): remove unneeded flag variable
It is never read, so we can pass NULL to resolve_ref_unsafe().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 101ffeff4c..c90ee51c6c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1165,7 +1165,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit *head_commit;
struct strbuf buf = STRBUF_INIT;
const char *head_arg;
- int flag, i, ret = 0, head_subsumed;
+ int i, ret = 0, head_subsumed;
int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1179,7 +1179,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* Check if we are _not_ on a detached HEAD, i.e. if there is a
* current branch.
*/
- branch = branch_to_free = resolve_refdup("HEAD", 0, head_sha1, &flag);
+ branch = branch_to_free = resolve_refdup("HEAD", 0, head_sha1, NULL);
if (branch && starts_with(branch, "refs/heads/"))
branch += 11;
if (!branch || is_null_sha1(head_sha1))
From be7651a34727d847e150cea564357c37d8a334c9 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:07 -0400
Subject: [PATCH 20/24] checkout_paths(): remove unneeded flag variable
It is never read, so we can pass NULL to resolve_ref_unsafe().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/checkout.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index efcbd8f6b5..ea2fe1cf3f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -242,7 +242,6 @@ static int checkout_paths(const struct checkout_opts *opts,
struct checkout state;
static char *ps_matched;
unsigned char rev[20];
- int flag;
struct commit *head;
int errs = 0;
struct lock_file *lock_file;
@@ -375,7 +374,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
- read_ref_full("HEAD", 0, rev, &flag);
+ read_ref_full("HEAD", 0, rev, NULL);
head = lookup_commit_reference_gently(rev, 1);
errs |= post_checkout_hook(head, head, 0);
From ded83936106a94f854873e949fa7928dce8dbdd1 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:08 -0400
Subject: [PATCH 21/24] check_aliased_update(): check that dst_name is non-NULL
If there is an error in resolve_ref_unsafe(), it returns NULL. We check
for this case, but not until after calling strip_namespace(). Instead,
call strip_namespace() *after* the NULL check.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/receive-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c8e32b297c..49cc88d72f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1081,13 +1081,13 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
if (!(flag & REF_ISSYMREF))
return;
- dst_name = strip_namespace(dst_name);
if (!dst_name) {
rp_error("refusing update to broken symref '%s'", cmd->ref_name);
cmd->skip_update = 1;
cmd->error_string = "broken symref";
return;
}
+ dst_name = strip_namespace(dst_name);
if ((item = string_list_lookup(list, dst_name)) == NULL)
return;
From 7fd12bfbef015bb63f2376f678d71bca54bbc64d Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Thu, 7 Apr 2016 15:03:09 -0400
Subject: [PATCH 22/24] show_head_ref(): check the result of
resolve_ref_namespace()
Only use the result of resolve_ref_namespace() if it is non-NULL.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
http-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 8870a2681e..214881459d 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -484,9 +484,9 @@ static int show_head_ref(const char *refname, const struct object_id *oid,
const char *target = resolve_ref_unsafe(refname,
RESOLVE_REF_READING,
unused.hash, NULL);
- const char *target_nons = strip_namespace(target);
- strbuf_addf(buf, "ref: %s\n", target_nons);
+ if (target)
+ strbuf_addf(buf, "ref: %s\n", strip_namespace(target));
} else {
strbuf_addf(buf, "%s\n", oid_to_hex(oid));
}
From 2d0663b21661f2677bce5742204ccdd73745a062 Mon Sep 17 00:00:00 2001
From: David Turner <dturner@twopensource.com>
Date: Thu, 7 Apr 2016 15:03:10 -0400
Subject: [PATCH 23/24] refs: move resolve_ref_unsafe into common code
Now that resolve_ref_unsafe's only interaction with the backend is
through read_raw_ref, we can move it into the common code. Later,
we'll replace read_raw_ref with a backend function.
Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.c | 74 +++++++++++++++++++++++++++++++++++++++
refs/files-backend.c | 82 ++------------------------------------------
refs/refs-internal.h | 6 ++++
3 files changed, 83 insertions(+), 79 deletions(-)
diff --git a/refs.c b/refs.c
index f0feff72da..87dc82f1d8 100644
--- a/refs.c
+++ b/refs.c
@@ -1155,3 +1155,77 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(NULL, "", fn, 0,
DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
+
+/* This function needs to return a meaningful errno on failure */
+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
+ unsigned char *sha1, int *flags)
+{
+ static struct strbuf sb_refname = STRBUF_INIT;
+ int unused_flags;
+ int symref_count;
+
+ if (!flags)
+ flags = &unused_flags;
+
+ *flags = 0;
+
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+ if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
+ !refname_is_safe(refname)) {
+ errno = EINVAL;
+ return NULL;
+ }
+
+ /*
+ * dwim_ref() uses REF_ISBROKEN to distinguish between
+ * missing refs and refs that were present but invalid,
+ * to complain about the latter to stderr.
+ *
+ * We don't know whether the ref exists, so don't set
+ * REF_ISBROKEN yet.
+ */
+ *flags |= REF_BAD_NAME;
+ }
+
+ for (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {
+ unsigned int read_flags = 0;
+
+ if (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {
+ *flags |= read_flags;
+ if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
+ return NULL;
+ hashclr(sha1);
+ if (*flags & REF_BAD_NAME)
+ *flags |= REF_ISBROKEN;
+ return refname;
+ }
+
+ *flags |= read_flags;
+
+ if (!(read_flags & REF_ISSYMREF)) {
+ if (*flags & REF_BAD_NAME) {
+ hashclr(sha1);
+ *flags |= REF_ISBROKEN;
+ }
+ return refname;
+ }
+
+ refname = sb_refname.buf;
+ if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
+ hashclr(sha1);
+ return refname;
+ }
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+ if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
+ !refname_is_safe(refname)) {
+ errno = EINVAL;
+ return NULL;
+ }
+
+ *flags |= REF_ISBROKEN | REF_BAD_NAME;
+ }
+ }
+
+ errno = ELOOP;
+ return NULL;
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a15986ccc2..71848ab9d6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1269,8 +1269,6 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)
return get_ref_dir(refs->loose);
}
-/* We allow "recursive" symbolic refs. Only within reason, though */
-#define MAXDEPTH 5
#define MAXREFLEN (1024)
/*
@@ -1300,7 +1298,7 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,
char buffer[128], *p;
char *path;
- if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)
+ if (recursion > SYMREF_MAXDEPTH || strlen(refname) > MAXREFLEN)
return -1;
path = *refs->name
? git_pathdup_submodule(refs->name, "%s", refname)
@@ -1420,8 +1418,8 @@ static int resolve_missing_loose_ref(const char *refname,
* - in all other cases, symref will be untouched, and therefore
* refname will still be valid and unchanged.
*/
-static int read_raw_ref(const char *refname, unsigned char *sha1,
- struct strbuf *symref, unsigned int *flags)
+int read_raw_ref(const char *refname, unsigned char *sha1,
+ struct strbuf *symref, unsigned int *flags)
{
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
@@ -1538,80 +1536,6 @@ out:
return ret;
}
-/* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
- unsigned char *sha1, int *flags)
-{
- static struct strbuf sb_refname = STRBUF_INIT;
- int unused_flags;
- int symref_count;
-
- if (!flags)
- flags = &unused_flags;
-
- *flags = 0;
-
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
- !refname_is_safe(refname)) {
- errno = EINVAL;
- return NULL;
- }
-
- /*
- * dwim_ref() uses REF_ISBROKEN to distinguish between
- * missing refs and refs that were present but invalid,
- * to complain about the latter to stderr.
- *
- * We don't know whether the ref exists, so don't set
- * REF_ISBROKEN yet.
- */
- *flags |= REF_BAD_NAME;
- }
-
- for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
- unsigned int read_flags = 0;
-
- if (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {
- *flags |= read_flags;
- if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
- return NULL;
- hashclr(sha1);
- if (*flags & REF_BAD_NAME)
- *flags |= REF_ISBROKEN;
- return refname;
- }
-
- *flags |= read_flags;
-
- if (!(read_flags & REF_ISSYMREF)) {
- if (*flags & REF_BAD_NAME) {
- hashclr(sha1);
- *flags |= REF_ISBROKEN;
- }
- return refname;
- }
-
- refname = sb_refname.buf;
- if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
- hashclr(sha1);
- return refname;
- }
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
- !refname_is_safe(refname)) {
- errno = EINVAL;
- return NULL;
- }
-
- *flags |= REF_ISBROKEN | REF_BAD_NAME;
- }
- }
-
- errno = ELOOP;
- return NULL;
-}
-
/*
* Peel the entry (if possible) and return its new peel_status. If
* repeel is true, re-peel the entry even if there is an old peeled
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 92aae80c21..3a4f634cb4 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -197,6 +197,8 @@ const char *find_descendant_ref(const char *dirname,
int rename_ref_available(const char *oldname, const char *newname);
+/* We allow "recursive" symbolic refs. Only within reason, though */
+#define SYMREF_MAXDEPTH 5
/* Include broken references in a do_for_each_ref*() iteration: */
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
@@ -206,4 +208,8 @@ int rename_ref_available(const char *oldname, const char *newname);
*/
int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn, int trim, int flags, void *cb_data);
+
+int read_raw_ref(const char *refname, unsigned char *sha1,
+ struct strbuf *symref, unsigned int *flags);
+
#endif /* REFS_REFS_INTERNAL_H */
From 41d796ed5cd78d159f8a8bfa3b54bdbe5d9383a3 Mon Sep 17 00:00:00 2001
From: David Turner <dturner@twopensource.com>
Date: Thu, 7 Apr 2016 15:03:11 -0400
Subject: [PATCH 24/24] refs: on symref reflog expire, lock symref not
referrent
When locking a symbolic ref to expire a reflog, lock the symbolic
ref (using REF_NODEREF) instead of its referent.
Add a test for this.
Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs/files-backend.c | 3 ++-
t/t1410-reflog.sh | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 71848ab9d6..5e67bfae68 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3314,7 +3314,8 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
* reference itself, plus we might need to update the
* reference if --updateref was specified:
*/
- lock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, 0, &type, &err);
+ lock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, REF_NODEREF,
+ &type, &err);
if (!lock) {
error("cannot lock ref '%s': %s", refname, err.buf);
strbuf_release(&err);
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index c623824b4d..9cf91dc6d2 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -338,4 +338,14 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
test_line_count = 3 actual
'
+test_expect_success 'reflog expire operates on symref not referrent' '
+ git branch -l the_symref &&
+ git branch -l referrent &&
+ git update-ref referrent HEAD &&
+ git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
+ test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
+ touch .git/refs/heads/referrent.lock &&
+ git reflog expire --expire=all the_symref
+'
+
test_done