From 6bc6113bb78f794ee8e1680db6ef4de9657be602 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 27 Sep 2025 23:04:30 +0100 Subject: [PATCH] Test with long paths on Windows Long paths can be greater than 260 characters long, but each path component is still limited to 255 characters. The latter is also commonly the component length limit on Linux. Support for long paths requires Windows 10 1607 or later and a Registry value to be set[1], so to make the cause of test failures clearer when that isn't the case, there's one test that checks that Registry value is set, and long paths are not used in the other tests if it isn't set, so only that one test should failed if the system isn't configured as expected. GitHub Action's Windows runners do have the Registry value set[2]. This doesn't set the manifest for the Rust tests because it seems to be very difficult to set a manifest for only the tests, and it's not worth the effort when the only tests that fail are those that try to create a symlink, especially since those cases are also covered by the C++ tests. [1]: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry#enable-long-paths-in-windows-10-version-1607-and-later [2]: https://github.com/actions/runner-images/blob/releases/win22/20250921/images/windows/scripts/build/Configure-BaseImage.ps1#L73 --- cpp/cmake/tests.cmake | 3 + cpp/src/tests/api/interface/main.cpp | 6 + cpp/src/tests/libloot_tests.manifest | 8 ++ cpp/src/tests/test_helpers.h | 32 +++++- src/game.rs | 33 +++--- src/tests.rs | 163 +++++++++++++++++---------- 6 files changed, 168 insertions(+), 77 deletions(-) create mode 100644 cpp/src/tests/libloot_tests.manifest diff --git a/cpp/cmake/tests.cmake b/cpp/cmake/tests.cmake index 99a23c6d..2baff82c 100644 --- a/cpp/cmake/tests.cmake +++ b/cpp/cmake/tests.cmake @@ -66,6 +66,9 @@ set(LIBLOOT_INTERFACE_TESTS_ALL_SOURCES "${PROJECT_SOURCE_DIR}/src/tests/test_helpers.h" "${PROJECT_SOURCE_DIR}/src/tests/printers.h") +if(MSVC) + list(APPEND LIBLOOT_INTERFACE_TESTS_ALL_SOURCES "${PROJECT_SOURCE_DIR}/src/tests/libloot_tests.manifest") +endif() ############################## # Define Targets diff --git a/cpp/src/tests/api/interface/main.cpp b/cpp/src/tests/api/interface/main.cpp index 9444f1eb..1df259b0 100644 --- a/cpp/src/tests/api/interface/main.cpp +++ b/cpp/src/tests/api/interface/main.cpp @@ -113,6 +113,12 @@ TEST(Filesystem, u8pathConvertsCharacterEncodingFromUtf8ToNative) { EXPECT_EQ(utf16, path.u16string()); } +#ifdef _WIN32 +TEST(WindowsRegistry, hasLongPathsEnabled) { + EXPECT_TRUE(loot::test::windowsHasLongPathsEnabled()); +} +#endif + namespace loot { namespace test { void testLoggingCallback(LogLevel, std::string_view) { diff --git a/cpp/src/tests/libloot_tests.manifest b/cpp/src/tests/libloot_tests.manifest new file mode 100644 index 00000000..9e2a2728 --- /dev/null +++ b/cpp/src/tests/libloot_tests.manifest @@ -0,0 +1,8 @@ + + + + + true + + + diff --git a/cpp/src/tests/test_helpers.h b/cpp/src/tests/test_helpers.h index 9b8d5a68..f8cec155 100644 --- a/cpp/src/tests/test_helpers.h +++ b/cpp/src/tests/test_helpers.h @@ -31,6 +31,10 @@ along with LOOT. If not, see #include "loot/enum/game_type.h" +#ifdef _WIN32 +#include +#endif + namespace loot::test { bool supportsLightPlugins(GameType gameType) { return gameType == GameType::tes5se || gameType == GameType::tes5vr || @@ -63,6 +67,22 @@ std::filesystem::path getSourceArchivesPath(GameType gameType) { } } +#ifdef _WIN32 +bool windowsHasLongPathsEnabled() { + DWORD value = 0; + DWORD valueSize = sizeof(value); + LONG ret = RegGetValue(HKEY_LOCAL_MACHINE, + L"SYSTEM\\CurrentControlSet\\Control\\FileSystem", + L"LongPathsEnabled", + RRF_RT_REG_DWORD, + NULL, + &value, + &valueSize); + + return ret == ERROR_SUCCESS && value == 1; +} +#endif + std::filesystem::path getRootTestPath() { std::random_device randomDevice; std::default_random_engine prng(randomDevice()); @@ -77,8 +97,16 @@ std::filesystem::path getRootTestPath() { directoryName.push_back(static_cast(dist(prng))); } - return std::filesystem::absolute(std::filesystem::temp_directory_path() / - std::filesystem::u8path(directoryName)); + auto tempPath = std::filesystem::temp_directory_path() / + std::filesystem::u8path(directoryName); + +#ifdef _WIN32 + if (windowsHasLongPathsEnabled()) { + tempPath /= std::filesystem::u8path(std::string(255, 'a')); + } +#endif + + return std::filesystem::absolute(tempPath); } } diff --git a/src/game.rs b/src/game.rs index 4b2ad4b1..aac48daf 100644 --- a/src/game.rs +++ b/src/game.rs @@ -834,6 +834,8 @@ mod tests { use super::*; + const MAX_PATH_COMPONENT: usize = 255; + #[cfg(windows)] fn symlink_dir(original: &Path, link: &Path) { std::os::windows::fs::symlink_dir(original, link).unwrap(); @@ -957,13 +959,14 @@ mod tests { #[test] fn should_succeed_if_given_a_symlink_path() { - let fixture = Fixture::new(GameType::Morrowind); + let fixture = Fixture::without_long_paths(GameType::Morrowind); - let game_path = fixture.game_path.with_extension("symlink"); - symlink_dir(&fixture.game_path, &game_path); - assert!(game_path.is_symlink()); + let symlink_game_path = fixture.root_path().join("a".repeat(MAX_PATH_COMPONENT)); - assert!(Game::new(fixture.game_type, &game_path).is_ok()); + symlink_dir(&fixture.game_path, &symlink_game_path); + assert!(symlink_game_path.is_symlink()); + + assert!(Game::new(fixture.game_type, &symlink_game_path).is_ok()); } #[cfg(windows)] @@ -1036,17 +1039,21 @@ mod tests { #[test] fn should_succeed_if_given_symlink_paths() { - let fixture = Fixture::new(GameType::Oblivion); + let fixture = Fixture::without_long_paths(GameType::Oblivion); - let game_path = fixture.game_path.with_extension("symlink"); - symlink_dir(&fixture.game_path, &game_path); - assert!(game_path.is_symlink()); + let symlink_game_path = fixture.root_path().join("a".repeat(MAX_PATH_COMPONENT)); + symlink_dir(&fixture.game_path, &symlink_game_path); + assert!(symlink_game_path.is_symlink()); - let local_path = fixture.local_path.with_extension("symlink"); - symlink_dir(&fixture.local_path, &local_path); - assert!(local_path.is_symlink()); + let symlink_local_path = fixture.root_path().join("b".repeat(MAX_PATH_COMPONENT)); + symlink_dir(&fixture.local_path, &symlink_local_path); + assert!(symlink_local_path.is_symlink()); - let game = Game::with_local_path(fixture.game_type, &game_path, &local_path); + let game = Game::with_local_path( + fixture.game_type, + &symlink_game_path, + &symlink_local_path, + ); assert!(game.is_ok()); } diff --git a/src/tests.rs b/src/tests.rs index abf607a4..b8f241c5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -177,35 +177,119 @@ fn data_path(game_type: GameType, game_path: &Path) -> PathBuf { } pub(crate) struct Fixture { - _temp_dir: TempDir, + temp_dir: TempDir, pub(crate) game_type: GameType, pub(crate) game_path: PathBuf, pub(crate) local_path: PathBuf, } +fn copy_starfield_plugins(source_plugins_path: &Path, data_path: &Path) { + copy_file(source_plugins_path, data_path, BLANK_FULL_ESM); + copy_file(source_plugins_path, data_path, BLANK_MEDIUM_ESM); + + copy( + source_plugins_path.join(BLANK_FULL_ESM), + data_path.join(BLANK_ESM), + ) + .unwrap(); + copy( + source_plugins_path.join(BLANK_FULL_ESM), + data_path.join(BLANK_DIFFERENT_ESM), + ) + .unwrap(); + copy( + source_plugins_path.join("Blank - Override.full.esm"), + data_path.join(BLANK_MASTER_DEPENDENT_ESM), + ) + .unwrap(); + + copy_file(source_plugins_path, data_path, BLANK_ESP); + copy( + source_plugins_path.join(BLANK_ESP), + data_path.join(BLANK_DIFFERENT_ESP), + ) + .unwrap(); + copy( + source_plugins_path.join(BLANK_OVERRIDE_ESP), + data_path.join(BLANK_MASTER_DEPENDENT_ESP), + ) + .unwrap(); +} + +fn copy_plugins(source_plugins_path: &Path, data_path: &Path) { + copy_file(source_plugins_path, data_path, BLANK_ESM); + copy_file(source_plugins_path, data_path, BLANK_DIFFERENT_ESM); + copy_file(source_plugins_path, data_path, BLANK_MASTER_DEPENDENT_ESM); + copy_file( + source_plugins_path, + data_path, + BLANK_DIFFERENT_MASTER_DEPENDENT_ESM, + ); + + copy_file(source_plugins_path, data_path, BLANK_ESP); + copy_file(source_plugins_path, data_path, BLANK_DIFFERENT_ESP); + copy_file(source_plugins_path, data_path, BLANK_MASTER_DEPENDENT_ESP); + copy_file( + source_plugins_path, + data_path, + BLANK_DIFFERENT_MASTER_DEPENDENT_ESP, + ); + copy_file(source_plugins_path, data_path, BLANK_PLUGIN_DEPENDENT_ESP); + copy_file( + source_plugins_path, + data_path, + BLANK_DIFFERENT_PLUGIN_DEPENDENT_ESP, + ); +} + impl Fixture { + const PREFIX: &str = "libloot-t\u{00E9}st-"; + pub(crate) fn new(game_type: GameType) -> Self { let temp_dir = tempfile::Builder::new() - .prefix("libloot-t\u{00E9}st-") + .prefix(Self::PREFIX) .tempdir() .unwrap(); - Self::with_tempdir(game_type, temp_dir) + Self::with_tempdir(game_type, temp_dir, true) } - pub(crate) fn in_path(game_type: GameType, in_path: &Path) -> Fixture { + /// This creates the fixture without intentionally creating paths that are + /// more than 260 characters long. It's intended for use when testing + /// symlinks, because Windows will only allow a test to create a symlink to + /// a long path if Windows is configured with long paths enabled and the + /// test executable also has a manifest that sets longPathAware to true, but + /// embedding a manifest isn't straightforward in Rust without the use of + /// build dependencies, which would also be built for non-test code. + /// The same functionality is more easily tested in the C++ wrapper, so it's + /// done there instead. + pub(crate) fn without_long_paths(game_type: GameType) -> Self { let temp_dir = tempfile::Builder::new() - .prefix("libloot-t\u{00E9}st-") + .prefix(Self::PREFIX) + .tempdir() + .unwrap(); + + Self::with_tempdir(game_type, temp_dir, false) + } + + pub(crate) fn in_path(game_type: GameType, in_path: &Path) -> Self { + let temp_dir = tempfile::Builder::new() + .prefix(Self::PREFIX) .tempdir_in(in_path) .unwrap(); - Self::with_tempdir(game_type, temp_dir) + Self::with_tempdir(game_type, temp_dir, true) } - fn with_tempdir(game_type: GameType, temp_dir: TempDir) -> Fixture { - let root_path = temp_dir.path(); - let game_path = root_path.join("games/game"); - let local_path = root_path.join("local/game"); + fn with_tempdir(game_type: GameType, temp_dir: TempDir, use_long_path: bool) -> Self { + let base_path = if use_long_path { + temp_dir.path().join("a".repeat(255)) + } else { + temp_dir.path().to_path_buf() + }; + + let game_path = base_path.join("games").join("game"); + let local_path = base_path.join("local").join("game"); let data_path = data_path(game_type, &game_path); create_dir_all(&data_path).unwrap(); @@ -214,58 +298,9 @@ impl Fixture { let source_plugins_path = source_plugins_path(game_type); if game_type == GameType::Starfield { - copy_file(&source_plugins_path, &data_path, BLANK_FULL_ESM); - copy_file(&source_plugins_path, &data_path, BLANK_MEDIUM_ESM); - - copy( - source_plugins_path.join(BLANK_FULL_ESM), - data_path.join(BLANK_ESM), - ) - .unwrap(); - copy( - source_plugins_path.join(BLANK_FULL_ESM), - data_path.join(BLANK_DIFFERENT_ESM), - ) - .unwrap(); - copy( - source_plugins_path.join("Blank - Override.full.esm"), - data_path.join(BLANK_MASTER_DEPENDENT_ESM), - ) - .unwrap(); - copy_file(&source_plugins_path, &data_path, BLANK_ESP); - copy( - source_plugins_path.join(BLANK_ESP), - data_path.join(BLANK_DIFFERENT_ESP), - ) - .unwrap(); - copy( - source_plugins_path.join(BLANK_OVERRIDE_ESP), - data_path.join(BLANK_MASTER_DEPENDENT_ESP), - ) - .unwrap(); + copy_starfield_plugins(&source_plugins_path, &data_path); } else { - copy_file(&source_plugins_path, &data_path, BLANK_ESM); - copy_file(&source_plugins_path, &data_path, BLANK_DIFFERENT_ESM); - copy_file(&source_plugins_path, &data_path, BLANK_MASTER_DEPENDENT_ESM); - copy_file( - &source_plugins_path, - &data_path, - BLANK_DIFFERENT_MASTER_DEPENDENT_ESM, - ); - copy_file(&source_plugins_path, &data_path, BLANK_ESP); - copy_file(&source_plugins_path, &data_path, BLANK_DIFFERENT_ESP); - copy_file(&source_plugins_path, &data_path, BLANK_MASTER_DEPENDENT_ESP); - copy_file( - &source_plugins_path, - &data_path, - BLANK_DIFFERENT_MASTER_DEPENDENT_ESP, - ); - copy_file(&source_plugins_path, &data_path, BLANK_PLUGIN_DEPENDENT_ESP); - copy_file( - &source_plugins_path, - &data_path, - BLANK_DIFFERENT_PLUGIN_DEPENDENT_ESP, - ); + copy_plugins(&source_plugins_path, &data_path); } if supports_light_plugins(game_type) { @@ -307,7 +342,7 @@ impl Fixture { .unwrap(); Self { - _temp_dir: temp_dir, + temp_dir, game_type, game_path, local_path, @@ -317,6 +352,10 @@ impl Fixture { pub(crate) fn data_path(&self) -> PathBuf { data_path(self.game_type, &self.game_path) } + + pub(crate) fn root_path(&self) -> &Path { + self.temp_dir.path() + } } #[test_parameter]