{"thread":{"id":"34669","subject":"[PATCH] push: respect --no-thin","startedAt":"2013-08-10T10:10:07Z","lastAt":"2013-08-13T16:43:31Z","messageCount":10,"participants":["Nguyễn Thái Ngọc Duy","Jeff King","Duy Nguyen","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225009","messageId":"1376129407-30590-1-git-send-email-pclouds@gmail.com","threadId":"34669","inReplyTo":null,"subject":"[PATCH] push: respect --no-thin","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-10T10:10:07Z","receivedAt":"2013-08-10T10:10:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Over the time the default value for --thin has been switched between\non and off. As of now it's always on, even if --no-thin is given.\nCorrect the code to respect --no-thin.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/push.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 04f0eaf..333a1fb 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n \tNULL,\n };\n \n-static int thin;\n+static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n@@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (receivepack)\n \t\ttransport_set_option(transport,\n \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n-\tif (thin)\n-\t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n+\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n \n \tif (verbosity > 0)\n \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\n-- \n1.8.2.83.gc99314b\n"},{"id":"225012","messageId":"20130810103123.GB6237@sigill.intra.peff.net","threadId":"34669","inReplyTo":"1376129407-30590-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] push: respect --no-thin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-10T10:31:23Z","receivedAt":"2013-08-10T10:31:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 10, 2013 at 05:10:07PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> Over the time the default value for --thin has been switched between\n> on and off. As of now it's always on, even if --no-thin is given.\n> Correct the code to respect --no-thin.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nThis makes sense, and I confirmed with the script I posted earlier that\nit does work (and didn't before). Thanks.\n\nIt might be nice to have a test, but I am not sure of a good way to\ncheck whether a push was thin. I guess we can grep the output of\nGIT_TRACE, though it feels a bit hacky.\n\n-Peff\n"},{"id":"225014","messageId":"20130810114515.GA14027@lanh","threadId":"34669","inReplyTo":"20130810103123.GB6237@sigill.intra.peff.net","subject":"Re: [PATCH] push: respect --no-thin","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-10T11:45:15Z","receivedAt":"2013-08-10T11:45:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Aug 10, 2013 at 06:31:23AM -0400, Jeff King wrote:\n> It might be nice to have a test, but I am not sure of a good way to\n> check whether a push was thin. I guess we can grep the output of\n> GIT_TRACE, though it feels a bit hacky.\n\nIn theory, if we have a way to tell receive-pack not to pass\n--fix-thin to index-pack, then it should barf on the thin pack\nreceived from push/send-pack. But it does not work this way, or at\nleast I set up the test the wrong way, or pack-objects decides not to\ncreate deltas.. anybody sees what's wrong in this test?\n\n-- 8< --\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..da60817 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -38,6 +38,7 @@ static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n+static int fix_thin = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n@@ -869,7 +870,8 @@ static const char *unpack(int err_fd)\n \t\tkeeper[i++] = \"--stdin\";\n \t\tif (fsck_objects)\n \t\t\tkeeper[i++] = \"--strict\";\n-\t\tkeeper[i++] = \"--fix-thin\";\n+\t\tif (fix_thin)\n+\t\t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n \t\tkeeper[i++] = keep_arg;\n \t\tkeeper[i++] = NULL;\n@@ -975,6 +977,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tstateless_rpc = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--no-thin\")) {\n+\t\t\t\tfix_thin = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\tusage(receive_pack_usage);\n \t\t}\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4691d51..9c9d43c 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -126,6 +126,16 @@ test_expect_success setup '\n \n '\n \n+test_expect_success 'push --no-thin' '\n+\tgit init no-thin &&\n+\tgit --git-dir=no-thin/.git config receive.unpacklimit 0 &&\n+\tgit push no-thin/.git refs/heads/master:refs/heads/foo &&\n+\techo modified >> path1 &&\n+\tgit commit -am modified &&\n+\tgit push --no-thin --receive-pack=\"git receive-pack --no-thin\" no-thin/.git refs/heads/master:refs/heads/foo\n+\tfalse\n+'\n+\n test_expect_success 'fetch without wildcard' '\n \tmk_empty testrepo &&\n \t(\n-- 8< --\n"},{"id":"225038","messageId":"1376184258-6784-1-git-send-email-pclouds@gmail.com","threadId":"34669","inReplyTo":"1376129407-30590-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] push: respect --no-thin","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-11T01:24:18Z","receivedAt":"2013-08-11T01:24:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Over the time the default value for --thin has been switched between\non and off. As of now it's always on, even if --no-thin is given.\nCorrect the code to respect --no-thin.\n\nreceive-pack learns about --no-thin only for testing purposes, hence\nno document update.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v2 adds a test for push --no-thin. I figured out the problem in my\n test. I hit size filtering heuristics in pack-objects.c:try_delta().\n\n builtin/push.c | 5 ++---\n builtin/receive-pack.c | 8 +++++++-\n t/t5516-fetch-push.sh | 12 ++++++++++++\n 3 files changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 04f0eaf..333a1fb 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n \tNULL,\n };\n \n-static int thin;\n+static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n@@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (receivepack)\n \t\ttransport_set_option(transport,\n \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n-\tif (thin)\n-\t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n+\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n \n \tif (verbosity > 0)\n \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..da60817 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -38,6 +38,7 @@ static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n+static int fix_thin = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n@@ -869,7 +870,8 @@ static const char *unpack(int err_fd)\n \t\tkeeper[i++] = \"--stdin\";\n \t\tif (fsck_objects)\n \t\t\tkeeper[i++] = \"--strict\";\n-\t\tkeeper[i++] = \"--fix-thin\";\n+\t\tif (fix_thin)\n+\t\t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n \t\tkeeper[i++] = keep_arg;\n \t\tkeeper[i++] = NULL;\n@@ -975,6 +977,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tstateless_rpc = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--no-thin\")) {\n+\t\t\t\tfix_thin = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\tusage(receive_pack_usage);\n \t\t}\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4691d51..e80247b 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1172,4 +1172,16 @@ test_expect_success 'push --follow-tag only pushes relevant tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push --no-thin must produce non-thin pack' '\n+\tfor i in `seq 100`; do echo modified >> path1; done &&\n+\tgit commit -am \"path1 big enough to pass size heuristics in try_delta\" &&\n+\tgit init no-thin &&\n+\tgit --git-dir=no-thin/.git config receive.unpacklimit 0 &&\n+\tgit push no-thin/.git refs/heads/master:refs/heads/foo &&\n+\techo modified >> path1 &&\n+\tgit commit -am modified &&\n+\tgit repack -adf &&\n+\tgit push --no-thin --receive-pack=\"git receive-pack --no-thin\" no-thin/.git refs/heads/master:refs/heads/foo\n+'\n+\n test_done\n-- \n1.8.2.83.gc99314b\n"},{"id":"225041","messageId":"CAPig+cSEUrKBg4e9Q+hdeJ1P=3ANwPJfZO6bohH79ZxxuO-PcQ@mail.gmail.com","threadId":"34669","inReplyTo":"1376184258-6784-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] push: respect --no-thin","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-11T05:12:49Z","receivedAt":"2013-08-11T05:12:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 10, 2013 at 9:24 PM, Nguyễn Thái Ngọc Duy wrote:\n> Over the time the default value for --thin has been switched between\n> on and off. As of now it's always on, even if --no-thin is given.\n> Correct the code to respect --no-thin.\n>\n> receive-pack learns about --no-thin only for testing purposes, hence\n> no document update.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 4691d51..e80247b 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1172,4 +1172,16 @@ test_expect_success 'push --follow-tag only pushes relevant tags' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'push --no-thin must produce non-thin pack' '\n> + for i in `seq 100`; do echo modified >> path1; done &&\n\ns/seq/test_seq/\n\nd17cf5f3a32f07bf (tests: Introduce test_seq; 2012-08-03)\n\n> + git commit -am \"path1 big enough to pass size heuristics in try_delta\" &&\n> + git init no-thin &&\n> + git --git-dir=no-thin/.git config receive.unpacklimit 0 &&\n> + git push no-thin/.git refs/heads/master:refs/heads/foo &&\n> + echo modified >> path1 &&\n> + git commit -am modified &&\n> + git repack -adf &&\n> + git push --no-thin --receive-pack=\"git receive-pack --no-thin\" no-thin/.git refs/heads/master:refs/heads/foo\n> +'\n> +\n> test_done\n> --\n> 1.8.2.83.gc99314b\n"},{"id":"225050","messageId":"1376211178-17444-1-git-send-email-pclouds@gmail.com","threadId":"34669","inReplyTo":"1376184258-6784-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3] push: respect --no-thin","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-11T08:52:58Z","receivedAt":"2013-08-11T08:52:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Over the time the default value for --thin has been switched between\non and off. As of now it's always on, even if --no-thin is given.\nCorrect the code to respect --no-thin.\n\nreceive-pack learns about --no-thin only for testing purposes, hence\nno document update.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v3 gets rid of seq in the test.\n\n builtin/push.c | 5 ++---\n builtin/receive-pack.c | 8 +++++++-\n t/t5516-fetch-push.sh | 16 ++++++++++++++++\n 3 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 04f0eaf..333a1fb 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n \tNULL,\n };\n \n-static int thin;\n+static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n@@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (receivepack)\n \t\ttransport_set_option(transport,\n \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n-\tif (thin)\n-\t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n+\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n \n \tif (verbosity > 0)\n \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..da60817 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -38,6 +38,7 @@ static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n+static int fix_thin = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n@@ -869,7 +870,8 @@ static const char *unpack(int err_fd)\n \t\tkeeper[i++] = \"--stdin\";\n \t\tif (fsck_objects)\n \t\t\tkeeper[i++] = \"--strict\";\n-\t\tkeeper[i++] = \"--fix-thin\";\n+\t\tif (fix_thin)\n+\t\t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n \t\tkeeper[i++] = keep_arg;\n \t\tkeeper[i++] = NULL;\n@@ -975,6 +977,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tstateless_rpc = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--no-thin\")) {\n+\t\t\t\tfix_thin = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\tusage(receive_pack_usage);\n \t\t}\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4691d51..3cfd1cd 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1172,4 +1172,20 @@ test_expect_success 'push --follow-tag only pushes relevant tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push --no-thin must produce non-thin pack' '\n+\tcat >>path1 <<\\EOF &&\n+keep base version of path1 big enough, compared to the new changes\n+later, in order to pass size heuristics in\n+builtin/pack-objects.c:try_delta()\n+EOF\n+\tgit commit -am initial &&\n+\tgit init no-thin &&\n+\tgit --git-dir=no-thin/.git config receive.unpacklimit 0 &&\n+\tgit push no-thin/.git refs/heads/master:refs/heads/foo &&\n+\techo modified >> path1 &&\n+\tgit commit -am modified &&\n+\tgit repack -adf &&\n+\tgit push --no-thin --receive-pack=\"git receive-pack --no-thin\" no-thin/.git refs/heads/master:refs/heads/foo\n+'\n+\n test_done\n-- \n1.8.2.83.gc99314b\n"},{"id":"225073","messageId":"7vhaev1mao.fsf@alter.siamese.dyndns.org","threadId":"34669","inReplyTo":"1376211178-17444-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3] push: respect --no-thin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-12T05:59:59Z","receivedAt":"2013-08-12T05:59:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Over the time the default value for --thin has been switched between\n> on and off. As of now it's always on, even if --no-thin is given.\n> Correct the code to respect --no-thin.\n>\n> receive-pack learns about --no-thin only for testing purposes, hence\n> no document update.\n\nPlease name it \"--reject-thin-pack-for-testing\" to make it crystal\nclear that a documentation patch to \"document undocumented option\"\nis unwanted.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 04f0eaf..333a1fb 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n> \tNULL,\n> };\n> \n> -static int thin;\n> +static int thin = 1;\n> static int deleterefs;\n> static const char *receivepack;\n> static int verbosity;\n> @@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n> \tif (receivepack)\n> \t\ttransport_set_option(transport,\n> \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n> -\tif (thin)\n> -\t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n> +\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n> \n> \tif (verbosity > 0)\n> \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\n\nHmm, I am utterly confused.\n\nHow does the original code have thin as an non-overridable default?\nThe variable is initialized to 0, parse-options specifies it as\nOPT_BOOLEAN, and TRANS_OPT_THIN is set only if \"thin\" is set.\n\nUpdated code flips the default to \"1\" but unconditionally uses\nTRANS_OPT_THIN to set it to either \"yes\" or NULL. While it is not\nwrong per-se, do_push() (i.e. the caller of this function) grabs the\ntransport from transport_get() which initializes the transport with\nthe thin option disabled by default, so it is not immediately\nobvious to me why \"always call TRANS_OPT_THIN but set it explicitly\nto NULL when --no-thin is asked\" is necessary or improvement.\n\nPuzzled....\n"},{"id":"225101","messageId":"CACsJy8C-c1TXRiYBO7Mm0jGZvttqzRTONTkskuoU3_kZxHhuCQ@mail.gmail.com","threadId":"34669","inReplyTo":"7vhaev1mao.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] push: respect --no-thin","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-12T11:30:24Z","receivedAt":"2013-08-12T11:30:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Aug 12, 2013 at 12:59 PM, Junio C Hamano wrote:\n>> @@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n>> NULL,\n>> };\n>>\n>> -static int thin;\n>> +static int thin = 1;\n>> static int deleterefs;\n>> static const char *receivepack;\n>> static int verbosity;\n>> @@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n>> if (receivepack)\n>> transport_set_option(transport,\n>> TRANS_OPT_RECEIVEPACK, receivepack);\n>> - if (thin)\n>> - transport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n>> + transport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n>>\n>> if (verbosity > 0)\n>> fprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\n>\n> Hmm, I am utterly confused.\n>\n> How does the original code have thin as an non-overridable default?\n> The variable is initialized to 0, parse-options specifies it as\n> OPT_BOOLEAN, and TRANS_OPT_THIN is set only if \"thin\" is set.\n>\n> Updated code flips the default to \"1\" but unconditionally uses\n> TRANS_OPT_THIN to set it to either \"yes\" or NULL. While it is not\n> wrong per-se, do_push() (i.e. the caller of this function) grabs the\n> transport from transport_get() which initializes the transport with\n> the thin option disabled by default,\n\ntransport_get() actually sets thin option to 1 by default. If I don't\nmisread the code, the first version of transport.c already flipped\n\"thin\" from 0 (in push.c) to 1 (in transport.c), see 9b28851 (Push\ncode for transport library - 2007-09-10). The funny thing is that\ncommit was just one day after Shawn flipped 'thin' from 1 to 0 in\npush.c in a4503a1.\n\n> so it is not immediately\n> obvious to me why \"always call TRANS_OPT_THIN but set it explicitly\n> to NULL when --no-thin is asked\" is necessary or improvement.\n-- \nDuy\n"},{"id":"225111","messageId":"1376315755-28930-1-git-send-email-pclouds@gmail.com","threadId":"34669","inReplyTo":"1376211178-17444-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4] push: respect --no-thin","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-12T13:55:55Z","receivedAt":"2013-08-12T13:55:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"- From the beginning of push.c in 755225d, 2006-04-29, \"thin\" option\n was enabled by default but could be turned off with --no-thin.\n\n- Then Shawn changed the default to 0 in favor of saving server\n resources in a4503a1, 2007-09-09. --no-thin worked great.\n\n- One day later, in 9b28851 Daniel extracted some code from push.c to\n create transport.c. He (probably accidentally) flipped the default\n value from 0 to 1 in transport_get().\n\nFrom then on --no-thin is effectively no-op because git-push still\nexpects the default value to be false and only calls\ntransport_set_option() when \"thin\" variable in push.c is true (which\nis unnecessary). Correct the code to respect --no-thin by calling\ntransport_set_option() in both cases.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v4 elaborates a bit on the story of \"thin\" variable in push.c and\n renames receive-pack --no-thin to --reject-thin-pack-for-testing.\n\n builtin/push.c | 5 ++---\n builtin/receive-pack.c | 8 +++++++-\n t/t5516-fetch-push.sh | 17 +++++++++++++++++\n 3 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 04f0eaf..333a1fb 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n \tNULL,\n };\n \n-static int thin;\n+static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n@@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (receivepack)\n \t\ttransport_set_option(transport,\n \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n-\tif (thin)\n-\t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n+\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n \n \tif (verbosity > 0)\n \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..fc6d53a 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -38,6 +38,7 @@ static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n+static int fix_thin = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n@@ -869,7 +870,8 @@ static const char *unpack(int err_fd)\n \t\tkeeper[i++] = \"--stdin\";\n \t\tif (fsck_objects)\n \t\t\tkeeper[i++] = \"--strict\";\n-\t\tkeeper[i++] = \"--fix-thin\";\n+\t\tif (fix_thin)\n+\t\t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n \t\tkeeper[i++] = keep_arg;\n \t\tkeeper[i++] = NULL;\n@@ -975,6 +977,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tstateless_rpc = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--reject-thin-pack-for-testing\")) {\n+\t\t\t\tfix_thin = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\tusage(receive_pack_usage);\n \t\t}\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4691d51..99c32d7 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1172,4 +1172,21 @@ test_expect_success 'push --follow-tag only pushes relevant tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push --no-thin must produce non-thin pack' '\n+\tcat >>path1 <<\\EOF &&\n+keep base version of path1 big enough, compared to the new changes\n+later, in order to pass size heuristics in\n+builtin/pack-objects.c:try_delta()\n+EOF\n+\tgit commit -am initial &&\n+\tgit init no-thin &&\n+\tgit --git-dir=no-thin/.git config receive.unpacklimit 0 &&\n+\tgit push no-thin/.git refs/heads/master:refs/heads/foo &&\n+\techo modified >> path1 &&\n+\tgit commit -am modified &&\n+\tgit repack -adf &&\n+\trcvpck=\"git receive-pack --reject-thin-pack-for-testing\" &&\n+\tgit push --no-thin --receive-pack=\"$rcvpck\" no-thin/.git refs/heads/master:refs/heads/foo\n+'\n+\n test_done\n-- \n1.8.2.83.gc99314b\n"},{"id":"225151","messageId":"7vob91y218.fsf@alter.siamese.dyndns.org","threadId":"34669","inReplyTo":"CACsJy8C-c1TXRiYBO7Mm0jGZvttqzRTONTkskuoU3_kZxHhuCQ@mail.gmail.com","subject":"Re: [PATCH v3] push: respect --no-thin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-13T16:43:31Z","receivedAt":"2013-08-13T16:43:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> transport_get() actually sets thin option to 1 by default.\n\nYeah, I missed your message in the nearby thread. It indeed does.\n"}]}