{"thread":{"id":"28267","subject":"[PATCH] sha1_file: Remove relative entries limitation","startedAt":"2011-08-31T06:41:36Z","lastAt":"2011-09-02T16:22:31Z","messageCount":3,"participants":["Hui Wang","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174598","messageId":"1314772896-22631-1-git-send-email-jason77.wang@gmail.com","threadId":"28267","inReplyTo":null,"subject":"[PATCH] sha1_file: Remove relative entries limitation","fromName":"Hui Wang","fromEmail":"jason77.wang@gmail.com","sentAt":"2011-08-31T06:41:36Z","receivedAt":"2011-08-31T06:41:36Z","isPatch":true,"sender":{"key":"jason77.wang@gmail.com","avatar":null},"body":"link_alt_odb_entries() will be called recursively if alternates has\nvalid object store paths, and to avoid nesting too deep, the\nrecursive depth is limited to 5, this limitation is reasonable and\nsafe for dead-loop reference situation.\n\nThere is another limitation in this function to only permit the 1st\nlevel alternates has relative paths, but there is no foreseeable\ngreater risk using relative paths in 2nd/3rd... level alternates than\nusing absolute paths, in addition to we already have max depth 5\nlimitation, we can safely remove this limitation.\n\nMoreover removing this limitation will make below two usage workable.\n\nusage1: base-repos has relative path in the alternates\n        %>git clone --reference base-repos src dest\nusage2: src2 has relative path to point src1, src1 has relative path\n    \tto point src\n        %>git clone src2 dest\n\nSigned-off-by: Hui Wang <jason77.wang@gmail.com>\n---\n sha1_file.c |   13 ++++---------\n 1 files changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f7c3408..4130ca0 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -332,15 +332,10 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,\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) && depth) {\n-\t\t\t\terror(\"%s: ignoring relative alternate object store %s\",\n-\t\t\t\t\t\trelative_base, last);\n-\t\t\t} else {\n-\t\t\t\tlink_alt_odb_entry(last, cp - last,\n-\t\t\t\t\t\trelative_base, depth);\n-\t\t\t}\n-\t\t}\n+\t\tif (last != cp)\n+\t\t\tlink_alt_odb_entry(last, cp - last,\n+\t\t\t\t\trelative_base, depth);\n+\n \t\twhile (cp < ep && *cp == sep)\n \t\t\tcp++;\n \t\tlast = cp;\n-- \n1.7.6\n"},{"id":"174740","messageId":"4E60A699.9080302@gmail.com","threadId":"28267","inReplyTo":"1314772896-22631-1-git-send-email-jason77.wang@gmail.com","subject":"Re: [PATCH] sha1_file: Remove relative entries limitation","fromName":"Hui Wang","fromEmail":"jason77.wang@gmail.com","sentAt":"2011-09-02T09:49:13Z","receivedAt":"2011-09-02T09:49:13Z","isPatch":true,"sender":{"key":"jason77.wang@gmail.com","avatar":null},"body":"Hi Junio,\n\nDoes this patch make sense?\n\nRegards,\nHui Wang.\n\nHui Wang wrote:\n> link_alt_odb_entries() will be called recursively if alternates has\n> valid object store paths, and to avoid nesting too deep, the\n> recursive depth is limited to 5, this limitation is reasonable and\n> safe for dead-loop reference situation.\n>\n> There is another limitation in this function to only permit the 1st\n> level alternates has relative paths, but there is no foreseeable\n> greater risk using relative paths in 2nd/3rd... level alternates than\n> using absolute paths, in addition to we already have max depth 5\n> limitation, we can safely remove this limitation.\n>\n> Moreover removing this limitation will make below two usage workable.\n>\n> usage1: base-repos has relative path in the alternates\n>         %>git clone --reference base-repos src dest\n> usage2: src2 has relative path to point src1, src1 has relative path\n>     \tto point src\n>         %>git clone src2 dest\n>\n> Signed-off-by: Hui Wang <jason77.wang@gmail.com>\n> ---\n>  sha1_file.c |   13 ++++---------\n>  1 files changed, 4 insertions(+), 9 deletions(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index f7c3408..4130ca0 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -332,15 +332,10 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,\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) && depth) {\n> -\t\t\t\terror(\"%s: ignoring relative alternate object store %s\",\n> -\t\t\t\t\t\trelative_base, last);\n> -\t\t\t} else {\n> -\t\t\t\tlink_alt_odb_entry(last, cp - last,\n> -\t\t\t\t\t\trelative_base, depth);\n> -\t\t\t}\n> -\t\t}\n> +\t\tif (last != cp)\n> +\t\t\tlink_alt_odb_entry(last, cp - last,\n> +\t\t\t\t\trelative_base, depth);\n> +\n>  \t\twhile (cp < ep && *cp == sep)\n>  \t\t\tcp++;\n>  \t\tlast = cp;\n>   \n"},{"id":"174754","messageId":"7vzkimlxrs.fsf@alter.siamese.dyndns.org","threadId":"28267","inReplyTo":"4E60A699.9080302@gmail.com","subject":"Re: [PATCH] sha1_file: Remove relative entries limitation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-02T16:22:31Z","receivedAt":"2011-09-02T16:22:31Z","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> Does this patch make sense?\n\nI do not see anything wrong with it per-se, but I'd like to see somebody\nmade an effort to dig why the restriction is there in the first place. The\ncheck is very much deliberately placed there and there may be a good\nreason why we do not resolve relatives of indirect alternates that you are\nmissing when you wrote the patch (and I am missing when I wrote this\nresponse).\n"}]}