[BUG] auto-repack exits prematurely, locking other processing out

>
> ==========================================================================
> [~/linux](master)$ git pull --rebase
> remote: Counting objects: 455, done.
> remote: Compressing objects: 100% (64/64), done.
> remote: Total 267 (delta 208), reused 262 (delta 203)
> Receiving objects: 100% (267/267), 44.43 KiB | 0 bytes/s, done.
> Resolving deltas: 100% (208/208), completed with 80 local objects.
> From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux
>    4b660a7..f02f79d  master     -> linus/master
> Auto packing the repository in background for optimum performance.
> See "git help gc" for manual housekeeping.
> First, rewinding head to replay your work on top of it...
> Applying: perf: tools: fix missing casts for printf arguments.
> Applying: vt: emulate 8- and 24-bit colour codes.
> fatal: Unable to create '/home/kilobyte/linux/.git/refs/heads/master.lock': File exists.
>
> If no other git process is currently running, this probably means a
> git process crashed in this repository earlier. Make sure no other git
> process is running and remove the file manually to continue.
> Could not move back to refs/heads/master
> [~/linux]((no branch, rebasing (null)))$
> ==========================================================================

> Adam Borowski <kilobyte@angband.pl> writes:
> > It looks like the periodic auto-repack backgrounds itself when it shouldn't
> > do so.  This causes the command it has triggered as a part of to fail:
> 
> Duy, 9f673f94 (gc: config option for running --auto in background,
> 2014-02-08) turns to be not such a hot idea.  Sure, if we kick it
> off background after doing something heavy, immediately before
> giving control back to the end-user, and expect that the user will
> stay thinking without making new changes (i.e. read-only stuff like
> "git show" would be OK), then daemonize might be a great thing, but
> we forgot, while doing that commit, that long-running operations
> trigger the auto gc in the middle *and* they want it finish before
> they continue, as the purpose of gc is to help the performance
> during their further operation.

> On Fri, May 23, 2014 at 02:40:41PM -0700, Junio C Hamano wrote:
>> Adam Borowski <kilobyte@angband.pl> writes:
>> > It looks like the periodic auto-repack backgrounds itself when it shouldn't
>> > do so.  This causes the command it has triggered as a part of to fail:
>> 
>> Duy, 9f673f94 (gc: config option for running --auto in background,
>> 2014-02-08) turns to be not such a hot idea.  Sure, if we kick it
>> off background after doing something heavy, immediately before
>> giving control back to the end-user, and expect that the user will
>> stay thinking without making new changes (i.e. read-only stuff like
>> "git show" would be OK), then daemonize might be a great thing, but
>> we forgot, while doing that commit, that long-running operations
>> trigger the auto gc in the middle *and* they want it finish before
>> they continue, as the purpose of gc is to help the performance
>> during their further operation.
>
> Just add a lock that's triggered by daemonize, and have things block on this
> lock.

> Duy, 9f673f94 (gc: config option for running --auto in background,
> 2014-02-08) turns to be not such a hot idea.  Sure, if we kick it
> off background after doing something heavy, immediately before
> giving control back to the end-user, and expect that the user will
> stay thinking without making new changes (i.e. read-only stuff like
> "git show" would be OK), then daemonize might be a great thing, but
> we forgot, while doing that commit, that long-running operations
> trigger the auto gc in the middle *and* they want it finish before
> they continue, as the purpose of gc is to help the performance
> during their further operation.

> 9f673f9 (gc: config option for running --auto in background -
> 2014-02-08) puts "gc --auto" in background to reduce user's wait
> time. Part of the garbage collecting is pack-refs and pruning
> reflogs. These require locking some refs and may abort other processes
> trying to lock the same ref. If gc --auto is fired in the middle of a
> script, gc's holding locks in the background could fail the script,
> which could never happen before 9f673f9.
>
> Keep running pack-refs and "reflog --prune" in foreground to stop
> parallel ref updates. The remaining background operations (repack,
> prune and rerere) should impact running git processes.
>
> Reported-by: Adam Borowski <kilobyte@angband.pl>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/gc.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 85f5c2b..8d219d8 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -26,6 +26,7 @@ static const char * const builtin_gc_usage[] = {
>  };
>  
>  static int pack_refs = 1;
> +static int prune_reflogs = 1;
>  static int aggressive_depth = 250;
>  static int aggressive_window = 250;
>  static int gc_auto_threshold = 6700;
> @@ -258,6 +259,19 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
>  	return NULL;
>  }
>  
> +static int gc_before_repack(void)
> +{
> +	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
> +		return error(FAILED_RUN, pack_refs_cmd.argv[0]);
> +
> +	if (prune_reflogs && run_command_v_opt(reflog.argv, RUN_GIT_CMD))
> +		return error(FAILED_RUN, reflog.argv[0]);
> +
> +	pack_refs = 0;
> +	prune_reflogs = 0;
> +	return 0;
> +}
> +
>  int cmd_gc(int argc, const char **argv, const char *prefix)
>  {
>  	int aggressive = 0;
> @@ -320,12 +334,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  				fprintf(stderr, _("Auto packing the repository for optimum performance.\n"));
>  			fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
>  		}
> -		if (detach_auto)
> +		if (detach_auto) {
> +			if (gc_before_repack())
> +				return -1;
>  			/*
>  			 * failure to daemonize is ok, we'll continue
>  			 * in foreground
>  			 */
>  			daemonize();
> +		}
>  	} else
>  		add_repack_all_option();
>  
> @@ -337,11 +354,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		    name, (uintmax_t)pid);
>  	}
>  
> -	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
> -		return error(FAILED_RUN, pack_refs_cmd.argv[0]);
> -
> -	if (run_command_v_opt(reflog.argv, RUN_GIT_CMD))
> -		return error(FAILED_RUN, reflog.argv[0]);
> +	if (gc_before_repack())
> +		return -1;
>  
>  	if (run_command_v_opt(repack.argv, RUN_GIT_CMD))
>  		return error(FAILED_RUN, repack.argv[0]);