Workaround probable Windows bug in path equivalency checks

In Windows, std::filesystem::equivalent throws
std::system_error if a path contains characters that can't be
represented in the system code page (e.g. Windows 1252). This is
undocumented and seems unnecessary as Microsoft's own docs say Windows
paths are stored natively in Unicode, so it sounds like a bug to me.

To work around this, return true early if the path objects are equal,
so std::filesystem::equivalent doesn't need to be called, and catch
system_error if it does need to be called - this may result in false
negatives, but for LOOT's purposes that is fine and is very unlikely
anyway.

src/api/plugin.cpp

@@ -304,12 +304,26 @@ std::filesystem::path replaceExtension(std::filesystem::path path, const std::st
 }
 
 bool equivalent(const std::filesystem::path& path1, const std::filesystem::path& path2) {
+  // If the paths are identical, they've got to be equivalent,
+  // it doesn't matter if the paths exist or not.
+  if (path1 == path2) {
+    return true;
+  }
+  // If the paths are not identical, the filesystem might be case-insensitive
+  // so check with the filesystem.
   try {
     return std::filesystem::equivalent(path1, path2);
   } catch (std::filesystem::filesystem_error) {
     // One of the paths checked for equivalence doesn't exist,
     // so they can't be equivalent.
     return false;
+  } catch (std::system_error) {
+    // This can be thrown if one or both of the paths contains a character
+    // that can't be represented in Windows' multi-byte code page (e.g.
+    // Windows-1252), even though Unicode paths shouldn't be a problem,
+    // and throwing system_error is undocumented. Seems like a bug in MSVC's
+    // implementation.
+    return false;
   }
 }
 

src/tests/api/internals/game/game_test.h

@@ -177,6 +177,17 @@ TEST_P(GameTest,
   EXPECT_EQ(1, game.GetCache()->GetArchivePaths().size());
 }
 
+TEST_P(GameTest,
+  loadPluginsShouldNotThrowIfAFilenameHasNonWindows1252EncodableCharacters) {
+  auto path = dataPath / std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+  std::ofstream out(path);
+  out.close();
+
+  Game game = Game(GetParam(), dataPath.parent_path(), localPath);
+
+  EXPECT_NO_THROW(loadInstalledPlugins(game, false));
+}
+
 TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasNotBeenLoaded) {
   Game game = Game(GetParam(), dataPath.parent_path(), localPath);
   game.LoadCurrentLoadOrderState();

src/tests/api/internals/main.cpp

@@ -164,6 +164,20 @@ TEST(Filesystem, equivalentShouldRequireThatBothPathsExist) {
 
   EXPECT_THROW(std::filesystem::equivalent(lower, upper), std::filesystem::filesystem_error);
 }
+
+TEST(Filesystem, equivalentShouldBeCaseInsensitive) {
+  auto upper = std::filesystem::path("LICENSE");
+  auto lower = std::filesystem::path("license");
+
+  EXPECT_TRUE(std::filesystem::equivalent(lower, upper));
+}
+
+TEST(Filesystem, equivalentCannotHandleCharactersThatAreUnrepresentableInTheSystemCodePage) {
+  auto path1 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+  auto path2 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+
+  EXPECT_THROW(std::filesystem::equivalent(path1, path2), std::system_error);
+}
 #else
 TEST(Filesystem, equivalentShouldNotRequireThatBothPathsExist) {
   auto upper = std::filesystem::path("LICENSE");
@@ -171,6 +185,13 @@ TEST(Filesystem, equivalentShouldNotRequireThatBothPathsExist) {
 
   EXPECT_FALSE(std::filesystem::equivalent(lower, upper));
 }
+
+TEST(Filesystem, equivalentShouldBeCaseSensitive) {
+  auto upper = std::filesystem::path("LICENSE");
+  auto lower = std::filesystem::path("license");
+
+  EXPECT_FALSE(std::filesystem::equivalent(lower, upper));
+}
 #endif
 
 TEST(Filesystem, canonicalShouldRequireThatThePathExists) {
@@ -178,13 +199,6 @@ TEST(Filesystem, canonicalShouldRequireThatThePathExists) {
 }
 
 #ifdef _WIN32
-TEST(Filesystem, equivalentShouldBeCaseInsensitive) {
-  auto upper = std::filesystem::path("LICENSE");
-  auto lower = std::filesystem::path("license");
-
-  EXPECT_TRUE(std::filesystem::equivalent(lower, upper));
-}
-
 TEST(Filesystem, canonicalShouldFoldCase) {
   auto upper = std::filesystem::canonical("LICENSE");
   auto lower = std::filesystem::canonical("license");
@@ -192,13 +206,6 @@ TEST(Filesystem, canonicalShouldFoldCase) {
   EXPECT_EQ(lower, upper);
 }
 #else
-TEST(Filesystem, equivalentShouldBeCaseSensitive) {
-  auto upper = std::filesystem::path("LICENSE");
-  auto lower = std::filesystem::path("license");
-
-  EXPECT_FALSE(std::filesystem::equivalent(lower, upper));
-}
-
 TEST(Filesystem, canonicalShouldNotFoldCase) {
   std::ofstream out("license");
   out.close();

src/tests/api/internals/plugin_test.h

@@ -523,6 +523,71 @@ TEST_P(
   EXPECT_EQ(GetParam() == GameType::fo4 || GetParam() == GameType::tes5se,
             result);
 }
+
+TEST(equivalent, shouldReturnTrueIfGivenEqualPathsThatExist) {
+  auto path1 = std::filesystem::path("LICENSE");
+  auto path2 = std::filesystem::path("LICENSE");
+
+  EXPECT_TRUE(loot::equivalent(path1, path2));
+}
+
+TEST(equivalent, shouldReturnTrueIfGivenEqualPathsThatDoNotExist) {
+  auto path1 = std::filesystem::path("LICENSE2");
+  auto path2 = std::filesystem::path("LICENSE2");
+
+  EXPECT_TRUE(loot::equivalent(path1, path2));
+}
+
+TEST(equivalent, shouldReturnFalseIfPathsAreNotCaseInsensitivelyEqual) {
+  auto upper = std::filesystem::path("LICENSE");
+  auto lower = std::filesystem::path("license2");
+
+  EXPECT_FALSE(loot::equivalent(lower, upper));
+}
+
+#ifdef _WIN32
+TEST(equivalent, shouldReturnTrueIfGivenCaseInsensitivelyEqualPathsThatExist) {
+  auto upper = std::filesystem::path("LICENSE");
+  auto lower = std::filesystem::path("license");
+
+  EXPECT_TRUE(loot::equivalent(lower, upper));
+}
+
+TEST(equivalent, shouldReturnFalseIfGivenCaseInsensitivelyEqualPathsThatDoNotExist) {
+  auto upper = std::filesystem::path("LICENSE2");
+  auto lower = std::filesystem::path("license2");
+
+  EXPECT_FALSE(loot::equivalent(lower, upper));
+}
+
+TEST(equivalent, shouldReturnTrueIfEqualPathsHaveCharactersThatAreUnrepresentableInTheSystemMultiByteCodePage) {
+  auto path1 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+  auto path2 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+
+  EXPECT_TRUE(loot::equivalent(path1, path2));
+}
+
+TEST(equivalent, shouldReturnFalseIfCaseInsensitivelyEqualPathsHaveCharactersThatAreUnrepresentableInTheSystemMultiByteCodePage) {
+  auto path1 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00E3\u00CE.txt");
+  auto path2 = std::filesystem::u8path(u8"\u2551\u00BB\u00C1\u2510\u2557\u00FE\u00C3\u00CE.txt");
+
+  EXPECT_FALSE(loot::equivalent(path1, path2));
+}
+#else
+TEST(equivalent, shouldReturnFalseIfGivenCaseInsensitivelyEqualPathsThatExist) {
+  auto upper = std::filesystem::path("LICENSE");
+  auto lower = std::filesystem::path("license");
+
+  EXPECT_FALSE(loot::equivalent(lower, upper));
+}
+
+TEST(equivalent, shouldReturnFalseIfGivenCaseInsensitivelyEqualPathsThatDoNotExist) {
+  auto upper = std::filesystem::path("LICENSE2");
+  auto lower = std::filesystem::path("license2");
+
+  EXPECT_FALSE(loot::equivalent(lower, upper));
+}
+#endif
 }
 }