From d23fb5c943c3f4accd8b5176db1b78d377bfec54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 12 Mar 2015 01:09:13 +0100 Subject: [PATCH 1/4] clone: add failing test for local transport with a tag When there is a tag, we must make sure that we get all referenced objects from this tag as well. This failing test shows that e.g. when there is a tagged tree, we insert the top tree but do not descend, thus causing the clone to have broken links. --- tests/clone/nonetwork.c | 45 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/clone/nonetwork.c b/tests/clone/nonetwork.c index 2a3157739..e4794fc14 100644 --- a/tests/clone/nonetwork.c +++ b/tests/clone/nonetwork.c @@ -246,6 +246,51 @@ void test_clone_nonetwork__can_detached_head(void) cl_fixture_cleanup("./foo1"); } +void test_clone_nonetwork__clone_tag_to_tree(void) +{ + git_repository *stage; + git_index_entry entry; + git_index *index; + git_odb *odb; + git_oid tree_id; + git_tree *tree; + git_reference *tag; + git_tree_entry *tentry; + const char *file_path = "some/deep/path.txt"; + const char *file_content = "some content\n"; + const char *tag_name = "refs/tags/tree-tag"; + + stage = cl_git_sandbox_init("testrepo.git"); + cl_git_pass(git_repository_odb(&odb, stage)); + cl_git_pass(git_index_new(&index)); + + memset(&entry, 0, sizeof(git_index_entry)); + entry.path = file_path; + entry.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_odb_write(&entry.id, odb, file_content, strlen(file_content), GIT_OBJ_BLOB)); + + cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_write_tree_to(&tree_id, index, stage)); + cl_git_pass(git_reference_create(&tag, stage, tag_name, &tree_id, 0, NULL)); + git_reference_free(tag); + git_odb_free(odb); + git_index_free(index); + + g_options.local = GIT_CLONE_NO_LOCAL; + cl_git_pass(git_clone(&g_repo, cl_git_path_url(git_repository_path(stage)), "./foo", &g_options)); + git_repository_free(stage); + + cl_git_pass(git_reference_lookup(&tag, g_repo, tag_name)); + cl_git_pass(git_tree_lookup(&tree, g_repo, git_reference_target(tag))); + git_reference_free(tag); + + cl_git_pass(git_tree_entry_bypath(&tentry, tree, file_path)); + git_tree_entry_free(tentry); + git_tree_free(tree); + + cl_fixture_cleanup("testrepo.git"); +} + static void assert_correct_reflog(const char *name) { git_reflog *log; From a61fa4c0c70fe2c203bf73d163f9003763520f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 12 Mar 2015 01:26:09 +0100 Subject: [PATCH 2/4] packbuilder: introduce git_packbuilder_insert_recur() This function recursively inserts the given object and any referenced ones. It can be thought of as a more general version of the functions to insert a commit or tree. --- include/git2/pack.h | 12 ++++++++++++ src/pack-objects.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/include/git2/pack.h b/include/git2/pack.h index 4cf426273..4941998eb 100644 --- a/include/git2/pack.h +++ b/include/git2/pack.h @@ -127,6 +127,18 @@ GIT_EXTERN(int) git_packbuilder_insert_commit(git_packbuilder *pb, const git_oid */ GIT_EXTERN(int) git_packbuilder_insert_walk(git_packbuilder *pb, git_revwalk *walk); +/** + * Recursively insert an object and its referenced objects + * + * Insert the object as well as any object it references. + * + * @param pb the packbuilder + * @param id the id of the root object to insert + * @param name optional name for the object + * @return 0 or an error code + */ +GIT_EXTERN(int) git_packbuilder_insert_recur(git_packbuilder *pb, const git_oid *id, const char *name); + /** * Write the contents of the packfile to an in-memory buffer * diff --git a/src/pack-objects.c b/src/pack-objects.c index 0f43b98e0..932764698 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -1404,6 +1404,42 @@ int git_packbuilder_insert_tree(git_packbuilder *pb, const git_oid *oid) return error; } +int git_packbuilder_insert_recur(git_packbuilder *pb, const git_oid *id, const char *name) +{ + git_object *obj; + int error; + + assert(pb && id); + + if ((error = git_object_lookup(&obj, pb->repo, id, GIT_OBJ_ANY)) < 0) + return error; + + switch (git_object_type(obj)) { + case GIT_OBJ_BLOB: + error = git_packbuilder_insert(pb, id, name); + break; + case GIT_OBJ_TREE: + error = git_packbuilder_insert_tree(pb, id); + break; + case GIT_OBJ_COMMIT: + error = git_packbuilder_insert_commit(pb, id); + break; + case GIT_OBJ_TAG: + if ((error = git_packbuilder_insert(pb, id, name)) < 0) + goto cleanup; + error = git_packbuilder_insert_recur(pb, git_tag_target_id((git_tag *) obj), NULL); + break; + + default: + giterr_set(GITERR_INVALID, "unknown object type"); + error = -1; + } + +cleanup: + git_object_free(obj); + return error; +} + uint32_t git_packbuilder_object_count(git_packbuilder *pb) { return pb->nr_objects; From 84511143fd1ec92ee7bc8888141aa2bf2074e9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 12 Mar 2015 01:49:07 +0100 Subject: [PATCH 3/4] tree: add more correct error messages for not found Don't use the full path, as that's not what we are asserting does not exist, but just the subpath we were looking up. --- src/tree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tree.c b/src/tree.c index 47ed20dbb..bdd17661b 100644 --- a/src/tree.c +++ b/src/tree.c @@ -858,7 +858,7 @@ int git_tree_entry_bypath( if (entry == NULL) { giterr_set(GITERR_TREE, - "The path '%s' does not exist in the given tree", path); + "the path '%.*s' does not exist in the given tree", filename_len, path); return GIT_ENOTFOUND; } @@ -868,7 +868,7 @@ int git_tree_entry_bypath( * then this entry *must* be a tree */ if (!git_tree_entry__is_tree(entry)) { giterr_set(GITERR_TREE, - "The path '%s' does not exist in the given tree", path); + "the path '%.*s' exists but is not a tree", filename_len, path); return GIT_ENOTFOUND; } From c84a9dd2da864a975b5dee408e3edaf84422b828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 12 Mar 2015 01:52:15 +0100 Subject: [PATCH 4/4] local: recusrively insert non-branch objects into the packfile When we insert e.g. a tag or tagged object into the packfile, we must make sure to insert any referenced objects as well, or we will have broken links. Use the recursive version of packfile insertion to make sure we send over not just the tagged object but also the objects it references. --- src/transports/local.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/transports/local.c b/src/transports/local.c index 64ddbd970..def8ac037 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -544,7 +544,8 @@ static int local_download_pack( error = 0; } } else { - error = git_packbuilder_insert(pack, &rhead->oid, rhead->name); + /* Tag or some other wanted object. Add it on its own */ + error = git_packbuilder_insert_recur(pack, &rhead->oid, rhead->name); } git_object_free(obj); if (error < 0)