From 7edb9071da8e78e8cf9aff969f1b8137bca3c33d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 May 2013 11:07:20 -0400 Subject: [PATCH 1/6] refdb_fs: do not require peeled packed refs to be tags Older versions of git would only write peeled entries for items under refs/tags/. Newer versions will write them for all refs, and we should be prepared to handle that. --- src/refdb_fs.c | 4 --- tests-clar/refs/peel.c | 31 ++++++++++++++++-- tests-clar/resources/peeled.git/HEAD | 1 + tests-clar/resources/peeled.git/config | 8 +++++ .../resources/peeled.git/objects/info/packs | 2 ++ ...4773eaf3fce1774755580e3dbb8d9f3a1adc45.idx | Bin 0 -> 1156 bytes ...773eaf3fce1774755580e3dbb8d9f3a1adc45.pack | Bin 0 -> 274 bytes tests-clar/resources/peeled.git/packed-refs | 6 ++++ .../resources/peeled.git/refs/heads/master | 1 + 9 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 tests-clar/resources/peeled.git/HEAD create mode 100644 tests-clar/resources/peeled.git/config create mode 100644 tests-clar/resources/peeled.git/objects/info/packs create mode 100644 tests-clar/resources/peeled.git/objects/pack/pack-e84773eaf3fce1774755580e3dbb8d9f3a1adc45.idx create mode 100644 tests-clar/resources/peeled.git/objects/pack/pack-e84773eaf3fce1774755580e3dbb8d9f3a1adc45.pack create mode 100644 tests-clar/resources/peeled.git/packed-refs create mode 100644 tests-clar/resources/peeled.git/refs/heads/master diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 85444b692..9ee3568da 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -132,10 +132,6 @@ static int packed_parse_peel( if (tag_ref == NULL) goto corrupt; - /* Ensure reference is a tag */ - if (git__prefixcmp(tag_ref->name, GIT_REFS_TAGS_DIR) != 0) - goto corrupt; - if (buffer + GIT_OID_HEXSZ > buffer_end) goto corrupt; diff --git a/tests-clar/refs/peel.c b/tests-clar/refs/peel.c index 34bd02ce0..f2fb6e259 100644 --- a/tests-clar/refs/peel.c +++ b/tests-clar/refs/peel.c @@ -1,19 +1,24 @@ #include "clar_libgit2.h" static git_repository *g_repo; +static git_repository *g_peel_repo; void test_refs_peel__initialize(void) { cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git"))); + cl_git_pass(git_repository_open(&g_peel_repo, cl_fixture("peeled.git"))); } void test_refs_peel__cleanup(void) { git_repository_free(g_repo); g_repo = NULL; + git_repository_free(g_peel_repo); + g_peel_repo = NULL; } -static void assert_peel( +static void assert_peel_generic( + git_repository *repo, const char *ref_name, git_otype requested_type, const char* expected_sha, @@ -23,7 +28,7 @@ static void assert_peel( git_reference *ref; git_object *peeled; - cl_git_pass(git_reference_lookup(&ref, g_repo, ref_name)); + cl_git_pass(git_reference_lookup(&ref, repo, ref_name)); cl_git_pass(git_reference_peel(&peeled, ref, requested_type)); @@ -36,6 +41,16 @@ static void assert_peel( git_reference_free(ref); } +static void assert_peel( + const char *ref_name, + git_otype requested_type, + const char* expected_sha, + git_otype expected_type) +{ + assert_peel_generic(g_repo, ref_name, requested_type, + expected_sha, expected_type); +} + static void assert_peel_error(int error, const char *ref_name, git_otype requested_type) { git_reference *ref; @@ -90,3 +105,15 @@ void test_refs_peel__can_peel_into_any_non_tag_object(void) assert_peel("refs/tags/test", GIT_OBJ_ANY, "e90810b8df3e80c413d903f631643c716887138d", GIT_OBJ_COMMIT); } + +void test_refs_peel__can_peel_fully_peeled_packed_refs(void) +{ + assert_peel_generic(g_peel_repo, + "refs/tags/tag-inside-tags", GIT_OBJ_ANY, + "0df1a5865c8abfc09f1f2182e6a31be550e99f07", + GIT_OBJ_COMMIT); + assert_peel_generic(g_peel_repo, + "refs/foo/tag-outside-tags", GIT_OBJ_ANY, + "0df1a5865c8abfc09f1f2182e6a31be550e99f07", + GIT_OBJ_COMMIT); +} diff --git a/tests-clar/resources/peeled.git/HEAD b/tests-clar/resources/peeled.git/HEAD new file mode 100644 index 000000000..cb089cd89 --- /dev/null +++ b/tests-clar/resources/peeled.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests-clar/resources/peeled.git/config b/tests-clar/resources/peeled.git/config new file mode 100644 index 000000000..88300524a --- /dev/null +++ b/tests-clar/resources/peeled.git/config @@ -0,0 +1,8 @@ +[core] + repositoryformatversion = 0 + filemode = true + bare = true +[remote "origin"] + url = /home/peff/compile/libgit2/tests-clar/resources/peeled + fetch = +refs/*:refs/* + mirror = true diff --git a/tests-clar/resources/peeled.git/objects/info/packs b/tests-clar/resources/peeled.git/objects/info/packs new file mode 100644 index 000000000..0d88b32e5 --- /dev/null +++ b/tests-clar/resources/peeled.git/objects/info/packs @@ -0,0 +1,2 @@ +P pack-e84773eaf3fce1774755580e3dbb8d9f3a1adc45.pack + diff --git a/tests-clar/resources/peeled.git/objects/pack/pack-e84773eaf3fce1774755580e3dbb8d9f3a1adc45.idx b/tests-clar/resources/peeled.git/objects/pack/pack-e84773eaf3fce1774755580e3dbb8d9f3a1adc45.idx new file mode 100644 index 0000000000000000000000000000000000000000..9b79e9b85f879d05623c23d8f2c6d73d6a70e807 GIT binary patch literal 1156 zcmexg;-AdGz`z8=r!Y~W~(S1(g3xJ#!=OZD+ z8cyM)c2uM$Rpkx0hi>N0C?BZ?xTt4JfokHS6|;Jse3~Ti?28bgta1e*{&H-gP8_(VeRdm99oB#j- literal 0 HcmV?d00001 diff --git a/tests-clar/resources/peeled.git/packed-refs b/tests-clar/resources/peeled.git/packed-refs new file mode 100644 index 000000000..ad053d550 --- /dev/null +++ b/tests-clar/resources/peeled.git/packed-refs @@ -0,0 +1,6 @@ +# pack-refs with: peeled fully-peeled +c2596aa0151888587ec5c0187f261e63412d9e11 refs/foo/tag-outside-tags +^0df1a5865c8abfc09f1f2182e6a31be550e99f07 +0df1a5865c8abfc09f1f2182e6a31be550e99f07 refs/heads/master +c2596aa0151888587ec5c0187f261e63412d9e11 refs/tags/tag-inside-tags +^0df1a5865c8abfc09f1f2182e6a31be550e99f07 diff --git a/tests-clar/resources/peeled.git/refs/heads/master b/tests-clar/resources/peeled.git/refs/heads/master new file mode 100644 index 000000000..76c15e203 --- /dev/null +++ b/tests-clar/resources/peeled.git/refs/heads/master @@ -0,0 +1 @@ +0df1a5865c8abfc09f1f2182e6a31be550e99f07 From 3bb00f3360bd11a48e1b04dc7dec971f0019891f Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 2 May 2013 17:17:46 +0200 Subject: [PATCH 2/6] refdb_fs: implement the fully-peeled trait --- src/refdb_fs.c | 7 ------- src/refs.h | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 9ee3568da..8a2d56327 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -677,13 +677,6 @@ static int packed_find_peel(refdb_fs_backend *backend, struct packref *ref) if (ref->flags & GIT_PACKREF_HAS_PEEL) return 0; - /* - * Only applies to tags, i.e. references - * in the /refs/tags folder - */ - if (git__prefixcmp(ref->name, GIT_REFS_TAGS_DIR) != 0) - return 0; - /* * Find the tagged object in the repository */ diff --git a/src/refs.h b/src/refs.h index 908e86f29..927bc83ce 100644 --- a/src/refs.h +++ b/src/refs.h @@ -26,7 +26,7 @@ #define GIT_SYMREF "ref: " #define GIT_PACKEDREFS_FILE "packed-refs" -#define GIT_PACKEDREFS_HEADER "# pack-refs with: peeled " +#define GIT_PACKEDREFS_HEADER "# pack-refs with: peeled fully-peeled" #define GIT_PACKEDREFS_FILE_MODE 0666 #define GIT_HEAD_FILE "HEAD" From f69db390fb5cbacbb1c63d146aef4e0fb6754ddf Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 2 May 2013 17:29:58 +0200 Subject: [PATCH 3/6] refdb_fs: store "cannot be peeled" flag for packed refs Fixes #1532 --- src/refdb_fs.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 8a2d56327..00d1c4fd5 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -26,8 +26,15 @@ GIT__USE_STRMAP; #define MAX_NESTING_LEVEL 10 enum { - GIT_PACKREF_HAS_PEEL = 1, - GIT_PACKREF_WAS_LOOSE = 2 + PACKREF_HAS_PEEL = 1, + PACKREF_WAS_LOOSE = 2, + PACKREF_CANNOT_PEEL = 4 +}; + +enum { + PEELING_NONE = 0, + PEELING_STANDARD, + PEELING_FULL }; struct packref { @@ -44,6 +51,7 @@ typedef struct refdb_fs_backend { char *path; git_refcache refcache; + int peeling_mode; } refdb_fs_backend; static int reference_read( @@ -150,6 +158,7 @@ static int packed_parse_peel( goto corrupt; } + tag_ref->flags |= PACKREF_HAS_PEEL; *buffer_out = buffer; return 0; @@ -201,6 +210,25 @@ static int packed_load(refdb_fs_backend *backend) buffer_start = (const char *)packfile.ptr; buffer_end = (const char *)(buffer_start) + packfile.size; + backend->peeling_mode = PEELING_NONE; + + if (buffer_start[0] == '#') { + static const char *traits_header = "# pack-refs with: "; + + if (git__prefixcmp(buffer_start, traits_header) == 0) { + const char *traits = buffer_start + strlen(traits_header); + const char *traits_end = strchr(traits, '\n'); + + if (strstr(traits, "fully-peeled") != NULL) { + backend->peeling_mode = PEELING_FULL; + } else if (strstr(traits, "peeled") != NULL) { + backend->peeling_mode = PEELING_STANDARD; + } + + buffer_start = traits_end + 1; + } + } + while (buffer_start < buffer_end && buffer_start[0] == '#') { buffer_start = strchr(buffer_start, '\n'); if (buffer_start == NULL) @@ -219,6 +247,8 @@ static int packed_load(refdb_fs_backend *backend) if (buffer_start[0] == '^') { if (packed_parse_peel(ref, &buffer_start, buffer_end) < 0) goto parse_failed; + } else if (backend->peeling_mode == PEELING_FULL) { + ref->flags |= PACKREF_CANNOT_PEEL; } git_strmap_insert(ref_cache->packfile, ref->name, ref, err); @@ -291,7 +321,7 @@ static int loose_lookup_to_packfile( return -1; } - ref->flags = GIT_PACKREF_WAS_LOOSE; + ref->flags = PACKREF_WAS_LOOSE; *ref_out = ref; git_buf_free(&ref_file); @@ -674,7 +704,7 @@ static int packed_find_peel(refdb_fs_backend *backend, struct packref *ref) { git_object *object; - if (ref->flags & GIT_PACKREF_HAS_PEEL) + if (ref->flags & PACKREF_HAS_PEEL || ref->flags & PACKREF_CANNOT_PEEL) return 0; /* @@ -695,7 +725,7 @@ static int packed_find_peel(refdb_fs_backend *backend, struct packref *ref) * Find the object pointed at by this tag */ git_oid_cpy(&ref->peel, git_tag_target_id(tag)); - ref->flags |= GIT_PACKREF_HAS_PEEL; + ref->flags |= PACKREF_HAS_PEEL; /* * The reference has now cached the resolved OID, and is @@ -728,7 +758,7 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) * This obviously only applies to tags. * The required peels have already been loaded into `ref->peel_target`. */ - if (ref->flags & GIT_PACKREF_HAS_PEEL) { + if (ref->flags & PACKREF_HAS_PEEL) { char peel[GIT_OID_HEXSZ + 1]; git_oid_fmt(peel, &ref->peel); peel[GIT_OID_HEXSZ] = 0; @@ -765,7 +795,7 @@ static int packed_remove_loose( for (i = 0; i < packing_list->length; ++i) { struct packref *ref = git_vector_get(packing_list, i); - if ((ref->flags & GIT_PACKREF_WAS_LOOSE) == 0) + if ((ref->flags & PACKREF_WAS_LOOSE) == 0) continue; if (git_buf_joinpath(&full_path, backend->path, ref->name) < 0) From 1022db2b6860d602e79169906eee4f299855975b Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 2 May 2013 17:42:09 +0200 Subject: [PATCH 4/6] refdb_fs: Traits are always surrounded by spaces This makes parsing easier! :p --- src/refdb_fs.c | 4 ++-- src/refs.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 00d1c4fd5..6a6f589f0 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -219,9 +219,9 @@ static int packed_load(refdb_fs_backend *backend) const char *traits = buffer_start + strlen(traits_header); const char *traits_end = strchr(traits, '\n'); - if (strstr(traits, "fully-peeled") != NULL) { + if (strstr(traits, " fully-peeled ") != NULL) { backend->peeling_mode = PEELING_FULL; - } else if (strstr(traits, "peeled") != NULL) { + } else if (strstr(traits, " peeled ") != NULL) { backend->peeling_mode = PEELING_STANDARD; } diff --git a/src/refs.h b/src/refs.h index 927bc83ce..f487ee3fc 100644 --- a/src/refs.h +++ b/src/refs.h @@ -26,7 +26,7 @@ #define GIT_SYMREF "ref: " #define GIT_PACKEDREFS_FILE "packed-refs" -#define GIT_PACKEDREFS_HEADER "# pack-refs with: peeled fully-peeled" +#define GIT_PACKEDREFS_HEADER "# pack-refs with: peeled fully-peeled " #define GIT_PACKEDREFS_FILE_MODE 0666 #define GIT_HEAD_FILE "HEAD" From 822645f6298ae0ff86fa717a79c5b7e105bc4a0d Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 2 May 2013 17:48:49 +0200 Subject: [PATCH 5/6] refdb_fs: Only strstr the traits line --- src/refdb_fs.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 6a6f589f0..2c45eabb7 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -216,8 +216,13 @@ static int packed_load(refdb_fs_backend *backend) static const char *traits_header = "# pack-refs with: "; if (git__prefixcmp(buffer_start, traits_header) == 0) { - const char *traits = buffer_start + strlen(traits_header); - const char *traits_end = strchr(traits, '\n'); + char *traits = (char *)buffer_start + strlen(traits_header); + char *traits_end = strchr(traits, '\n'); + + if (traits_end == NULL) + goto parse_failed; + + *traits_end = '\0'; if (strstr(traits, " fully-peeled ") != NULL) { backend->peeling_mode = PEELING_FULL; From a591ed3ea9e46771510628f1f677f2f3791078d6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 May 2013 12:06:46 -0400 Subject: [PATCH 6/6] refdb_fs: respect PEELING_STANDARD We only set our negative flag for PEELING_FULL; we can fall back to the lesser PEELING_STANDARD if our ref is in the refs/tags/ hierarchy. --- src/refdb_fs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 2c45eabb7..c0a32bae7 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -252,7 +252,9 @@ static int packed_load(refdb_fs_backend *backend) if (buffer_start[0] == '^') { if (packed_parse_peel(ref, &buffer_start, buffer_end) < 0) goto parse_failed; - } else if (backend->peeling_mode == PEELING_FULL) { + } else if (backend->peeling_mode == PEELING_FULL || + (backend->peeling_mode == PEELING_STANDARD && + git__prefixcmp(ref->name, GIT_REFS_TAGS_DIR) == 0)) { ref->flags |= PACKREF_CANNOT_PEEL; }