Skip to content

Look for redumper on PATH#4

Open
gmipf wants to merge 1 commit into
Deterous:mainfrom
gmipf:pr/2-redumper-on-path
Open

Look for redumper on PATH#4
gmipf wants to merge 1 commit into
Deterous:mainfrom
gmipf:pr/2-redumper-on-path

Conversation

@gmipf

@gmipf gmipf commented Jul 14, 2026

Copy link
Copy Markdown

find_redumper() only looks next to the executable, so the program refuses to start with "'redumper' executable not found" even when redumper is installed and on PATH. That is exactly the situation a distribution package is in: the two programs are separate packages, and the GUI is not installed next to redumper.

MPF.Check is already located this way — postprocess.rs looks in the executable's directory first and falls back to PATH. This gives redumper the same treatment.

dump.rs resolved the path a second time and independently of the startup check, so a redumper found on PATH would have been lost again the moment a dump started: the program would have come up and then failed to spawn. It now calls find_redumper() as well, which also removes two unwrap() calls on current_exe() that would panic rather than report.

Measured on Fedora 44 against main (b3939d7), on real hardware — a Plextor PX-760A over USB with a CD in it — with redumper on PATH and no redumper next to the executable:

Build Result
main modal error, program exits — although /usr/bin/redumper is on PATH
this branch starts; DUMP spawns the redumper from PATH and dumps the disc

On this branch the spawned process was /usr/bin/redumper disc --auto-detect --drive=/dev/sg1 --image-name=… --retries=1 --speed=16, running from the output directory, with its output streaming into the log window as usual. It read the disc through to the end — 226575 of 226575 sectors, no SCSI errors and no C2 errors — so both call sites resolve, not just the startup check. (I stopped the run during redumper's trailing Plextor lead-in phase; that was my harness timing out, nothing to do with this change.)


Prepared with AI assistance (Claude Opus 4.8) and reviewed before submission.

find_redumper() only looked next to the executable, so the program refused to
start with "'redumper' executable not found" even when redumper was installed
and on PATH. That is the situation a distribution package is in: the two
programs are separate packages, and the GUI is not installed next to redumper.

Look on PATH when the executable's own directory does not hold it, the same
way MPF.Check is already located in postprocess.rs.

dump.rs resolved the path a second time and independently, so a redumper found
on PATH at startup would have been lost again when the dump started. It now
uses find_redumper() as well, which also removes two unwrap() calls on
current_exe().
@Deterous

Deterous commented Jul 15, 2026

Copy link
Copy Markdown
Owner

My concern about this is that there is no way to verify that the version of redumper on PATH is a compatible version. Both older and newer versions are not guaranteed to have the same set of flags, or output to the log in the same way that the GUI is expecting.

That is why I bundle redumper with my releases. Even better for mac: I put redumper inside the .app file.
For packaging for distributions, you could potentially have a symlink to the same redumper version as mentioned in my github YML and put it next to the GUI executable? This would also potentially solve a case where the user wants both latest redumper installed on their machine, as well as the version that the GUI is expecting.

The reason why I look at PATH for MPF.Check is that I have no requirement for the version of MPF, later versions are likely to work just fine.

@gmipf

gmipf commented Jul 15, 2026

Copy link
Copy Markdown
Author

Thanks — that's a fair concern, and you're right that the GUI is coupled to redumper: it parses redumper's log output (disc-profile refinement, sector counts), so an unexpected version could misbehave. Pinning is justified. I think the change is narrower than it may read, though, and doesn't weaken that.

Precedence is unchanged: the executable's own directory is checked first, and PATH is only a fallback. Measured on this branch with a stub redumper in two places:

Setup Resolved to
redumper only on PATH, none beside the binary the PATH one
redumper beside the binary (also on PATH) the one beside the binary — PATH never consulted

So the portable release is unaffected: it bundles redumper next to the executable (and inside the .app on macOS), that copy always wins, and PATH is never read. The fallback can only fire when nothing is placed next to the executable — the case where main currently refuses to start.

Which redumper runs is decided by whoever assembles the install, not by this lookup: your release bundles the pinned build; a distribution pins it in packaging. For a distribution that pins, the mechanism is exactly what you suggested — place that build next to the GUI. The Fedora/openSUSE packaging does this with a symlink in the GUI's private directory (…/redumper-gui/redumper → the pinned redumper<build>), found by the executable-directory lookup. It works on unmodified main, and this PR leaves it winning over PATH. A bare PATH fallback would actually resolve to the rolling redumper (/usr/bin/redumper), not the pinned build, so PATH is not the pinning mechanism and I'm not proposing it as one.

What the PR changes is limited to:

  • a from-source build with no redumper placed next to it but one on PATH now starts and runs, instead of failing at launch;
  • dump.rs resolved the path a second time (current_exe().unwrap().parent().unwrap()), which panics instead of reporting an error; it now calls the same find_redumper(), so both call sites agree.

If you'd rather users always place redumper next to the GUI, I'm happy to narrow this to just the dump.rs fix or close it — exe-dir-first means it never overrides a bundled or pinned redumper either way.

Generated with Claude Opus 4.8.

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.

2 participants