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

Subsystems: the rest

STALE3737d

15 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 23:06:42 

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.

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 <redacted>
---
 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();
+	}
 
 	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
+'
 
 test_done
-- 
2.5.3.windows.1.3.gc322723

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

From: Max Kirillov <hidden>
Date: 2016-06-15 23:06:44 

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.
quoted hunk
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 <redacted>
---
 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.
quoted hunk
 	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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

From: Max Kirillov <hidden>
Date: 2016-06-15 23:06:44 

On Wed, Sep 30, 2015 at 10:28:14PM +0300, Max Kirillov wrote:
On Mon, Sep 28, 2015 at 09:44:57PM +0200, Johannes Schindelin wrote:
quoted
-	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.
Apparently, pack file is closed just after mapping if it's
smaller than core.packedGitWindowSize. Could it be the
reason that this patch worked in you test case?

-- 
Max





  

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

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
Hi Max,

On 2015-10-01 06:39, Max Kirillov wrote:
On Wed, Sep 30, 2015 at 10:28:14PM +0300, Max Kirillov wrote:
quoted
On Mon, Sep 28, 2015 at 09:44:57PM +0200, Johannes Schindelin wrote:
quoted
-	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();
+	}
quoted
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.
Apparently, pack file is closed just after mapping if it's
smaller than core.packedGitWindowSize. Could it be the
reason that this patch worked in you test case?
That must be the reason, thanks!

Ciao,
Dscho





  

    
[PATCH v2 0/4] Fix locking issues on Windows with `git clone --dissociate`

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
This is version 2, split into multiple commits for easier digestion.

Max, I hope that this helps also your use case!

Johannes Schindelin (4):
  Demonstrate a Windows file locking issue with `git clone --dissociate`
  Consolidate code to close a pack's file descriptor
  Add a function to release all packs
  clone --dissociate: avoid locking pack files

 builtin/clone.c            |  4 +++-
 cache.h                    |  1 +
 sha1_file.c                | 56 ++++++++++++++++++++++++++++------------------
 t/t5700-clone-reference.sh | 21 +++++++++++++++++
 4 files changed, 59 insertions(+), 23 deletions(-)

-- 
2.5.3.windows.1.3.gc322723





  

    
[PATCH v2 1/4] Demonstrate a Windows file locking issue with `git clone --dissociate`

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
On Windows, dissociating from a reference can fail very easily due to
pack files that are still in use when they want to be removed.

Signed-off-by: Johannes Schindelin <redacted>
---
 t/t5700-clone-reference.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index ef1779f..f7cec85 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_failure MINGW '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
+'
 
 test_done
-- 
2.5.3.windows.1.3.gc322723





  

    
[PATCH v2 2/4] Consolidate code to close a pack's file descriptor

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
There was a lot of repeated code to close the file descriptor of
a given pack. Let's just refactor this code into a single function.

Signed-off-by: Johannes Schindelin <redacted>
---
 sha1_file.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d295a32..8c3c913 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -786,6 +786,18 @@ void close_pack_windows(struct packed_git *p)
 	}
 }
 
