You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ja...@apache.org on 2025/09/23 23:08:32 UTC
(lucene) branch branch_9_12 updated: Fix WindowsFS to support Windows (#14627)
This is an automated email from the ASF dual-hosted git repository.
jainankitk pushed a commit to branch branch_9_12
in repository https://gitbox.apache.org/repos/asf/lucene.git
The following commit(s) were added to refs/heads/branch_9_12 by this push:
new e1e23659e3b Fix WindowsFS to support Windows (#14627)
e1e23659e3b is described below
commit e1e23659e3b02e08a2e134798bbaa1b6adef0648
Author: Uwe Schindler <us...@apache.org>
AuthorDate: Thu May 8 14:29:31 2025 +0200
Fix WindowsFS to support Windows (#14627)
Windows does not support file keys (the attribute returns null). Therefor WindowsFS cannot be used on Windows. Due to Windows 11 no longer preventing depleting of open files, we need to be able to use WindowsFS on Windows (irony), fix this to fallback to path as key. It should be good enough like an inode. Renaming/moving files is already handles by WindowsFS when it detects a change in key after rename/move.
Closes #11920
---
.../org/apache/lucene/index/TestIndexWriter.java | 26 +++-------------------
.../lucene/store/TestFileSwitchDirectory.java | 2 --
.../store/TestHardLinkCopyDirectoryWrapper.java | 3 ---
.../apache/lucene/tests/mockfile/WindowsFS.java | 10 ++++-----
.../apache/lucene/tests/mockfile/WindowsPath.java | 17 ++++++--------
.../tests/util/TestRuleTemporaryFilesCleanup.java | 8 ++-----
.../lucene/tests/mockfile/TestWindowsFS.java | 20 ++++++++++-------
7 files changed, 29 insertions(+), 57 deletions(-)
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
index 80d80b8e67d..1fd8cdd4ae0 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
@@ -88,7 +88,6 @@ import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.LockObtainFailedException;
-import org.apache.lucene.store.MMapDirectory;
import org.apache.lucene.store.NIOFSDirectory;
import org.apache.lucene.store.NoLockFactory;
import org.apache.lucene.store.SimpleFSLockFactory;
@@ -107,7 +106,6 @@ import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.lucene.tests.util.TestUtil;
import org.apache.lucene.util.Bits;
import org.apache.lucene.util.BytesRef;
-import org.apache.lucene.util.Constants;
import org.apache.lucene.util.IOSupplier;
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.InfoStream;
@@ -1274,16 +1272,9 @@ public class TestIndexWriter extends LuceneTestCase {
WindowsFS provider = new WindowsFS(path.getFileSystem());
Path indexPath = provider.wrapPath(path);
- // NOTE: on Unix, we cannot use MMapDir, because WindowsFS doesn't see/think it keeps file
- // handles open. Yet, on Windows, we MUST use
- // MMapDir because the windows OS will in fact prevent file deletion for us, and fails
- // otherwise:
- FSDirectory dir;
- if (Constants.WINDOWS) {
- dir = new MMapDirectory(indexPath);
- } else {
- dir = new NIOFSDirectory(indexPath);
- }
+ // NOTE: We cannot use MMapDir, because WindowsFS doesn't see/think it keeps file
+ // handles open.
+ FSDirectory dir = new NIOFSDirectory(indexPath);
MergePolicy mergePolicy = newLogMergePolicy(true);
@@ -2945,9 +2936,6 @@ public class TestIndexWriter extends LuceneTestCase {
}
public void testPendingDeleteDVGeneration() throws IOException {
- // irony: currently we don't emulate windows well enough to work on windows!
- assumeFalse("windows is not supported", Constants.WINDOWS);
-
Path path = createTempDir();
// Use WindowsFS to prevent open files from being deleted:
@@ -3017,9 +3005,6 @@ public class TestIndexWriter extends LuceneTestCase {
}
public void testPendingDeletionsRollbackWithReader() throws IOException {
- // irony: currently we don't emulate windows well enough to work on windows!
- assumeFalse("windows is not supported", Constants.WINDOWS);
-
Path path = createTempDir();
// Use WindowsFS to prevent open files from being deleted:
@@ -3056,9 +3041,6 @@ public class TestIndexWriter extends LuceneTestCase {
}
public void testWithPendingDeletions() throws Exception {
- // irony: currently we don't emulate windows well enough to work on windows!
- assumeFalse("windows is not supported", Constants.WINDOWS);
-
Path path = createTempDir();
// Use WindowsFS to prevent open files from being deleted:
@@ -3112,8 +3094,6 @@ public class TestIndexWriter extends LuceneTestCase {
public void testPendingDeletesAlreadyWrittenFiles() throws IOException {
Path path = createTempDir();
- // irony: currently we don't emulate windows well enough to work on windows!
- assumeFalse("windows is not supported", Constants.WINDOWS);
// Use WindowsFS to prevent open files from being deleted:
WindowsFS provider = new WindowsFS(path.getFileSystem());
diff --git a/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java
index 2298ef82ea3..32f8c388c5f 100644
--- a/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java
+++ b/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java
@@ -36,7 +36,6 @@ import org.apache.lucene.tests.mockfile.WindowsFS;
import org.apache.lucene.tests.store.BaseDirectoryTestCase;
import org.apache.lucene.tests.store.MockDirectoryWrapper;
import org.apache.lucene.tests.util.TestUtil;
-import org.apache.lucene.util.Constants;
public class TestFileSwitchDirectory extends BaseDirectoryTestCase {
@@ -176,7 +175,6 @@ public class TestFileSwitchDirectory extends BaseDirectoryTestCase {
public void testDeleteAndList() throws IOException {
// relies on windows semantics
Path path = createTempDir();
- assumeFalse("Irony we seem to not emulate windows well enough", Constants.WINDOWS);
WindowsFS provider = new WindowsFS(path.getFileSystem());
Path indexPath = provider.wrapPath(path);
try (final FileSwitchDirectory dir =
diff --git a/lucene/misc/src/test/org/apache/lucene/misc/store/TestHardLinkCopyDirectoryWrapper.java b/lucene/misc/src/test/org/apache/lucene/misc/store/TestHardLinkCopyDirectoryWrapper.java
index 4e83f23da41..045b3204af2 100644
--- a/lucene/misc/src/test/org/apache/lucene/misc/store/TestHardLinkCopyDirectoryWrapper.java
+++ b/lucene/misc/src/test/org/apache/lucene/misc/store/TestHardLinkCopyDirectoryWrapper.java
@@ -33,7 +33,6 @@ import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.NIOFSDirectory;
import org.apache.lucene.tests.mockfile.WindowsFS;
import org.apache.lucene.tests.store.BaseDirectoryTestCase;
-import org.apache.lucene.util.Constants;
import org.apache.lucene.util.IOUtils;
// See: https://issues.apache.org/jira/browse/SOLR-12028 Tests cannot remove files on Windows
@@ -105,8 +104,6 @@ public class TestHardLinkCopyDirectoryWrapper extends BaseDirectoryTestCase {
}
public void testRenameWithHardLink() throws Exception {
- // irony: currently we don't emulate windows well enough to work on windows!
- assumeFalse("windows is not supported", Constants.WINDOWS);
Path path = createTempDir();
WindowsFS provider = new WindowsFS(path.getFileSystem());
Directory dir1 = new NIOFSDirectory(provider.wrapPath(path));
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java b/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java
index ebacd6a17b7..27343b0c628 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java
@@ -21,10 +21,10 @@ import java.nio.file.CopyOption;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.attribute.BasicFileAttributeView;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
/**
* FileSystem that (imperfectly) acts like windows.
@@ -55,10 +55,10 @@ public class WindowsFS extends HandleTrackingFS {
/** Returns file "key" (e.g. inode) for the specified path */
private Object getKey(Path existing) throws IOException {
- BasicFileAttributeView view =
- Files.getFileAttributeView(existing, BasicFileAttributeView.class);
- BasicFileAttributes attributes = view.readAttributes();
- return attributes.fileKey();
+ // the key may be null, e.g. on real Windows!
+ // in that case we fallback to the file path as key.
+ return Optional.ofNullable(Files.readAttributes(existing, BasicFileAttributes.class).fileKey())
+ .orElse(existing);
}
@Override
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java b/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java
index a92fcb2887f..5c13475a08f 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java
@@ -18,20 +18,17 @@ package org.apache.lucene.tests.mockfile;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Set;
class WindowsPath extends FilterPath {
- static HashSet<Character> RESERVED_CHARACTERS =
- new HashSet<>(Arrays.asList('<', '>', ':', '\"', '\\', '|', '?', '*'));
+ static final Set<Character> RESERVED_CHARACTERS =
+ Set.of('<', '>', ':', '\"', '\\', '|', '?', '*');
- static HashSet<String> RESERVED_NAMES =
- new HashSet<>(
- Arrays.asList(
- "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
- "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8",
- "LPT9"));
+ static final Set<String> RESERVED_NAMES =
+ Set.of(
+ "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
+ "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9");
WindowsPath(Path path, FilterFileSystem fileSystem) {
super(path, fileSystem);
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/util/TestRuleTemporaryFilesCleanup.java b/lucene/test-framework/src/java/org/apache/lucene/tests/util/TestRuleTemporaryFilesCleanup.java
index 6d1b6ad1f3a..517fcdb99cb 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/util/TestRuleTemporaryFilesCleanup.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/util/TestRuleTemporaryFilesCleanup.java
@@ -43,7 +43,6 @@ import org.apache.lucene.tests.mockfile.WindowsFS;
import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems;
import org.apache.lucene.tests.util.LuceneTestCase.SuppressFsync;
import org.apache.lucene.tests.util.LuceneTestCase.SuppressTempFileChecks;
-import org.apache.lucene.util.Constants;
import org.apache.lucene.util.IOUtils;
/**
@@ -157,11 +156,8 @@ final class TestRuleTemporaryFilesCleanup extends TestRuleAdapter {
fs = new HandleLimitFS(fs, limit).getFileSystem(null);
}
// windows is currently slow
- if (random.nextInt(10) == 0) {
- // don't try to emulate windows on windows: they don't get along
- if (!Constants.WINDOWS && allowed(avoid, WindowsFS.class)) {
- fs = new WindowsFS(fs).getFileSystem(null);
- }
+ if (random.nextInt(10) == 0 && allowed(avoid, WindowsFS.class)) {
+ fs = new WindowsFS(fs).getFileSystem(null);
}
if (allowed(avoid, ExtrasFS.class)) {
fs = new ExtrasFS(fs, random.nextInt(4) == 0, random.nextBoolean()).getFileSystem(null);
diff --git a/lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java b/lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java
index 2dbba2d1ad1..ac2bce53a24 100644
--- a/lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java
+++ b/lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java
@@ -29,6 +29,7 @@ import java.nio.file.StandardCopyOption;
import java.util.Random;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Predicate;
import org.apache.lucene.util.Constants;
/** Basic tests for WindowsFS */
@@ -37,11 +38,6 @@ public class TestWindowsFS extends MockFileSystemTestCase {
@Override
public void setUp() throws Exception {
super.setUp();
- // irony: currently we don't emulate windows well enough to work on windows!
- // TODO: Can we fork this class and create a new class with only those tests that can run on
- // Windows and then check if
- // the Lucene WindowsFS error is same as the one OG Windows throws
- assumeFalse("windows is not supported", Constants.WINDOWS);
}
@Override
@@ -188,8 +184,12 @@ public class TestWindowsFS extends MockFileSystemTestCase {
}
public void testFileName() {
- Character[] reservedCharacters = WindowsPath.RESERVED_CHARACTERS.toArray(new Character[0]);
- String[] reservedNames = WindowsPath.RESERVED_NAMES.toArray(new String[0]);
+ Character[] reservedCharacters =
+ WindowsPath.RESERVED_CHARACTERS.stream()
+ .filter(Predicate.not(Character.valueOf('\\')::equals))
+ .sorted()
+ .toArray(Character[]::new);
+ String[] reservedNames = WindowsPath.RESERVED_NAMES.stream().sorted().toArray(String[]::new);
String fileName;
Random r = random();
Path dir = wrap(createTempDir());
@@ -211,7 +211,11 @@ public class TestWindowsFS extends MockFileSystemTestCase {
expectThrows(InvalidPathException.class, () -> dir.resolve("foo:bar:tar"));
expectThrows(InvalidPathException.class, () -> dir.resolve("foo?bar"));
expectThrows(InvalidPathException.class, () -> dir.resolve("foo<bar"));
- expectThrows(InvalidPathException.class, () -> dir.resolve("foo\\bar"));
+ if (!Constants.WINDOWS) {
+ // we need to exclude that test on Windows, because the backslash is resolved before our code
+ // checks the filename:
+ expectThrows(InvalidPathException.class, () -> dir.resolve("foo\\bar"));
+ }
expectThrows(InvalidPathException.class, () -> dir.resolve("foo*bar|tar"));
expectThrows(InvalidPathException.class, () -> dir.resolve("foo|bar?tar"));
}