fix: replace hardcoded app ID paths with BuildConfig.APPLICATION_ID#585
fix: replace hardcoded app ID paths with BuildConfig.APPLICATION_ID#585jeremybernstein wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
builds with applicationIdSuffix (.debug, .gold) break gamepad input, Wine E: drive mount, DXVK cache, and mediaconv paths. use BuildConfig.APPLICATION_ID / EVSHIM_DATA_DIR env var instead.
📝 WalkthroughWalkthroughThis PR replaces hardcoded application data directory paths throughout the codebase with dynamic paths derived from BuildConfig.APPLICATION_ID, and introduces EVSHIM_DATA_DIR environment variable support for runtime configuration of the data directory. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/winlator/core/WineUtils.java">
<violation number="1" location="app/src/main/java/com/winlator/core/WineUtils.java:41">
P2: Existing containers with an E: drive already set will not be migrated to the new BuildConfig.APPLICATION_ID path, leaving stale E: mappings from the old hardcoded app ID.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| if (!currentDrives.contains("E:")) { | ||
| missingDrives += "E:/data/data/app.gamenative/storage"; | ||
| missingDrives += "E:/data/data/" + app.gamenative.BuildConfig.APPLICATION_ID + "/storage"; |
There was a problem hiding this comment.
P2: Existing containers with an E: drive already set will not be migrated to the new BuildConfig.APPLICATION_ID path, leaving stale E: mappings from the old hardcoded app ID.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/core/WineUtils.java, line 41:
<comment>Existing containers with an E: drive already set will not be migrated to the new BuildConfig.APPLICATION_ID path, leaving stale E: mappings from the old hardcoded app ID.</comment>
<file context>
@@ -38,7 +38,7 @@ public static void createDosdevicesSymlinks(Container container) {
}
if (!currentDrives.contains("E:")) {
- missingDrives += "E:/data/data/app.gamenative/storage";
+ missingDrives += "E:/data/data/" + app.gamenative.BuildConfig.APPLICATION_ID + "/storage";
}
String updatedDrives = missingDrives + currentDrives;
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java (2)
186-194: MovetmpDirafterimageFsis available and derive from it.
tmpDiris computed at line 186 beforecontext(line 205) andimageFs(line 206) are obtained, forcing use of the hardcoded/data/data/prefix. Moving the computation a few lines down allows usingimageFs.getRootDir(), which is context-aware and consistent with howDXVKHelper.javanow derives its paths.♻️ Proposed fix
Move
tmpDirbelow line 206 and derive fromimageFs:- String tmpDir = "/data/data/" + BuildConfig.APPLICATION_ID + "/files/imagefs/tmp"; - for (int i = 0; i < enabledPlayerCount; i++) { ... Context context = environment.getContext(); ImageFs imageFs = ImageFs.find(context); File rootDir = imageFs.getRootDir(); + String tmpDir = imageFs.getRootDir().getPath() + "/tmp"; + for (int i = 0; i < enabledPlayerCount; i++) {Note:
imageFs.getRootDir().getPath() + "/tmp"resolves to<filesDir>/imagefs/tmp, which equals the current hardcoded value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java` around lines 186 - 194, tmpDir is computed too early using a hardcoded /data/data/... path; move its declaration in BionicProgramLauncherComponent below where context and imageFs are obtained (after imageFs is available) and derive tmpDir from imageFs by using imageFs.getRootDir().getPath() + "/tmp" so the path is context-aware and consistent with DXVKHelper.java's approach; update any references to the old tmpDir variable accordingly.
306-306:EVSHIM_DATA_DIRshould also use a context-derived path.
"/data/data/" + BuildConfig.APPLICATION_IDhardcodes the/data/data/prefix. At this point in the codecontextis available (line 205), socontext.getFilesDir().getParentFile().getPath()returns the correct app data directory for all Android users.♻️ Proposed fix
- envVars.put("EVSHIM_DATA_DIR", "/data/data/" + BuildConfig.APPLICATION_ID); + envVars.put("EVSHIM_DATA_DIR", context.getFilesDir().getParentFile().getPath());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java` at line 306, EVSHIM_DATA_DIR is set with a hardcoded "/data/data/" prefix; change the assignment in BionicProgramLauncherComponent where envVars is populated to derive the app data directory from the available context (use context.getFilesDir().getParentFile().getPath()) instead of concatenating BuildConfig.APPLICATION_ID, so envVars.put("EVSHIM_DATA_DIR", ...) uses the context-derived path; ensure you reference the existing envVars map and keep BuildConfig.APPLICATION_ID out of the path composition.app/src/main/java/com/winlator/core/WineUtils.java (1)
41-53: Addimport app.gamenative.BuildConfigfor consistency.Both lines use the fully-qualified
app.gamenative.BuildConfig.APPLICATION_IDinline. Every other file changed in this PR (Container.java,WinHandler.java,BionicProgramLauncherComponent.java) importsBuildConfigat the top. Using the short name is cleaner and consistent.♻️ Proposed change
Add to imports:
+import app.gamenative.BuildConfig;Then simplify the usage:
- missingDrives += "E:/data/data/" + app.gamenative.BuildConfig.APPLICATION_ID + "/storage"; + missingDrives += "E:/data/data/" + BuildConfig.APPLICATION_ID + "/storage";- if (!linkTarget.isDirectory() && path.endsWith("/" + app.gamenative.BuildConfig.APPLICATION_ID + "/storage")) { + if (!linkTarget.isDirectory() && path.endsWith("/" + BuildConfig.APPLICATION_ID + "/storage")) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/core/WineUtils.java` around lines 41 - 53, The file WineUtils.java is using the fully-qualified app.gamenative.BuildConfig.APPLICATION_ID inline; add an import for app.gamenative.BuildConfig at the top of the file and replace occurrences of the fully-qualified name with the short BuildConfig.APPLICATION_ID to match other changed files (e.g., references in the string concatenation when computing missingDrives and the path check inside the loop over container.drivesIterator()); update all instances in WineUtils (used near container.setDrives/Log.d and the linkTarget path.endsWith check) to use BuildConfig.APPLICATION_ID.app/src/main/java/com/winlator/winhandler/WinHandler.java (1)
523-535: Preferactivity.getFilesDir()over the hardcoded/data/data/prefix.
activity(aContext) is already set in the constructor. UsinggetFilesDir()is more robust — it returns the correct path for all Android users (primary and secondary), whereas the hardcoded/data/data/prefix is only correct for user 0.♻️ Proposed fix
- String tmpDir = "/data/data/" + BuildConfig.APPLICATION_ID + "/files/imagefs/tmp"; + String tmpDir = activity.getFilesDir().getPath() + "/imagefs/tmp";This renders the
import app.gamenative.BuildConfig(line 48) removable if not needed elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/winhandler/WinHandler.java` around lines 523 - 535, Replace the hardcoded tmpDir construction that uses "/data/data/"+BuildConfig.APPLICATION_ID with a path built from the Activity context: use activity.getFilesDir() (e.g. activity.getFilesDir().getPath() + "/imagefs/tmp") when computing tmpDir so p1_mem_path, extra_mem_path and subsequent file operations (p1_memFile, RandomAccessFile, gamepadBuffer, extraGamepadBuffers) use the correct per-user files directory; ensure parent directories are still created with getParentFile().mkdirs(); after this change you can remove the now-unused BuildConfig import if it isn’t referenced elsewhere.app/src/main/cpp/extras/evshim.c (1)
169-179: LGTM — consider checkingsnprintfreturn for truncation.The
EVSHIM_DATA_DIRenv-var path is correct; the fallback default aligns with the PR description. One minor robustness note:snprintfis not checked for truncation. Ifdata_dirwere unusually long,pathwould be silently truncated andopen()would fail withENOENT, which is already caught and logged at line 186. So the failure mode is safe, but an explicit guard would be cleaner.♻️ Proposed guard
+ int written = snprintf(path, sizeof path, - snprintf(path, sizeof path, "%s/files/imagefs/tmp/gamepad%s.mem", data_dir, (i == 0) ? "" : (char[2]){'0' + i, '\0'}); + if (written < 0 || (size_t)written >= sizeof path) { + LOGE("P%d: path truncated, skipping\n", i); + continue; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/cpp/extras/evshim.c` around lines 169 - 179, Check the return value of snprintf when writing into path and handle truncation/errors: after the call to snprintf(path, sizeof path, "%s/files/imagefs/tmp/gamepad%s.mem", data_dir, (i == 0) ? "" : (char[2]){'0' + i, '\0'}) inspect the int result (rv); if rv < 0 or rv >= (int)sizeof(path) treat it as an error (log via the same logger used elsewhere, skip this player or fail gracefully) instead of relying on open() to report ENOENT, so the code using data_dir/path and the per-player loop over players detects and handles truncated paths explicitly.app/src/main/java/com/winlator/container/Container.java (1)
26-58: Note:/data/data/prefix is hardcoded in static constants.Since
Containeris a data class with noContext, usingBuildConfig.APPLICATION_IDis the correct approach andDATA_DIRis the right abstraction. However, the hardcoded/data/data/prefix will be incorrect for apps installed under a secondary Android user profile (where the correct prefix is/data/user/{userId}/). This limitation is not new — the old hardcoded string had the same issue — but worth noting since context-based derivation is now used inconsistently across the codebase.Where
Contextis unavailable (static constants), this is accepted. For places that do haveContext(seeWinHandler.java,BionicProgramLauncherComponent.java),context.getFilesDir()should be preferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/container/Container.java` around lines 26 - 58, DATA_DIR is built with a hardcoded "/data/data/" prefix which fails for secondary Android users; change DATA_DIR from a compile-time static constant to a runtime value derived from a Context (e.g., provide Container.getDataDir(Context) or a lazy-initializer that uses context.getFilesDir()/context.getDataDir()), and update dependent constants that rely on it (MEDIACONV_ENV_VARS and DEFAULT_DRIVES) to be generated at runtime via that getter; also update places that already have a Context (WinHandler, BionicProgramLauncherComponent) to call context.getFilesDir() (or getDataDir()) instead of using Container.DATA_DIR so all file paths use the correct per-user directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/cpp/extras/evshim.c`:
- Around line 169-179: Check the return value of snprintf when writing into path
and handle truncation/errors: after the call to snprintf(path, sizeof path,
"%s/files/imagefs/tmp/gamepad%s.mem", data_dir, (i == 0) ? "" : (char[2]){'0' +
i, '\0'}) inspect the int result (rv); if rv < 0 or rv >= (int)sizeof(path)
treat it as an error (log via the same logger used elsewhere, skip this player
or fail gracefully) instead of relying on open() to report ENOENT, so the code
using data_dir/path and the per-player loop over players detects and handles
truncated paths explicitly.
In `@app/src/main/java/com/winlator/container/Container.java`:
- Around line 26-58: DATA_DIR is built with a hardcoded "/data/data/" prefix
which fails for secondary Android users; change DATA_DIR from a compile-time
static constant to a runtime value derived from a Context (e.g., provide
Container.getDataDir(Context) or a lazy-initializer that uses
context.getFilesDir()/context.getDataDir()), and update dependent constants that
rely on it (MEDIACONV_ENV_VARS and DEFAULT_DRIVES) to be generated at runtime
via that getter; also update places that already have a Context (WinHandler,
BionicProgramLauncherComponent) to call context.getFilesDir() (or getDataDir())
instead of using Container.DATA_DIR so all file paths use the correct per-user
directory.
In `@app/src/main/java/com/winlator/core/WineUtils.java`:
- Around line 41-53: The file WineUtils.java is using the fully-qualified
app.gamenative.BuildConfig.APPLICATION_ID inline; add an import for
app.gamenative.BuildConfig at the top of the file and replace occurrences of the
fully-qualified name with the short BuildConfig.APPLICATION_ID to match other
changed files (e.g., references in the string concatenation when computing
missingDrives and the path check inside the loop over
container.drivesIterator()); update all instances in WineUtils (used near
container.setDrives/Log.d and the linkTarget path.endsWith check) to use
BuildConfig.APPLICATION_ID.
In `@app/src/main/java/com/winlator/winhandler/WinHandler.java`:
- Around line 523-535: Replace the hardcoded tmpDir construction that uses
"/data/data/"+BuildConfig.APPLICATION_ID with a path built from the Activity
context: use activity.getFilesDir() (e.g. activity.getFilesDir().getPath() +
"/imagefs/tmp") when computing tmpDir so p1_mem_path, extra_mem_path and
subsequent file operations (p1_memFile, RandomAccessFile, gamepadBuffer,
extraGamepadBuffers) use the correct per-user files directory; ensure parent
directories are still created with getParentFile().mkdirs(); after this change
you can remove the now-unused BuildConfig import if it isn’t referenced
elsewhere.
In
`@app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java`:
- Around line 186-194: tmpDir is computed too early using a hardcoded
/data/data/... path; move its declaration in BionicProgramLauncherComponent
below where context and imageFs are obtained (after imageFs is available) and
derive tmpDir from imageFs by using imageFs.getRootDir().getPath() + "/tmp" so
the path is context-aware and consistent with DXVKHelper.java's approach; update
any references to the old tmpDir variable accordingly.
- Line 306: EVSHIM_DATA_DIR is set with a hardcoded "/data/data/" prefix; change
the assignment in BionicProgramLauncherComponent where envVars is populated to
derive the app data directory from the available context (use
context.getFilesDir().getParentFile().getPath()) instead of concatenating
BuildConfig.APPLICATION_ID, so envVars.put("EVSHIM_DATA_DIR", ...) uses the
context-derived path; ensure you reference the existing envVars map and keep
BuildConfig.APPLICATION_ID out of the path composition.
1 participant
Problem
/data/data/app.gamenativeis hardcoded in several Java and C files. Any build variant withapplicationIdSuffix(e.g..debug,.gold) breaks:gamepad.memfiles not found)Fix
BuildConfig.APPLICATION_IDto construct paths at runtimeevshim.c): read newEVSHIM_DATA_DIRenv var, fall back toapp.gamenativeImportant: imagefs rebuild required
The
libevshim.sobundled in the imagefs must be rebuilt from the updatedevshim.c. Without this, the evshim won't readEVSHIM_DATA_DIRand gamepad input will remain broken for non-default app IDs.Files changed
Container.javaDATA_DIRconstant fromBuildConfig.APPLICATION_ID; used inMEDIACONV_ENV_VARSandDEFAULT_DRIVESDXVKHelper.javaDXVK_STATE_CACHE_PATHusesimageFs.getRootDir()WineUtils.javaBuildConfig.APPLICATION_IDWinHandler.javaBuildConfig.APPLICATION_IDBionicProgramLauncherComponent.javaEVSHIM_DATA_DIRenv varevshim.cEVSHIM_DATA_DIRenv var (fallback:app.gamenative)Testing
./gradlew compileDebugKotlinpasses/data/data/app.gamenativein modified files (except comment + C fallback)Summary by cubic
Replaced hardcoded /data/data/app.gamenative paths with BuildConfig.APPLICATION_ID and EVSHIM_DATA_DIR so build variants (e.g., .debug, .gold) work correctly. This fixes gamepad input, Wine E: drive mounts, DXVK state cache, and mediaconv paths for non-default app IDs.
Bug Fixes
Migration
Written for commit 8f1af16. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Fixes #618