Repository navigation
Engines: updating calibre no longer fails on its read-only files - #1124
Merged
Merged
Conversation
Every file of the calibre MSI is read-only. Replace deleted the previous copy in place, which stopped at the first one (amatch.dll), after the marker had gone, so the card fell back to Install and every retry failed the same way. The staging cleanup then threw the same message over the real error. Replace moves the previous version aside as Uninstall does, the cleanup can no longer replace the error from the try block, and DeleteFolder copes with read-only files and brief locks for every kind and the uninstall cleanup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 4, 2026
This was referenced Nov 4, 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
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.
Fixes #1084
Fixes #1092
Fixes #1109
Root cause
Updating calibre fails, every time. A fresh install works, which is why not everyone is affected. 2.3.0 was the first release to change the calibre pin (9.15.0 to 9.17.0), so it was the first time anyone had an old copy to replace.
msidbFileAttributesReadOnlyon all 1345 rows of 9.15.0 and all 1357 of 9.17.0), andmsiexec /akeeps that flag.Replacedeleted the old copy in place withDirectory.Delete(target, recursive: true), which stops at the first read-only file. In name order that isapp\bin\amatch.dll. By then the marker has already gone, so the card falls back to "Install" with no Update or Remove, as both screenshots show, and every retry hits the same file.finallythen tried to delete the staging folder, a tree from the same MSI, and threw the identical message over the real exception. That is why the stack pointed at the cleanup. Each attempt also left its staging folder behind (about 666 MB).How it was confirmed
The real
EngineInstaller.InstallAsyncwas run against the real pinned MSIs (SHA-256 checked) in temporary engines roots:Access to the path 'amatch.dll' is denied, same stack as #1092, marker gone,~xxxxxxleft behindfinallymade non-throwingReplacedeleting the old copy, and the cleanup failing as wellebook-convert --versionreports 9.17.0#1092's log has the same stack. #1084 and #1109 have no log but show the same message, and #1084's screenshot shows the same card state.
Changes
Replacemoves the previous version aside, asUninstallhas done since Engine install/remove: half-deleted engines, leftover .part downloads, busy message #963, instead of deleting it in place. While an engine runs the move fails and the working copy keeps its marker. Extensions (oxt) now useReplacetoo.TryDeleteFolderfor every kind, so a failure is logged and the error from the try block is the one reported.TryDeletein the otherfinallynow also catchesUnauthorizedAccessException.DeleteFolderclears the flag (not through links) and retries for about 2 seconds.TryDeleteFolderandUninstallCleanupuse it. Before this, removing calibre left a~removed-calibre-*folder that could never be cleared, and the uninstall cleanup failed on it.LibreOffice is not affected today: its pinned 26.2.6 MSI has no read-only files (0 of 19496 File rows). It gets the same protection anyway.
Tests
Seven new tests in
EngineInstallerTests, building their data in code. All seven fail with the old delete behaviour restored, the main one with the reporters' exactamatch.dllmessage.InStagingAsyncis split out so a test can put a read-only file in staging and fail inside it.Focused runs pass: engine installer, portable, real path and own copy tests (55),
EnginesCommandTests(13),EnginesPageTests(6).dotnet format --verify-no-changesis clean on the changed files.Not in this PR
Staging folders left by failed 2.3.0 attempts are not removed. Their
~xxxxxxnames don't say which package or process they belong to, so clearing them safely needs an age rule. The reporter comments cover deleting them by hand.🤖 Generated with Claude Code