{"thread":{"id":"28161","subject":"[PATCH 1/1] clone: replace relative paths in the alternates","startedAt":"2011-08-19T05:23:15Z","lastAt":"2011-08-19T06:39:23Z","messageCount":4,"participants":["Hui Wang","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"173833","messageId":"1313731396-10458-1-git-send-email-jason77.wang@gmail.com","threadId":"28161","inReplyTo":null,"subject":"[PATCH 0/1] git-clone: fix relative path problem in the alternates","fromName":"Hui Wang","fromEmail":"jason77.wang@gmail.com","sentAt":"2011-08-19T05:23:15Z","receivedAt":"2011-08-19T05:23:15Z","isPatch":true,"sender":{"key":"jason77.wang@gmail.com","avatar":null},"body":"Hi Junio & everyone,\n\nSeveral days ago, i reported a \"git clone\" issue, the url is at:\nhttp://marc.info/?l=git&m=131219022632724&w=2\n\nHere i describe the problem again and try to provide a fix for this\nproblem, please help to review it. Thanks.\n\nThe problem:\nSuppose we have two git repositories on the local machine, one is\ncalled base repository, another is called source repository, source\nrepository need refer to some objects of the base repository via\n.git/objects/info/alternates. If we set a relative path (source repository\nto base repository) in the alternates, we can execute any git commands (branch,\ntag, pull, checkout...) successfully in the source repository. But if we\nwant to clone a destiantion repository from source repository without\n\"--shared\" on the same machine, just like this:\n%>git clone $local_path/src_repos $local_path/des_repos\nThis clone command will fail because git can't get objects of the base\nrepository.\n\nThe reason:\nWhen we execute git clone $local_path/src_repos $local_path/des_repos,\ngit will copy alternates of the src_repos to the des_repos first, then\ndes_repos will use this alternates to refer to base repository, this is\nthe problem, the relative in the alternates is src_repos to base_repos\nrather than des_repos to base_repos.\n\nThe fix:\nAfter the alternates is copied from src_repos to des_repos and before this\nfile is used by the des_repos, we can parse it and replace all relative\npaths to a corrected path (absolute path of base_repos or new relative path\nof des_repos to base_repos). In my patch, i choose to replace relative paths\nto absoulte paths of base_repos.\n\nIf we add \"--shared\", this problem doesn't exist becase git doesn't need to\ncopy alternates, if we add prefix before src_repos like \"file://\", \"git://\"...,\nthis problem also doesn't exist. So i add a function to replace relative paths\nin the clone_local(), this only affect git clone $local_path/src_repos\n$local_path/des_repos without \"--shared\" and it has replative paths in the\nalternates of src_repos, for other situation, this function will not introduce\nany changes. Below is a execution result example of my fix:\n\nthe content of the alternates in the source repos (4 lines):\n$<relative_path1_source_repos_to_base1>\n$<relative_path2_source_repos_to_base2>\n# comment line\n$<absolute_path_of_base3>\n\nAfter clone from source repos, the content of the alternates in the des_repos (4 lines):\n$<absolute_path_of_base1>\n$<absolute_path_of_base2>\n# comment line\n$<absolute_path_of_base3> \n"},{"id":"173832","messageId":"1313731396-10458-2-git-send-email-jason77.wang@gmail.com","threadId":"28161","inReplyTo":"1313731396-10458-1-git-send-email-jason77.wang@gmail.com","subject":"[PATCH 1/1] clone: replace relative paths in the alternates","fromName":"Hui Wang","fromEmail":"jason77.wang@gmail.com","sentAt":"2011-08-19T05:23:16Z","receivedAt":"2011-08-19T05:23:16Z","isPatch":true,"sender":{"key":"jason77.wang@gmail.com","avatar":null},"body":"When we clone a local source repository to a new local destination\nrepository without \"--shared\" option, if source repository has\nrelative path in the alternates, the clone will fail.\n\nThe root cause is when cloning local source repository to local\ndestination repository, the alternates of the source repository\nwill be copied to the destination repository, the relative path in\nthe alternates is source repository to base repository rather than\ndestination repository to base repository, so the destination\nrepository get a wrong relative path in the alternates, this will\ncause errors when refer to base repository in the destination\nrepository.\n\nTo fix it, adding a function update_alt_rel_path() to replace all\nrelative paths to absolute paths in the alternates of the destination\nrepository before destination repository use those paths to look for\nthe base repository.\n\nupdate_alt_rel_path() is referred to link_alt_odb_entries() in the\nsha1_file.c, update_alternates_file() is referred to\nadd_to_alternates_file() in the sha1_file.c.\n\nSigned-off-by: Hui Wang <jason77.wang@gmail.com>\n---\n builtin/clone.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n cache.h         |    1 +\n sha1_file.c     |   11 +++++++\n 3 files changed, 95 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 7663bc2..65c1eaf 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -286,6 +286,87 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)\n \tclosedir(dir);\n }\n \n+#define linesize(end, start) (end - start + 1)\n+/*\n+ * This function is called by clone_local() of clone.c\n+ * If alternates in the src_repo (source repository) has relative paths,\n+ * call this function will replace the relative paths to absolute paths\n+ * in the des_repo (destination repository) alternates.\n+ */\n+static void update_alt_rel_path(const char *src_repo)\n+{\n+\tvoid *map;\n+\tsize_t mapsz;\n+\tchar *cp, *last, *ep, sep = '\\n';\n+\tint fd, buf_start = 0, needwrite = 0;\n+\tchar *tmpbuf;\n+\tstruct stat st;\n+\n+\tfd = open(git_path(\"objects/info/alternates\"), O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn;\n+\n+\tif (fstat(fd, &st) || (st.st_size == 0)) {\n+\t\tclose(fd);\n+\t\treturn;\n+\t}\n+\n+\tmapsz = xsize_t(st.st_size);\n+\tmap = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);\n+\tclose(fd);\n+\n+\ttmpbuf = mapsz > 4096 ? xmalloc(mapsz * 2) : xmalloc(mapsz + 4096);\n+\n+\tep = map + mapsz;\n+\tlast = map;\n+\tbuf_start = 0;\n+\twhile (last < ep) {\n+\t\tcp = last;\n+\t\tif (cp < ep && *cp == '#') {\n+\t\t\twhile (cp < ep && *cp != sep)\n+\t\t\t\tcp++;\n+\t\t\tmemcpy(tmpbuf + buf_start, last, linesize(cp, last));\n+\t\t\tbuf_start += linesize(cp, last);\n+\t\t\tlast = cp + 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\twhile (cp < ep && *cp != sep)\n+\t\t\tcp++;\n+\t\tif (last != cp) {\n+\t\t\tif (!is_absolute_path(last)) {\n+\t\t\t\tchar *abs_path, *abs_buf;\n+\t\t\t\tint len;\n+\n+\t\t\t\tneedwrite = 1;\n+\t\t\t\tmemcpy(tmpbuf + buf_start, last, linesize(cp, last));\n+\t\t\t\ttmpbuf[buf_start + linesize(cp, last)] = '\\0';\n+\t\t\t\tabs_path = mkpath(\"%s/objects/%s\", src_repo, tmpbuf + buf_start);\n+\t\t\t\tabs_buf = xmalloc(PATH_MAX);\n+\t\t\t\tnormalize_path_copy(abs_buf, abs_path);\n+\t\t\t\tlen = strlen(abs_buf);\n+\t\t\t\tmemcpy(tmpbuf + buf_start, abs_buf, len);\n+\t\t\t\tfree(abs_buf);\n+\t\t\t\tbuf_start += len;\n+\t\t\t} else {\n+\t\t\t\tmemcpy(tmpbuf + buf_start, last, linesize(cp, last));\n+\t\t\t\tbuf_start += linesize(cp, last);\n+\t\t\t}\n+\t\t}\n+\t\twhile (cp < ep && *cp == sep)\n+\t\t\tcp++;\n+\t\tlast = cp;\n+\t}\n+\ttmpbuf[buf_start] = '\\0';\n+\n+\tif (needwrite)\n+\t\tupdate_alternates_file_content(tmpbuf);\n+\n+\tmunmap(map, mapsz);\n+\tfree(tmpbuf);\n+\n+\treturn;\n+}\n+\n static const struct ref *clone_local(const char *src_repo,\n \t\t\t\t     const char *dest_repo)\n {\n@@ -303,8 +384,9 @@ static const struct ref *clone_local(const char *src_repo,\n \t\tcopy_or_link_directory(&src, &dest);\n \t\tstrbuf_release(&src);\n \t\tstrbuf_release(&dest);\n-\t}\n \n+\t\tupdate_alt_rel_path(src_repo);\n+\t}\n \tremote = remote_get(src_repo);\n \ttransport = transport_get(remote, src_repo);\n \tret = transport_get_remote_refs(transport);\ndiff --git a/cache.h b/cache.h\nindex fcf4501..d81c0f0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -913,6 +913,7 @@ extern struct alternate_object_database {\n } *alt_odb_list;\n extern void prepare_alt_odb(void);\n extern void add_to_alternates_file(const char *reference);\n+extern void update_alternates_file_content(const char *content);\n typedef int alt_odb_fn(struct alternate_object_database *, void *);\n extern void foreach_alt_odb(alt_odb_fn, void*);\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex d5616dc..dae7bf6 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -388,6 +388,17 @@ void add_to_alternates_file(const char *reference)\n \t\tlink_alt_odb_entries(alt, alt + strlen(alt), '\\n', NULL, 0);\n }\n \n+void update_alternates_file_content(const char *content)\n+{\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tint fd = hold_lock_file_for_update(lock, git_path(\"objects/info/alternates\"), LOCK_DIE_ON_ERROR);\n+\twrite_or_die(fd, content, strlen(content));\n+\tif (commit_lock_file(lock))\n+\t\tdie(\"could not close alternates file\");\n+\tif (alt_odb_tail)\n+\t\tlink_alt_odb_entries(content, content + strlen(content), '\\n', NULL, 0);\n+}\n+\n void foreach_alt_odb(alt_odb_fn fn, void *cb)\n {\n \tstruct alternate_object_database *ent;\n-- \n1.7.6\n"},{"id":"173838","messageId":"7vy5yp53aj.fsf@alter.siamese.dyndns.org","threadId":"28161","inReplyTo":"1313731396-10458-2-git-send-email-jason77.wang@gmail.com","subject":"Re: [PATCH 1/1] clone: replace relative paths in the alternates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-19T06:22:12Z","receivedAt":"2011-08-19T06:22:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hui Wang <jason77.wang@gmail.com> writes:\n\n> +\tmapsz = xsize_t(st.st_size);\n> +\tmap = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);\n> +\tclose(fd);\n> +\n> +\ttmpbuf = mapsz > 4096 ? xmalloc(mapsz * 2) : xmalloc(mapsz + 4096);\n\nWhere do these magic numbers come from, and what guarantee do we have that\nthese magic numbers give safe upper bound for pathnames that are expanded\nto be absolute?\n\nWouldn't it be a lot cleaner and much less error prone if you open the\noriginal, read from it with strbuf_readline(), convert the path using\nstrbuf manipulation functions and write the resulting line out to the\nlockfile you obtained to update it, line by line?\n"},{"id":"173839","messageId":"4E4E051B.5020803@gmail.com","threadId":"28161","inReplyTo":"7vy5yp53aj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/1] clone: replace relative paths in the alternates","fromName":"Hui Wang","fromEmail":"jason77.wang@gmail.com","sentAt":"2011-08-19T06:39:23Z","receivedAt":"2011-08-19T06:39:23Z","isPatch":true,"sender":{"key":"jason77.wang@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> Hui Wang <jason77.wang@gmail.com> writes:\n>\n>   \n>> +\tmapsz = xsize_t(st.st_size);\n>> +\tmap = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);\n>> +\tclose(fd);\n>> +\n>> +\ttmpbuf = mapsz > 4096 ? xmalloc(mapsz * 2) : xmalloc(mapsz + 4096);\n>>     \n>\n> Where do these magic numbers come from, and what guarantee do we have that\n> these magic numbers give safe upper bound for pathnames that are expanded\n> to be absolute?\n>\n>   \nIf an alternates file has dozens of relative path lines, this code has \ngreat risk.\n> Wouldn't it be a lot cleaner and much less error prone if you open the\n> original, read from it with strbuf_readline(), convert the path using\n> strbuf manipulation functions and write the resulting line out to the\n> lockfile you obtained to update it, line by line?\n>   \nThis is a good suggestion, i will try to use the above way to prepare a \nV2 patch.\n\nThanks,\nJason.\n"}]}