Skip to content

Fix MakeMKV detection on Windows (makemkvcon64.exe with makemkvcon.exe fallback) - #31

Open
shaunmower wants to merge 4 commits into
Poisonite:masterfrom
shaunmower:fix/windows-makemkvcon64
Open

shaunmower wants to merge 4 commits into
Poisonite:masterfrom
shaunmower:fix/windows-makemkvcon64

Conversation

@shaunmower

Copy link
Copy Markdown

Description

MakeMKV was not detected on Windows. AppConfig.getMakeMKVExecutable() runs makemkvcon64.exe, but detectMakeMKVInstallation() and validateMakeMKVInstallation() looked for makemkvcon.exe. Recent MakeMKV installs (v2.0.0 in my case) only include makemkvcon64.exe, so detection failed even though MakeMKV was installed.

All three now use one new helper, FileSystemUtils.findMakeMKVExecutable(). On Windows it tries makemkvcon64.exe first and then makemkvcon.exe, so installs with only the 32-bit executable keep working. Detection, validation and the command that runs always agree on which executable to use. macOS and Linux still use makemkvcon.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Testing

  • filesystem.test.js: checks that detection and validation look for makemkvcon64.exe on Windows, and that makemkvcon.exe is used when the 64-bit executable is missing.
  • config.test.js: now stubs findMakeMKVExecutable alongside the existing detection stubs, so it doesn't depend on the real filesystem.

npx vitest run: all tests pass apart from native-optical-drive.test.js > should validate drive letter parameter, which also fails on master without this change.

General Testing

  • Tested main ripping functionality (npm start)

Test Configuration:

  • OS: Windows 11
  • Node.js version: v24.19.0
  • MakeMKV version: v2.0.0
  • Administrator privileges: No

Documentation

  • Added/updated JSDoc comments for new code
  • Updated CHANGELOG.md

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • My changes generate no new warnings or errors

@shaunmower shaunmower mentioned this pull request Oct 5, 2026
14 tasks done

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant