{"thread":{"id":"37464","subject":"[PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice","startedAt":"2014-08-31T04:33:30Z","lastAt":"2014-09-04T14:13:44Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"248673","messageId":"1409459610-8695-1-git-send-email-pclouds@gmail.com","threadId":"37464","inReplyTo":null,"subject":"[PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-31T04:33:30Z","receivedAt":"2014-08-31T04:33:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"In the --auto code path, gc_before_repack() is called once in parent\nprocess then again in the forked process. Stop the second run.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/gc.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8d219d8..fb0cff3 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \n static int gc_before_repack(void)\n {\n+\tstatic int nr_runs;\n+\n+\tif (nr_runs++)\n+\t\treturn 0;\n+\n \tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n \t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n \n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"248722","messageId":"xmqqzjeh28p0.fsf@gitster.dls.corp.google.com","threadId":"37464","inReplyTo":"1409459610-8695-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-02T18:12:11Z","receivedAt":"2014-09-02T18:12:11Z","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> In the --auto code path, gc_before_repack() is called once in parent\n> process then again in the forked process. Stop the second run.\n\nHmph. Is the true reason why this happens because we have two calls\nto gc_before_repack() in the main, and one is before daemonize()?\nShouldn't this protection be in the caller?\n\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/gc.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 8d219d8..fb0cff3 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n> \n> static int gc_before_repack(void)\n> {\n> +\tstatic int nr_runs;\n> +\n> +\tif (nr_runs++)\n> +\t\treturn 0;\n> +\n> \tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n> \t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n"},{"id":"248731","messageId":"xmqqtx4pzvcp.fsf@gitster.dls.corp.google.com","threadId":"37464","inReplyTo":"xmqqzjeh28p0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-02T19:16:22Z","receivedAt":"2014-09-02T19:16:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> In the --auto code path, gc_before_repack() is called once in parent\n>> process then again in the forked process. Stop the second run.\n>\n> Hmph. Is the true reason why this happens because we have two calls\n> to gc_before_repack() in the main, and one is before daemonize()?\n> Shouldn't this protection be in the caller?\n\nActually, I think the function already is protecting wastage of\ngetting called twice by saying \"I've done pack-refs already\" and\n\"I've done prune-reflogs already\" by setting the two variables to\nzero. Isn't that sufficient?\n\nWhat problem is this patch trying to fix?\n\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>> ---\n>> builtin/gc.c | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/builtin/gc.c b/builtin/gc.c\n>> index 8d219d8..fb0cff3 100644\n>> --- a/builtin/gc.c\n>> +++ b/builtin/gc.c\n>> @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n>> \n>> static int gc_before_repack(void)\n>> {\n>> +\tstatic int nr_runs;\n>> +\n>> +\tif (nr_runs++)\n>> +\t\treturn 0;\n>> +\n>> \tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n>> \t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n"},{"id":"248836","messageId":"CACsJy8AY_hXMH+ML9SUm_89ykWDOMvB2XThVkaq5Aufes8=5AQ@mail.gmail.com","threadId":"37464","inReplyTo":"xmqqtx4pzvcp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-09-04T14:13:44Z","receivedAt":"2014-09-04T14:13:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Sep 3, 2014 at 2:16 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> In the --auto code path, gc_before_repack() is called once in parent\n>>> process then again in the forked process. Stop the second run.\n>>\n>> Hmph. Is the true reason why this happens because we have two calls\n>> to gc_before_repack() in the main, and one is before daemonize()?\n>> Shouldn't this protection be in the caller?\n>\n> Actually, I think the function already is protecting wastage of\n> getting called twice by saying \"I've done pack-refs already\" and\n> \"I've done prune-reflogs already\" by setting the two variables to\n> zero. Isn't that sufficient?\n>\n> What problem is this patch trying to fix?\n\nOops. I missed that (and should have verified the problem in the first\nplace). Sorry for the noise.\n-- \nDuy\n"}]}