From a650ae8ad3f459f02f5e362baef49316deed80bb Mon Sep 17 00:00:00 2001
From: Han-Wen Nienhuys <hanwen@google.com>
Date: Mon, 31 Jan 2022 12:23:55 +0100
Subject: [PATCH] reftable: tweaks for Windows

Reload the stack _before_ trying to delete the files. This ensures we
don't trip over our own open file handles when deleting compacted
tables.

If there is another process reading the file, it may be impossible to
delete the compacted tables. In this case, ignore the failure.

For cleaning the garbage in this case, the protocol as described in
https://www.git-scm.com/docs/reftable#_windows should be implemented.
This is left for another commit.

Bug: 578454
Change-Id: I7aa43508450041eb9376d9f67a0262ff7cc53c73
---
 .../storage/file/FileReftableStackTest.java         |  6 ++++++
 .../internal/storage/file/FileReftableStack.java    | 13 +++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
index 6c74f0079..98e4b3e1b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
@@ -14,6 +14,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
 
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -32,10 +33,12 @@
 import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.SystemReader;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+
 public class FileReftableStackTest {
 
 	private static Ref newRef(String name, ObjectId id) {
@@ -118,6 +121,9 @@ public void testCompaction1024() throws Exception {
 	@SuppressWarnings({ "resource", "unused" })
 	@Test
 	public void missingReftable() throws Exception {
+		// Can't delete in-use files on Windows.
+		assumeFalse(SystemReader.getInstance().isWindows());
+
 		try (FileReftableStack stack = new FileReftableStack(
 				new File(reftableDir, "refs"), reftableDir, null,
 				() -> new Config())) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
index f02c8613a..5152367d2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
@@ -40,6 +40,7 @@
 import org.eclipse.jgit.internal.storage.reftable.ReftableWriter;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.SystemReader;
 
 /**
  * A mutable stack of reftables on local filesystem storage. Not thread-safe.
@@ -527,11 +528,19 @@ boolean compactRange(int first, int last) throws IOException {
 				return false;
 			}
 
+			reload();
 			for (File f : deleteOnSuccess) {
-				Files.delete(f.toPath());
+				try {
+					Files.delete(f.toPath());
+				} catch (IOException e) {
+					// Ignore: this can happen on Windows in case of concurrent processes.
+					// leave the garbage and continue.
+					if (!SystemReader.getInstance().isWindows()) {
+						throw e;
+					}
+				}
 			}
 
-			reload();
 			return true;
 		} finally {
 			if (tmpTable != null) {