From d7c2f6e355a41c5faef8ccd8292683ddf9951863 Mon Sep 17 00:00:00 2001 From: Khushboo Mehta Date: Wed, 19 Aug 2026 12:20:14 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20Reinstall=20doesn't=20remove=20files=20d?= =?UTF-8?q?eleted=20by=20the=20new=20version=20=E2=80=94=20install=20merge?= =?UTF-8?q?s=20into=20the=20existing=20directory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes #313 --- .../AppManager/AppManagerPanelHeader.qml | 6 +- src/PackageCoordinator.cpp | 100 ++++++++++++++---- src/PackageCoordinator.h | 10 ++ 3 files changed, 90 insertions(+), 26 deletions(-) diff --git a/src/Basecamp/AppManager/AppManagerPanelHeader.qml b/src/Basecamp/AppManager/AppManagerPanelHeader.qml index c3739bf..7c99e56 100644 --- a/src/Basecamp/AppManager/AppManagerPanelHeader.qml +++ b/src/Basecamp/AppManager/AppManagerPanelHeader.qml @@ -78,11 +78,11 @@ Item { LogosButton { Layout.fillWidth: true Layout.minimumWidth: 100 - Layout.preferredWidth: 130 - Layout.maximumWidth: 130 + Layout.preferredWidth: implicitWidth + Layout.maximumWidth: implicitWidth Layout.preferredHeight: 40 radius: Theme.spacing.radiusLarge - text: qsTr("Repositories") + text: qsTr("Manage Repositories") onClicked: root.repositoriesClicked() } diff --git a/src/PackageCoordinator.cpp b/src/PackageCoordinator.cpp index 63b5cd7..1bc3b1f 100644 --- a/src/PackageCoordinator.cpp +++ b/src/PackageCoordinator.cpp @@ -604,30 +604,8 @@ QVariantMap PackageCoordinator::buildPlanPayload(const QStringList& batch, // Cascade confirmation — triggered from QML once the user OKs the dialog. // --------------------------------------------------------------------------- -void PackageCoordinator::confirmUninstallCascade(const QString& moduleName) +void PackageCoordinator::cascadeUnloadForPackage(const QString& moduleName) { - if ((m_pendingAction.op != PendingOp::UninstallCascade && - m_pendingAction.op != PendingOp::UpgradeCascade) - || m_pendingAction.name != moduleName) { - qWarning() << "confirmUninstallCascade for" << moduleName - << "but pending action is" << m_pendingAction.name; - return; - } - - // Snapshot before clearing — the callbacks below capture by value. - const bool isUpgrade = (m_pendingAction.op == PendingOp::UpgradeCascade); - const QString releaseTag = m_pendingAction.releaseTag; - m_pendingAction = {}; - - // Defer the cascade body off the QML click stack: unloadModuleWithDependents - // spins a nested event loop, and running that under the dialog's onClicked - // handler trips a QQmlData::destroyed qFatal. Pending state is cleared above; - // queue the rest so the click handler unwinds first. - QPointer selfDefer(this); - QMetaObject::invokeMethod(this, - [this, selfDefer, moduleName, isUpgrade, releaseTag]() { - if (!selfDefer) return; - // Snapshot the loaded-dependents list BEFORE the cascade — once // unloadModuleWithDependents returns, the target is off the loaded- // modules list and the filter would come up empty. UI-plugin dependents @@ -670,6 +648,29 @@ void PackageCoordinator::confirmUninstallCascade(const QString& moduleName) } m_uiPluginManager->teardownUiPluginWidget(moduleName); } +} + +void PackageCoordinator::confirmUninstallCascade(const QString& moduleName) +{ + if ((m_pendingAction.op != PendingOp::UninstallCascade && + m_pendingAction.op != PendingOp::UpgradeCascade) + || m_pendingAction.name != moduleName) { + qWarning() << "confirmUninstallCascade for" << moduleName + << "but pending action is" << m_pendingAction.name; + return; + } + + // Snapshot before clearing — the callbacks below capture by value. + const bool isUpgrade = (m_pendingAction.op == PendingOp::UpgradeCascade); + const QString releaseTag = m_pendingAction.releaseTag; + m_pendingAction = {}; + + QPointer selfDefer(this); + QMetaObject::invokeMethod(this, + [this, selfDefer, moduleName, isUpgrade, releaseTag]() { + if (!selfDefer) return; + + cascadeUnloadForPackage(moduleName); // Hand the actual package-lifecycle work back to the module. if (!m_logosAPI) return; @@ -1928,6 +1929,59 @@ void PackageCoordinator::installOnePackage(const QVariantMap& dl, return; } + if (!m_logosAPI) { + if (onDone) onDone(false, QStringLiteral("package_manager not connected")); + return; + } + + const bool alreadyInstalled = m_installedNameSet.contains(packageName); + const bool isEmbedded = + m_installTypeByModule.value(packageName) == QLatin1String("embedded"); + + // Never tear down or remove our own UI — same guard uninstallUiModule + // carries, for the same reason: it would brick Basecamp mid-install. + // AppsFilterProxy::excludeMainUi only hides it from the list; it is not a + // safety gate, and a resolver result can name it as a transitive entry. + // Falling through installs over it, which is the old merge behaviour — + // strictly better than deleting the running UI. + const bool isSelf = (packageName == QStringLiteral("main_ui")); + if (isSelf && alreadyInstalled) { + qWarning() << "Refusing to remove main_ui before install; " + "installing over it instead"; + } + + if (alreadyInstalled && !isEmbedded && !isSelf) { + cascadeUnloadForPackage(packageName); + + LogosModules logos(m_logosAPI); + QPointer self(this); + logos.package_manager.uninstallPackageAsync(packageName, + [self, dl, packageName, onDone](QVariantMap uninstallResult) { + if (!self) return; + if (!uninstallResult.value("success", false).toBool()) { + const QString err = uninstallResult.value("error").toString(); + qWarning() << "Pre-install removal of" << packageName + << "failed, aborting install:" << err; + if (onDone) + onDone(false, err.isEmpty() + ? QStringLiteral("Could not remove the installed version") + : err); + return; + } + self->installDownloadedFile(dl, onDone); + }); + return; + } + + installDownloadedFile(dl, onDone); +} + +void PackageCoordinator::installDownloadedFile(const QVariantMap& dl, + std::function onDone) +{ + const QString packageName = dl.value("name").toString(); + const QString filePath = dl.value("path").toString(); + LogosModules logos(m_logosAPI); QPointer self(this); // Installing was left on the default 20 s IPC deadline while DOWNLOADING diff --git a/src/PackageCoordinator.h b/src/PackageCoordinator.h index b6fa223..1937f7c 100644 --- a/src/PackageCoordinator.h +++ b/src/PackageCoordinator.h @@ -370,8 +370,18 @@ private: const QString& topLevelName, int index, QStringList failures = QStringList{}); + // Unload/tear down a package and its loaded dependents ahead of any + // destructive package-lifecycle work. Shared by confirmUninstallCascade + // (the gated uninstall/upgrade flow) and by the App Manager's + // replace-before-install step, so both run identical teardown. + void cascadeUnloadForPackage(const QString& moduleName); + + // installOnePackage tears down + removes an already-installed package + // before handing off to installDownloadedFile, the bare installPlugin call. void installOnePackage(const QVariantMap& downloadResult, std::function onDone); + void installDownloadedFile(const QVariantMap& downloadResult, + std::function onDone); // Drive the in-flight registry. setOpStage updates the InstallRegistry entry // and emits catalogInstallStageChanged.