Fix case-sensitive filesystem creating duplicate directories #941
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package app.gamenative.utils | ||
|
|
||
| import okio.FileSystem | ||
| import okio.ForwardingFileSystem | ||
| import okio.Path | ||
| import java.util.concurrent.ConcurrentHashMap | ||
|
|
||
| /** | ||
| * Okio [FileSystem] wrapper that resolves each path component against on-disk | ||
| * casing before delegating to [FileSystem.SYSTEM]. Prevents duplicate directories | ||
| * when Steam depot manifests use different casing than what's already installed | ||
| * (e.g. DLC referencing `_Work` when the base game created `_work`). | ||
| * | ||
| * Resolved segments are cached for the lifetime of this instance (one download | ||
| * session) so repeated lookups for the same parent+segment are O(1). | ||
| */ | ||
| class CaseInsensitiveFileSystem( | ||
| delegate: FileSystem = SYSTEM, | ||
| ) : ForwardingFileSystem(delegate) { | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // (parent, lowercased segment) → resolved child path. | ||
| // keyed by lowercase so all casing variants ("Saves", "saves", "SAVES") hit | ||
| // the same entry. computeIfAbsent is atomic on ConcurrentHashMap, so | ||
| // concurrent threads won't race to create duplicate directories. | ||
| private val cache = ConcurrentHashMap<Pair<Path, String>, Path>() | ||
|
|
||
| override fun onPathParameter(path: Path, functionName: String, parameterName: String): Path { | ||
| val root = path.root ?: return path | ||
| var resolved = root | ||
| for (segment in path.segments) { | ||
| val key = resolved to segment.lowercase() | ||
| resolved = cache.computeIfAbsent(key) { | ||
| // fast path: exact casing exists | ||
| val exact = resolved / segment | ||
| if (delegate.metadataOrNull(exact) != null) { | ||
| exact | ||
| } else { | ||
| // slow path: list parent and match case-insensitively. | ||
| // if multiple entries match (e.g. leftover _Work + _work from a | ||
| // prior bug), the first one returned by the filesystem wins — | ||
| // non-deterministic but unavoidable without deeper heuristics. | ||
| delegate.listOrNull(resolved) | ||
| ?.firstOrNull { it.name.equals(segment, ignoreCase = true) } | ||
| ?: exact | ||
|
Comment on lines
+38
to
+44
Owner
There was a problem hiding this comment. how slow is the "slow" path? Worried it might affect external storage badly. Having said that case sensitivitiy bugs are worse than slow SD reads/writes
Contributor
Author
There was a problem hiding this comment. Slow = one With the ConcurrentHashMap cache, each unique (parent, segment) pair only hits the slow path once per download session. Subsequent lookups are O(1). On SD cards the first |
||
| } | ||
| } | ||
| } | ||
| return resolved | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| package app.gamenative.utils | ||
|
|
||
| import okio.Path.Companion.toOkioPath | ||
| import okio.Path.Companion.toPath | ||
| import org.junit.After | ||
| import org.junit.Assert.* | ||
| import org.junit.Before | ||
| import org.junit.Test | ||
| import java.io.File | ||
|
|
||
| /** | ||
| * Simulates a depot download where manifests reference the same directory tree | ||
| * with different casing (e.g. base game creates "Game/_work", DLC references | ||
| * "Game/_Work"). Verifies CaseInsensitiveFileSystem resolves both to a single | ||
| * on-disk directory. | ||
| * | ||
| * LIMITATION: on macOS (case-insensitive FS) these tests pass trivially because | ||
| * the OS itself prevents duplicate-cased dirs. The tests are meaningful on | ||
| * Linux/Android (case-sensitive FS) where duplicates would actually be created | ||
| * without CaseInsensitiveFileSystem. CI typically runs on Linux. | ||
| */ | ||
| class CaseInsensitiveFileSystemTest { | ||
|
|
||
| private lateinit var tmpDir: File | ||
| private lateinit var fs: CaseInsensitiveFileSystem | ||
|
|
||
| @Before | ||
| fun setUp() { | ||
| tmpDir = createTempDir("depot_test") | ||
| fs = CaseInsensitiveFileSystem() | ||
| } | ||
|
|
||
| @After | ||
| fun tearDown() { | ||
| tmpDir.deleteRecursively() | ||
| } | ||
|
|
||
| @Test | ||
| fun `mixed-case writes land in single directory tree`() { | ||
| val root = tmpDir.toOkioPath() | ||
|
|
||
| // base game creates directories with one casing | ||
| val baseDir = root / "steamapps" / "common" / "MyGame" / "_work" / "data" | ||
| fs.createDirectories(baseDir) | ||
| fs.write(baseDir / "base.pak") { writeUtf8("base") } | ||
|
|
||
| // DLC references same tree with different casing | ||
| val dlcDir = root / "steamapps" / "common" / "MyGame" / "_Work" / "Data" | ||
| fs.createDirectories(dlcDir) | ||
| fs.write(dlcDir / "dlc.pak") { writeUtf8("dlc") } | ||
|
|
||
| // verify only one directory tree exists on disk | ||
| val gameDir = File(tmpDir, "steamapps/common/MyGame") | ||
| val subdirs = gameDir.listFiles()?.filter { it.isDirectory } ?: emptyList() | ||
|
|
||
| // on case-sensitive FS with the fix: 1 dir. on case-insensitive FS: trivially 1 dir. | ||
| // without the fix on case-sensitive FS: would be 2 (_work and _Work) | ||
| assertEquals( | ||
| "expected single directory, got: ${subdirs.map { it.name }}", | ||
| 1, | ||
| subdirs.size, | ||
| ) | ||
|
|
||
| // both files should be reachable under the single tree | ||
| val workDir = subdirs[0] | ||
| val dataDir = workDir.listFiles()?.filter { it.isDirectory } ?: emptyList() | ||
| assertEquals( | ||
| "expected single data dir, got: ${dataDir.map { it.name }}", | ||
| 1, | ||
| dataDir.size, | ||
| ) | ||
|
|
||
| val files = dataDir[0].listFiles()?.map { it.name }?.sorted() ?: emptyList() | ||
| assertEquals(listOf("base.pak", "dlc.pak"), files) | ||
| } | ||
|
|
||
| @Test | ||
| fun `concurrent mixed-case writes from multiple threads`() { | ||
| val root = tmpDir.toOkioPath() | ||
| val base = root / "steamapps" / "common" / "TestGame" | ||
| fs.createDirectories(base) | ||
|
|
||
| // simulate concurrent depot workers writing with different casing | ||
| val threads = listOf( | ||
| Thread { | ||
| val dir = base / "Saves" / "Profile" | ||
| fs.createDirectories(dir) | ||
| fs.write(dir / "slot1.sav") { writeUtf8("save1") } | ||
| }, | ||
| Thread { | ||
| val dir = base / "saves" / "profile" | ||
| fs.createDirectories(dir) | ||
| fs.write(dir / "slot2.sav") { writeUtf8("save2") } | ||
| }, | ||
| Thread { | ||
| val dir = base / "SAVES" / "PROFILE" | ||
| fs.createDirectories(dir) | ||
| fs.write(dir / "slot3.sav") { writeUtf8("save3") } | ||
| }, | ||
| ) | ||
|
|
||
| threads.forEach { it.start() } | ||
| threads.forEach { it.join() } | ||
|
|
||
| val gameDir = File(tmpDir, "steamapps/common/TestGame") | ||
| val saveDirs = gameDir.listFiles()?.filter { it.isDirectory } ?: emptyList() | ||
| assertEquals( | ||
| "expected single saves dir, got: ${saveDirs.map { it.name }}", | ||
| 1, | ||
| saveDirs.size, | ||
| ) | ||
|
|
||
| val profileDirs = saveDirs[0].listFiles()?.filter { it.isDirectory } ?: emptyList() | ||
| assertEquals( | ||
| "expected single profile dir, got: ${profileDirs.map { it.name }}", | ||
| 1, | ||
| profileDirs.size, | ||
| ) | ||
|
|
||
| val files = profileDirs[0].listFiles()?.map { it.name }?.sorted() ?: emptyList() | ||
| assertEquals(listOf("slot1.sav", "slot2.sav", "slot3.sav"), files) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does it work with DD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tiny DD change was made and the pointer updated in Gradle deps. works great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the
CaseInsensitiveFileSystemis passed as thefilesystemparam toDepotDownloader, which uses Okio. So all file writes during download go through the case-insensitive resolver, which means that DD writes to the correctly-cased existing directory instead of creating a new one.