+static int close_pack_fd(struct packed_git *p)
+{
+	if (p->pack_fd < 0)
+		return 0;
+
+	close(p->pack_fd);
+	pack_open_fds--;
+	p->pack_fd = -1;
+
+	return 1;
+}
+
 /*
  * The LRU pack is the one with the oldest MRU window, preferring packs
  * with no used windows, or the oldest mtime if it has no windows allocated.
@@ -853,12 +865,8 @@ static int close_one_pack(void)
 		find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
 	}
 
-	if (lru_p) {
-		close(lru_p->pack_fd);
-		pack_open_fds--;
-		lru_p->pack_fd = -1;
-		return 1;
-	}
+	if (lru_p)
+		return close_pack_fd(lru_p);
 
 	return 0;
 }
@@ -899,10 +907,7 @@ void free_pack_by_name(const char *pack_name)
 		if (strcmp(pack_name, p->pack_name) == 0) {
 			clear_delta_base_cache();
 			close_pack_windows(p);
-			if (p->pack_fd != -1) {
-				close(p->pack_fd);
-				pack_open_fds--;
-			}
+			close_pack_fd(p);
 			close_pack_index(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
@@ -1037,11 +1042,7 @@ static int open_packed_git(struct packed_git *p)
 {
 	if (!open_packed_git_1(p))
 		return 0;
-	if (p->pack_fd != -1) {
-		close(p->pack_fd);
-		pack_open_fds--;
-		p->pack_fd = -1;
-	}
+	close_pack_fd(p);
 	return -1;
 }
 
@@ -1107,11 +1108,8 @@ unsigned char *use_pack(struct packed_git *p,
 					p->pack_name,
 					strerror(errno));
 			if (!win->offset && win->len == p->pack_size
-				&& !p->do_not_close) {
-				close(p->pack_fd);
-				pack_open_fds--;
-				p->pack_fd = -1;
-			}
+				&& !p->do_not_close)
+				close_pack_fd(p);
 			pack_mmap_calls++;
 			pack_open_windows++;
 			if (pack_mapped > peak_pack_mapped)
-- 
2.5.3.windows.1.3.gc322723





  

    
[PATCH v2 3/4] Add a function to release all packs

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
On Windows, files that are in use cannot be removed or renamed. That
means that we have to release pack files when we are about to, say,
repack them. Let's introduce a convenient function to close them
pack files.

While at it, we consolidate the close windows/close fd/close index
stanza in `free_pack_by_name()` into the `close_pack()` function that
is used by the new `close_all_packs()` function to avoid repeated code.

Signed-off-by: Johannes Schindelin <redacted>
---
 cache.h     |  1 +
 sha1_file.c | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..57f6a74 100644
--- a/cache.h
+++ b/cache.h
@@ -1275,6 +1275,7 @@ extern void close_pack_index(struct packed_git *);
 
 extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);
 extern void close_pack_windows(struct packed_git *);
+extern void close_all_packs(void);
 extern void unuse_pack(struct pack_window **);
 extern void free_pack_by_name(const char *);
 extern void clear_delta_base_cache(void);
diff --git a/sha1_file.c b/sha1_file.c
index 8c3c913..fe823fe 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -798,6 +798,22 @@ static int close_pack_fd(struct packed_git *p)
 	return 1;
 }
 
+static void close_pack(struct packed_git *p)
+{
+	close_pack_windows(p);
+	close_pack_fd(p);
+	close_pack_index(p);
+}
+
+void close_all_packs(void)
+{
+	struct packed_git *p;
+
+	for (p = packed_git; p; p = p->next)
+		close_pack(p);
+}
+
+
 /*
  * The LRU pack is the one with the oldest MRU window, preferring packs
  * with no used windows, or the oldest mtime if it has no windows allocated.
@@ -906,9 +922,7 @@ void free_pack_by_name(const char *pack_name)
 		p = *pp;
 		if (strcmp(pack_name, p->pack_name) == 0) {
 			clear_delta_base_cache();
-			close_pack_windows(p);
-			close_pack_fd(p);
-			close_pack_index(p);
+			close_pack(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
 			if (last_found_pack == p)
-- 
2.5.3.windows.1.3.gc322723





  

    
[PATCH v2 4/4] clone --dissociate: avoid locking pack files

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
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.

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

The test case to demonstrate the breakage technically does not need to
be run on Linux or MacOSX. It won't hurt, either, though.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/clone.c            | 4 +++-
 t/t5700-clone-reference.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 578da85..cc896e2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1064,8 +1064,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	transport_unlock_pack(transport);
 	transport_disconnect(transport);
 
-	if (option_dissociate)
+	if (option_dissociate) {
+		close_all_packs();
 		dissociate_from_references();
+	}
 
 	junk_mode = JUNK_LEAVE_REPO;
 	err = checkout();
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index f7cec85..2250ef4 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -188,7 +188,7 @@ test_expect_success 'clone and dissociate from reference' '
 	test_must_fail git -C R fsck &&
 	git -C S fsck
 '
-test_expect_failure MINGW 'clone, dissociate from partial reference and repack' '
+test_expect_success 'clone, dissociate from partial reference and repack' '
 	rm -fr P Q R &&
 	git init P &&
 	(
-- 
2.5.3.windows.1.3.gc322723





  

    
[PATCH v3 0/4] Fix locking issues on Windows with `git clone --dissociate`

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
This is version 3, adding that BUG! message if do_not_close was set.

Max, I still hope that this patch series helps also your use case!

Interdiff below the diffstat.

Johannes Schindelin (4):
  Demonstrate a Windows file locking issue with `git clone --dissociate`
  Consolidate code to close a pack's file descriptor
  Add a function to release all packs
  clone --dissociate: avoid locking pack files

 builtin/clone.c            |  4 +++-
 cache.h                    |  1 +
 sha1_file.c                | 59 +++++++++++++++++++++++++++++-----------------
 t/t5700-clone-reference.sh | 21 +++++++++++++++++
 4 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index fe823fe..ca699d7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -810,7 +810,10 @@ void close_all_packs(void)
 	struct packed_git *p;
 
 	for (p = packed_git; p; p = p->next)
-		close_pack(p);
+		if (p->do_not_close)
+			die("BUG! Want to close pack marked 'do-not-close'");
+		else
+			close_pack(p);
 }
 
 
-- 
2.6.1.windows.1





  

    
[PATCH v3 1/4] Demonstrate a Windows file locking issue with `git clone --dissociate`

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
On Windows, dissociating from a reference can fail very easily due to
pack files that are still in use when they want to be removed.

Signed-off-by: Johannes Schindelin <redacted>
---
 t/t5700-clone-reference.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index ef1779f..f7cec85 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_failure MINGW '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
+'
 
 test_done
-- 
2.6.1.windows.1





  

    
[PATCH v3 2/4] Consolidate code to close a pack's file descriptor

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
There was a lot of repeated code to close the file descriptor of
a given pack. Let's just refactor this code into a single function.

Signed-off-by: Johannes Schindelin <redacted>
---
 sha1_file.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d295a32..8c3c913 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -786,6 +786,18 @@ void close_pack_windows(struct packed_git *p)
 	}
 }
 
+static int close_pack_fd(struct packed_git *p)
+{
+	if (p->pack_fd < 0)
+		return 0;
+
+	close(p->pack_fd);
+	pack_open_fds--;
+	p->pack_fd = -1;
+
+	return 1;
+}
+
 /*
  * The LRU pack is the one with the oldest MRU window, preferring packs
  * with no used windows, or the oldest mtime if it has no windows allocated.
@@ -853,12 +865,8 @@ static int close_one_pack(void)
 		find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
 	}
 
-	if (lru_p) {
-		close(lru_p->pack_fd);
-		pack_open_fds--;
-		lru_p->pack_fd = -1;
-		return 1;
-	}
+	if (lru_p)
+		return close_pack_fd(lru_p);
 
 	return 0;
 }
@@ -899,10 +907,7 @@ void free_pack_by_name(const char *pack_name)
 		if (strcmp(pack_name, p->pack_name) == 0) {
 			clear_delta_base_cache();
 			close_pack_windows(p);
-			if (p->pack_fd != -1) {
-				close(p->pack_fd);
-				pack_open_fds--;
-			}
+			close_pack_fd(p);
 			close_pack_index(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
@@ -1037,11 +1042,7 @@ static int open_packed_git(struct packed_git *p)
 {
 	if (!open_packed_git_1(p))
 		return 0;
-	if (p->pack_fd != -1) {
-		close(p->pack_fd);
-		pack_open_fds--;
-		p->pack_fd = -1;
-	}
+	close_pack_fd(p);
 	return -1;
 }
 
@@ -1107,11 +1108,8 @@ unsigned char *use_pack(struct packed_git *p,
 					p->pack_name,
 					strerror(errno));
 			if (!win->offset && win->len == p->pack_size
-				&& !p->do_not_close) {
-				close(p->pack_fd);
-				pack_open_fds--;
-				p->pack_fd = -1;
-			}
+				&& !p->do_not_close)
+				close_pack_fd(p);
 			pack_mmap_calls++;
 			pack_open_windows++;
 			if (pack_mapped > peak_pack_mapped)
-- 
2.6.1.windows.1





  

    
[PATCH v3 3/4] Add a function to release all packs

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
On Windows, files that are in use cannot be removed or renamed. That
means that we have to release pack files when we are about to, say,
repack them. Let's introduce a convenient function to close them
pack files.

While at it, we consolidate the close windows/close fd/close index
stanza in `free_pack_by_name()` into the `close_pack()` function that
is used by the new `close_all_packs()` function to avoid repeated code.

Signed-off-by: Johannes Schindelin <redacted>
---
 cache.h     |  1 +
 sha1_file.c | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..57f6a74 100644
--- a/cache.h
+++ b/cache.h
@@ -1275,6 +1275,7 @@ extern void close_pack_index(struct packed_git *);
 
 extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);
 extern void close_pack_windows(struct packed_git *);
+extern void close_all_packs(void);
 extern void unuse_pack(struct pack_window **);
 extern void free_pack_by_name(const char *);
 extern void clear_delta_base_cache(void);
diff --git a/sha1_file.c b/sha1_file.c
index 8c3c913..ca699d7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -798,6 +798,25 @@ static int close_pack_fd(struct packed_git *p)
 	return 1;
 }
 
+static void close_pack(struct packed_git *p)
+{
+	close_pack_windows(p);
+	close_pack_fd(p);
+	close_pack_index(p);
+}
+
+void close_all_packs(void)
+{
+	struct packed_git *p;
+
+	for (p = packed_git; p; p = p->next)
+		if (p->do_not_close)
+			die("BUG! Want to close pack marked 'do-not-close'");
+		else
+			close_pack(p);
+}
+
+
 /*
  * The LRU pack is the one with the oldest MRU window, preferring packs
  * with no used windows, or the oldest mtime if it has no windows allocated.
@@ -906,9 +925,7 @@ void free_pack_by_name(const char *pack_name)
 		p = *pp;
 		if (strcmp(pack_name, p->pack_name) == 0) {
 			clear_delta_base_cache();
-			close_pack_windows(p);
-			close_pack_fd(p);
-			close_pack_index(p);
+			close_pack(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
 			if (last_found_pack == p)
-- 
2.6.1.windows.1





  

    
[PATCH v3 4/4] clone --dissociate: avoid locking pack files

    

      From: Johannes Schindelin <hidden>

      Date: 2016-06-15 23:06:46 
    

  

  
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.

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

The test case to demonstrate the breakage technically does not need to
be run on Linux or MacOSX. It won't hurt, either, though.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/clone.c            | 4 +++-
 t/t5700-clone-reference.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 578da85..cc896e2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1064,8 +1064,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	transport_unlock_pack(transport);
 	transport_disconnect(transport);
 
-	if (option_dissociate)
+	if (option_dissociate) {
+		close_all_packs();
 		dissociate_from_references();
+	}
 
 	junk_mode = JUNK_LEAVE_REPO;
 	err = checkout();
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index f7cec85..2250ef4 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -188,7 +188,7 @@ test_expect_success 'clone and dissociate from reference' '
 	test_must_fail git -C R fsck &&
 	git -C S fsck
 '
-test_expect_failure MINGW 'clone, dissociate from partial reference and repack' '
+test_expect_success 'clone, dissociate from partial reference and repack' '
 	rm -fr P Q R &&
 	git init P &&
 	(
-- 
2.6.1.windows.1





  

    
Re: [PATCH v3 0/4] Fix locking issues on Windows with `git clone --dissociate`

    

      From: Max Kirillov <hidden>

      Date: 2016-06-15 23:06:49 
    

  

  
On Tue, Oct 06, 2015 at 03:17:36PM +0200, Johannes Schindelin wrote:
This is version 3, adding that BUG! message if do_not_close was set.

Max, I still hope that this patch series helps also your use case!
Thanks, this mostly makes gone one of my commits. I only
need to invoke the function after builtin is done. And maybe
spend some time to check that dies do not happen.

The other one with cloexec I think I will still need.

It migh take some time before I return to it.

-- 
Max




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help