Refactored default Proton downloads to launch deps#1031
Merged
utkarshdalal merged 1 commit intoutkarshdalal:masterfrom Mar 27, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughRemoved Proton-specific download/extract from preLaunchApp and introduced a new BionicDefaultProtonDependency that detects, downloads (via SteamService), and extracts Proton archives into ImageFs when running in BIONIC containers with proton-9.0-* wine versions. Changes
Sequence Diagram(s)sequenceDiagram
participant Launcher as LaunchDependencies
participant Dep as BionicDefaultProtonDependency
participant Steam as SteamService
participant FS as ImageFs
participant Tar as TarCompressorUtils
Launcher->>Dep: ensureLaunchDependencies(container, gameSource, gameId)
Dep->>FS: stat `opt/<wineVersion>/bin` (isSatisfied)
alt not present
Dep->>Steam: isFileInstallable(proton-9.0-*.txz)?
alt installable or available remotely
Dep->>Steam: downloadFile(...).await() (progress callbacks)
Steam-->>Dep: saved archive path
end
Dep->>Tar: extract archive -> FS root `opt/<wineVersion>` (set unknown progress)
Tar-->>FS: extracted files
Dep->>FS: delete downloaded archive
end
Dep-->>Launcher: dependency satisfied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved 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/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt:30">
P2: `File(rootDir, "/opt/$protonVersion")` uses an absolute child path, so the ImageFs root is ignored and this resolves to `/opt/...` on the device root rather than under imagefs. That will make `isSatisfied`/extract checks look in the wrong place. Use a relative child path (no leading slash).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Around line 75-88: The extraction result from
TarCompressorUtils.extract(downloaded, outFile) is ignored causing the archive
to be deleted even on failure and leaving partial files; update the block around
outFile/binDir to capture the boolean return of TarCompressorUtils.extract (call
site: TarCompressorUtils.extract(...) and variable downloaded), only call
downloaded.delete() when extraction succeeded, and on failure remove any
partially created outFile (or its binDir) and surface the error by updating
callbacks (e.g., callbacks.setLoadingMessage / setLoadingProgress) and
returning/failing so isSatisfied() cannot incorrectly pass; ensure you reference
outFile, binDir, TarCompressorUtils.extract, downloaded and isSatisfied when
making these checks and cleanup steps.
- Around line 53-73: The unprotected .await() on SteamService.downloadFile can
throw IOException (from SteamService.fetchFileWithFallback) and crash the
install; wrap each coroutineScope { SteamService.downloadFile(...).await() }
invocation in a try-catch that catches IOException (and optionally
CancellationException), call the UI callbacks (e.g., callbacks.setLoadingMessage
and/or a failure callback) with a user-friendly error including e.message,
stop/abort the install flow cleanly (return/throw a controlled error) instead of
letting the raw exception propagate; update the blocks around protonVersion
checks and the downloadFile await calls to implement this handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d370ef3-e821-49cf-aae4-0f2f3a804fbf
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/utils/LaunchDependencies.ktapp/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/app/gamenative/ui/PluviaMain.kt
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
Outdated
Show resolved
Hide resolved
unbelievableflavour
force-pushed
the
default-proton-downloads-refactor
branch
2 times, most recently
from
d2f09d5 to
848b0be
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt (1)
83-85:⚠️ Potential issue | 🟡 MinorPartial extraction not cleaned up on failure.
When extraction fails, the code throws an exception but leaves potentially corrupt partial files in
outFile. This could causeisSatisfied()to returntrueon retry if thebin/directory was partially created, or leave orphaned files consuming storage.🛡️ Proposed fix to clean up partial extraction
val success = TarCompressorUtils.extract( TarCompressorUtils.Type.XZ, downloaded, outFile, ) if (!success) { + outFile.deleteRecursively() + Timber.e("Failed to extract $protonVersion") throw IllegalStateException("Failed to extract $protonVersion from ${downloaded.absolutePath}") } downloaded.delete()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt` around lines 83 - 85, The extraction failure path currently throws an IllegalStateException but leaves partial files behind (outFile / partially-created bin/), so update the failure branch in the extraction routine to delete the destination (outFile) and any partially created extraction directory (e.g., the target that contains bin/) before throwing; specifically, in the block surrounding the throw for "Failed to extract $protonVersion from ${downloaded.absolutePath}" ensure you close any streams, attempt a recursive delete of outFile/target directory, log the cleanup attempt, and only then rethrow the IllegalStateException so subsequent isSatisfied() checks won't be fooled by leftover partial files.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt (1)
69-69: RedundantImageFs.find(context)call.
imageFsis already retrieved on line 50 but is not reused here. Consider using the existing reference to avoid redundant lookups.♻️ Proposed fix
- val outFile = File(ImageFs.find(context).getRootDir(), "opt/$protonVersion") + val outFile = File(imageFs.getRootDir(), "opt/$protonVersion")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt` at line 69, The code redundantly calls ImageFs.find(context) when an imageFs variable was already obtained earlier; update the outFile construction (val outFile in BionicDefaultProtonDependency.kt) to reuse the existing imageFs reference (imageFs.getRootDir()) instead of calling ImageFs.find(context) again, keeping the same "opt/$protonVersion" path and preserving protonVersion and context usage elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Line 58: The hardcoded loading message in BionicDefaultProtonDependency always
says "Downloading arm64ec Proton" regardless of which archive is being
downloaded; update the callbacks.setLoadingMessage call to compute the message
from the actual archiveName (or derive arch from archiveName) so it displays the
correct variant (e.g., "Downloading <arch> Proton" or include archiveName
directly). Locate the callbacks.setLoadingMessage invocation in
BionicDefaultProtonDependency.kt and replace the literal string with a formatted
message built from the archiveName variable (or a small function that maps
archiveName to a user-friendly architecture label) so the UI reflects the real
download target.
---
Duplicate comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Around line 83-85: The extraction failure path currently throws an
IllegalStateException but leaves partial files behind (outFile /
partially-created bin/), so update the failure branch in the extraction routine
to delete the destination (outFile) and any partially created extraction
directory (e.g., the target that contains bin/) before throwing; specifically,
in the block surrounding the throw for "Failed to extract $protonVersion from
${downloaded.absolutePath}" ensure you close any streams, attempt a recursive
delete of outFile/target directory, log the cleanup attempt, and only then
rethrow the IllegalStateException so subsequent isSatisfied() checks won't be
fooled by leftover partial files.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Line 69: The code redundantly calls ImageFs.find(context) when an imageFs
variable was already obtained earlier; update the outFile construction (val
outFile in BionicDefaultProtonDependency.kt) to reuse the existing imageFs
reference (imageFs.getRootDir()) instead of calling ImageFs.find(context) again,
keeping the same "opt/$protonVersion" path and preserving protonVersion and
context usage elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af893e95-f292-4953-b572-ac30efef6bf5
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/utils/LaunchDependencies.ktapp/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/app/gamenative/ui/PluviaMain.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/utils/LaunchDependencies.kt
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
Outdated
Show resolved
Hide resolved
unbelievableflavour
force-pushed
the
default-proton-downloads-refactor
branch
from
848b0be to
c9f51eb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt (1)
77-86:⚠️ Potential issue | 🟠 MajorHandle extraction failure with cleanup to avoid false “satisfied” state.
If extraction fails, Line 84 throws, but partial files in
outFileremain. SinceisSatisfied()(Line 27-31) only checks forbin/, a partial extract can make future runs skip reinstall incorrectly.🧹 Proposed failure cleanup
withContext(Dispatchers.IO) { val success = TarCompressorUtils.extract( TarCompressorUtils.Type.XZ, downloaded, outFile, ) if (!success) { + outFile.deleteRecursively() + downloaded.delete() throw IllegalStateException("Failed to extract $protonVersion from ${downloaded.absolutePath}") } downloaded.delete() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt` around lines 77 - 86, The extraction failure currently throws an IllegalStateException but leaves partial files in outFile which can make BionicDefaultProtonDependency.isSatisfied() return true later; modify the extraction block inside BionicDefaultProtonDependency so that if TarCompressorUtils.extract(...) returns false (or an exception is thrown) you perform cleanup of partial extraction artifacts (e.g., delete outFile or its extracted children) and still rethrow the error (or throw the IllegalStateException) afterward; ensure this covers both the boolean-false path and any exceptions from TarCompressorUtils.extract so partial contents are removed before signaling failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Around line 17-18: The KDoc at the top of BionicDefaultProtonDependency (the
comment describing extraction destination) is out of sync with the
implementation: the code writes Proton to opt/<wineVersion> (see usage of
wineVersion and the write/extract logic), but the KDoc still says
imagefs_shared/proton; update the KDoc to state that Proton is downloaded and
extracted to opt/<wineVersion> (or to whichever exact opt path the
implementation uses) and adjust the wording to match the conditional behavior
for BIONIC and proton-9.0-arm64ec/proton-9.0-x86_64.
---
Duplicate comments:
In
`@app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt`:
- Around line 77-86: The extraction failure currently throws an
IllegalStateException but leaves partial files in outFile which can make
BionicDefaultProtonDependency.isSatisfied() return true later; modify the
extraction block inside BionicDefaultProtonDependency so that if
TarCompressorUtils.extract(...) returns false (or an exception is thrown) you
perform cleanup of partial extraction artifacts (e.g., delete outFile or its
extracted children) and still rethrow the error (or throw the
IllegalStateException) afterward; ensure this covers both the boolean-false path
and any exceptions from TarCompressorUtils.extract so partial contents are
removed before signaling failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d09bb27a-b526-474b-8281-c70668699e2c
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/utils/LaunchDependencies.ktapp/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/app/gamenative/ui/PluviaMain.kt
app/src/main/java/app/gamenative/utils/launchdependencies/BionicDefaultProtonDependency.kt
Show resolved
Hide resolved
This was referenced Mar 30, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by cubic
Moved default Proton 9.0 (arm64ec/x86_64) download and extraction into a launch dependency for Bionic containers. This simplifies pre-launch and standardizes behavior; GLIBC containers are unchanged.
BionicDefaultProtonDependencyforContainer.BIONICwhenwineVersionisproton-9.0-arm64ecorproton-9.0-x86_64.LaunchDependenciesand removed inline handling fromPluviaMain.opt/<protonVersion>/bin, downloads viaSteamService.downloadFilewith progress, extracts the.txzto/opt/<protonVersion>, then deletes the archive.Written for commit c9f51eb. Summary will update on new commits.
Summary by CodeRabbit
Refactor
New Features