# [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice 4 messages from 2014-08-31 to 2014-09-04. Participants: Nguyễn Thái Ngọc Duy, Junio C Hamano, Duy Nguyen. Thread: https://gitlist.dev/t/37464 ## Nguyễn Thái Ngọc Duy, 2014-08-31 04:33 Subject: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice Message-ID: <1409459610-8695-1-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1409459610-8695-1-git-send-email-pclouds%40gmail.com ``` In the --auto code path, gc_before_repack() is called once in parent process then again in the forked process. Stop the second run. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/gc.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index 8d219d8..fb0cff3 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) static int gc_before_repack(void) { + static int nr_runs; + + if (nr_runs++) + return 0; + if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) return error(FAILED_RUN, pack_refs_cmd.argv[0]); -- 2.1.0.rc0.78.gc0d8480 ``` ## Junio C Hamano, 2014-09-02 18:12 Subject: Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice Message-ID: URL: https://gitlist.dev/e/xmqqzjeh28p0.fsf%40gitster.dls.corp.google.com In-Reply-To: <1409459610-8695-1-git-send-email-pclouds@gmail.com> ``` Nguyễn Thái Ngọc Duy writes: > In the --auto code path, gc_before_repack() is called once in parent > process then again in the forked process. Stop the second run. Hmph. Is the true reason why this happens because we have two calls to gc_before_repack() in the main, and one is before daemonize()? Shouldn't this protection be in the caller? > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > builtin/gc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/builtin/gc.c b/builtin/gc.c > index 8d219d8..fb0cff3 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) > > static int gc_before_repack(void) > { > + static int nr_runs; > + > + if (nr_runs++) > + return 0; > + > if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) > return error(FAILED_RUN, pack_refs_cmd.argv[0]); ``` ## Junio C Hamano, 2014-09-02 19:16 Subject: Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice Message-ID: URL: https://gitlist.dev/e/xmqqtx4pzvcp.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > Nguyễn Thái Ngọc Duy writes: > >> In the --auto code path, gc_before_repack() is called once in parent >> process then again in the forked process. Stop the second run. > > Hmph. Is the true reason why this happens because we have two calls > to gc_before_repack() in the main, and one is before daemonize()? > Shouldn't this protection be in the caller? Actually, I think the function already is protecting wastage of getting called twice by saying "I've done pack-refs already" and "I've done prune-reflogs already" by setting the two variables to zero. Isn't that sufficient? What problem is this patch trying to fix? >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- >> builtin/gc.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/builtin/gc.c b/builtin/gc.c >> index 8d219d8..fb0cff3 100644 >> --- a/builtin/gc.c >> +++ b/builtin/gc.c >> @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) >> >> static int gc_before_repack(void) >> { >> + static int nr_runs; >> + >> + if (nr_runs++) >> + return 0; >> + >> if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) >> return error(FAILED_RUN, pack_refs_cmd.argv[0]); ``` ## Duy Nguyen, 2014-09-04 14:13 Subject: Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice Message-ID: URL: https://gitlist.dev/e/CACsJy8AY_hXMH%2BML9SUm_89ykWDOMvB2XThVkaq5Aufes8%3D5AQ%40mail.gmail.com In-Reply-To: ``` On Wed, Sep 3, 2014 at 2:16 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Nguyễn Thái Ngọc Duy writes: >> >>> In the --auto code path, gc_before_repack() is called once in parent >>> process then again in the forked process. Stop the second run. >> >> Hmph. Is the true reason why this happens because we have two calls >> to gc_before_repack() in the main, and one is before daemonize()? >> Shouldn't this protection be in the caller? > > Actually, I think the function already is protecting wastage of > getting called twice by saying "I've done pack-refs already" and > "I've done prune-reflogs already" by setting the two variables to > zero. Isn't that sufficient? > > What problem is this patch trying to fix? Oops. I missed that (and should have verified the problem in the first place). Sorry for the noise. -- Duy ```