Subject: Re: [PATCH] clone --dissociate: avoid locking pack files

From: Max Kirillov <[email-masked]>
Date: Wed, 30 Sep 2015 22:28:14 +0300

On Mon, Sep 28, 2015 at 09:44:57PM +0200, Johannes Schindelin wrote:
> When `git clone` is asked to dissociate the repository from the
> reference repository whose objects were used, it is quite possible that
> the pack files need to be repacked. In that case, the pack files need to
> be deleted that were originally hard-links to the reference repository's
> pack files.

Hello. For 1.9.* I used to have some hack for closing files
also. The case was to allow scheduled git gc to remove packs
even if I forgot to quit some less in some console.

> On platforms where a file cannot be deleted if another process still
> holds a handle on it, we therefore need to take pains to release all
> pack files and indexes before dissociating.
> 
> This fixes https://github.com/git-for-windows/git/issues/446
> 
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  builtin/clone.c            |  9 ++++++++-
>  t/t5700-clone-reference.sh | 21 +++++++++++++++++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 578da85..223adc4 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1064,8 +1064,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	transport_unlock_pack(transport);
>  	transport_disconnect(transport);
>  
> -	if (option_dissociate)
> +	if (option_dissociate) {
> +		struct packed_git *p;
> +
> +		for (p = packed_git; p; p = p->next) {
> +			close_pack_windows(p);
> +			close_pack_index(p);
> +		}
>  		dissociate_from_references();
> +	}

This does not seem to close handles to the pack files
themseves, does Windows still allow removing the files? I
probably did not tried that, because I started from handles,
and discovered mapped files only later.

>  	junk_mode = JUNK_LEAVE_REPO;
>  	err = checkout();
> diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
> index ef1779f..2250ef4 100755
> --- a/t/t5700-clone-reference.sh
> +++ b/t/t5700-clone-reference.sh
> @@ -188,5 +188,26 @@ test_expect_success 'clone and dissociate from reference' '
>  	test_must_fail git -C R fsck &&
>  	git -C S fsck
>  '
> +test_expect_success 'clone, dissociate from partial reference and repack' '
> +	rm -fr P Q R &&
> +	git init P &&
> +	(
> +		cd P &&
> +		test_commit one &&
> +		git repack &&
> +		test_commit two &&
> +		git repack
> +	) &&
> +	git clone --bare P Q &&
> +	(
> +		cd P &&
> +		git checkout -b second &&
> +		test_commit three &&
> +		git repack
> +	) &&
> +	git clone --bare --dissociate --reference=P Q R &&
> +	ls R/objects/pack/*.pack >packs.txt &&
> +	test_line_count = 1 packs.txt
> +'

Unless it goes very lowlevel like running lsof of readin
proc testing this should always pass on Linux, even if the
issue is not fixed, maybe should be a conditional for
Windows only?

>  test_done
> -- 
> 2.5.3.windows.1.3.gc322723
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html