ElementalWarrior wine-10.18-winmd#123
Conversation
mrshmllow
force-pushed
the
marshmallow/p-lyowtsqpyoqv
branch
from
b29c93c to
6d91eb1
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated Wine source and version; removed direct Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wine/default.nix`:
- Line 21: The version attribute in packages/wine/default.nix is missing the
build-ref suffix; update the version = "10.18" string to include the ref suffix
used elsewhere (e.g., change version to "10.18-winmd") so it matches the ref
`wine-10.18-winmd` and preserves the existing versioning pattern.
🪄 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: a1ef42a6-ea06-411e-b586-ee224389ae63
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
flake.nixpackages/wine/default.nix
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common.nix`:
- Around line 70-72: The hardcoded MSI name in the Wine prefix setup (inside the
if [[ "$type" == "v2" ]] block that calls ${lib.getExe wine} msiexec /i
"${wineUnwrapped}/share/wine/mono/wine-mono-9.3.0-x86.msi") must be replaced
with a glob or lookup to find the actual mono MSI shipped with the Wine version
(e.g., wine-mono-10*.msi); update the invocation to resolve the MSI path
dynamically (using shell globbing or a small find/ls fallback) from
"${wineUnwrapped}/share/wine/mono/" and pass that resolved filename to
${lib.getExe wine} msiexec /i so the code uses the real
wine-mono-<version>-x86.msi instead of the hardcoded 9.3.0 filename.
🪄 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: f6b21428-c9fd-4624-b357-989dd348dc41
📒 Files selected for processing (3)
packages/common.nixpackages/unified/scripts.nixpackages/wine/default.nix
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/common.nix (1)
68-72:⚠️ Potential issue | 🟠 MajorHardcoded Wine Mono MSI filename is brittle and likely wrong for Wine 10.18.
Line 71 still pins
wine-mono-9.3.0-x86.msi; this can break prefix initialization when the bundled Mono version differs. Resolve the MSI path dynamically from${wineUnwrapped}/share/wine/mono/.Suggested fix
- if [[ "$type" == "v2" ]]; then - ${lib.getExe wine} msiexec /i "${wineUnwrapped}/share/wine/mono/wine-mono-9.3.0-x86.msi" - fi + if [[ "$type" == "v2" ]]; then + mono_candidates=( "${wineUnwrapped}"/share/wine/mono/wine-mono-*-x86.msi ) + if [[ ! -f "${mono_candidates[0]}" ]]; then + echo "affinity-nix: Could not find wine-mono MSI in ${wineUnwrapped}/share/wine/mono" + exit 1 + fi + ${lib.getExe wine} msiexec /i "${mono_candidates[0]}" + fiFor nixpkgs `wineWow64Packages.full` at Wine 10.18, what exact filename is shipped under `share/wine/mono/` and should be passed to `wine msiexec /i`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common.nix` around lines 68 - 72, The code hardcodes "wine-mono-9.3.0-x86.msi" which can mismatch bundled mono; change the msiexec invocation to discover the MSI under ${wineUnwrapped}/share/wine/mono/ at runtime and pass that resolved filename to ${lib.getExe wine} msiexec /i instead of the literal string — locate the MSI by globbing or listing files in ${wineUnwrapped}/share/wine/mono/ (reference variables/functions: wineUnwrapped, ${lib.getExe wine}, msiexec) and use the discovered path in place of the hardcoded "wine-mono-9.3.0-x86.msi".
🧹 Nitpick comments (1)
packages/common.nix (1)
133-139: Preferrm -foverrm ... || truehere.This keeps logs cleaner and expresses intent more directly.
Suggested cleanup
- rm "${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.Services.winmd" || true - rm "${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.System.winmd" || true + rm -f "${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.Services.winmd" + rm -f "${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.System.winmd"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common.nix` around lines 133 - 139, The two deletion lines inside the conditional that currently use "rm ... || true" (the rm calls operating on "${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.Services.winmd" and ".../Windows.System.winmd") should be changed to use "rm -f" instead; update those two rm invocations in the block guarded by the prefixRevision/type check (the code referencing variables prefixRevision, type and affinityPath) so they use the -f flag to suppress errors rather than appending "|| true", keeping intent clear and logs cleaner.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/common.nix`:
- Around line 68-72: The code hardcodes "wine-mono-9.3.0-x86.msi" which can
mismatch bundled mono; change the msiexec invocation to discover the MSI under
${wineUnwrapped}/share/wine/mono/ at runtime and pass that resolved filename to
${lib.getExe wine} msiexec /i instead of the literal string — locate the MSI by
globbing or listing files in ${wineUnwrapped}/share/wine/mono/ (reference
variables/functions: wineUnwrapped, ${lib.getExe wine}, msiexec) and use the
discovered path in place of the hardcoded "wine-mono-9.3.0-x86.msi".
---
Nitpick comments:
In `@packages/common.nix`:
- Around line 133-139: The two deletion lines inside the conditional that
currently use "rm ... || true" (the rm calls operating on
"${affinityPath}/drive_c/windows/system32/WinMetadata/Windows.Services.winmd"
and ".../Windows.System.winmd") should be changed to use "rm -f" instead; update
those two rm invocations in the block guarded by the prefixRevision/type check
(the code referencing variables prefixRevision, type and affinityPath) so they
use the -f flag to suppress errors rather than appending "|| true", keeping
intent clear and logs cleaner.
34ca9b2 to
65f5a76
Compare
mrshmllow
force-pushed
the
marshmallow/p-lyowtsqpyoqv
branch
from
65f5a76 to
4bfaa10
Compare
1 participant
Todo: