Skip to content

fix(updater): remove leftover installer during config version migration - #10680

Open
chrip wants to merge 1 commit into
nextcloud:masterfrom
chrip:fix/7009-installer-cleanup-on-migration
Open

fix(updater): remove leftover installer during config version migration#10680
chrip wants to merge 1 commit into
nextcloud:masterfrom
chrip:fix/7009-installer-cleanup-on-migration

Conversation

@chrip

@chrip chrip commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Resolves

#7009

Summary

Every update leaves its installer (~200 MB) and msi.log behind in %APPDATA%\Nextcloud, one file per release.

#8980 did not take effect in practice. On the first start of a newly installed version, Application::configVersionMigration() calls ConfigFile::cleanUpdaterConfiguration() (application.cpp:161-164), which drops Updater/updateAvailable — the only record of where the installer is — before main() reaches NSISUpdater::handleStartup() (main.cpp:104 vs :142). Both wipeUpdateData() call sites then sit behind a guard that can no longer be true, and the file is orphaned with nothing left to find it by.

The trigger is the version number in nextcloud.cfg versus the one compiled into the running binary. They differ on every real update, and are identical when the same version is reinstalled — which is why this passes local testing, and why CI cannot express the case at all (one binary, one MIRALL_VERSION_STRING). It regressed in v3.16.0 with b3886ed (#7807); before that the main flow cleaned up correctly and only unusual paths leaked.

ConfigFile::cleanUpdaterConfiguration() now deletes the installer and msi.log before dropping the keys, and NSISUpdater::wipeUpdateData() delegates to it instead of duplicating the same four key removals. Startup ordering is untouched, so #7807's intent is preserved.

Deliberately not included: installers that have already accumulated (no recorded path remains, so they need a directory sweep) and the download location itself.

Tests

testLeftoverInstallerIsRemovedAfterVersionChange fails on master and passes with the fix.

before after
Windows, version change, real nextcloud.exe Version changed… logged, no removal, both files survive two Removed leftover updater file: lines, both files gone
Windows, pending update, no version change untouched untouched (must not regress)
Windows ctest 78/78 78/78
Linux ctest 79/79 79/79

All seven paths the updater can take were driven against the real binary — the five dialog outcomes ("New update ready" → Cancel / OK; "Update Failed" → Ask again later / Restart and update / Update manually) plus the two non-interactive ones (version change, external install).

Deletion happens only in the two cases where the update is genuinely finished, and in none of the five where a decision is still pending — a pending installer is never removed.

Built and verified on Windows (Craft/MSVC 2022) and Linux (Qt 6.10.2). Not tested: the locked-file case (QFile::remove failing under an antivirus or open handle), which logs a warning and leaves the file.

Checklist

AI (if applicable)

Application::configVersionMigration() clears the Updater/* keys on the
first start of a newly installed version, before NSISUpdater::handleStartup()
gets a chance to see them and clean up the downloaded installer. The
installer and msi.log are then orphaned in the config folder with nothing
left to find them by, repeating on every update.

ConfigFile::cleanUpdaterConfiguration() now deletes the installer and
msi.log before dropping the keys, so both the migration path and the
updater's own cleanup remove the file. NSISUpdater::wipeUpdateData()
delegates to it instead of duplicating the removal logic.

Reported by @tgebler as still reproducing after nextcloud#8980:
nextcloud#8980 (comment)

Closes nextcloud#7009.

Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: chrip <christoph.schaefer@nextcloud.com>
@mgallien
mgallien force-pushed the fix/7009-installer-cleanup-on-migration branch from 4253651 to 3dab671 Compare August 28, 2026 11:21
@mgallien mgallien self-assigned this Aug 28, 2026
@mgallien mgallien added this to the 35.0.0 milestone Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Artifact containing the AppImage: nextcloud-appimage-pr-10680.zip

Digest: sha256:e837b2f4e697440d43c321c376da6cb8bd2c850e6779753ed056030cf6ded8fa

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks
one small change only

};

QString msiLogFile = cfg.configPath() + msiLogFileNameC;
QString msiLogFile = cfg.msiLogFilePath();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
QString msiLogFile = cfg.msiLogFilePath();
const auto msiLogFile = cfg.msiLogFilePath();

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants