From 0f7bcd38a1757cd96074ac254015ec02dff9fb43 Mon Sep 17 00:00:00 2001 From: Sergey Pak Date: Fri, 26 Jul 2024 16:57:39 +0200 Subject: [PATCH] IJPL-157266 Exclude automatically disabled plugins from Settings Sync GitOrigin-RevId: b66e2c30957490fa218178ec1951bdc62e021dc1 --- .../SettingsSyncPluginInstallerImpl.kt | 72 ++++++++++++------- .../plugins/SettingsSyncPluginManager.kt | 3 +- .../SettingsSyncPluginManagerTest.kt | 33 +++++++-- .../SettingsSyncPluginManagerTestUtil.kt | 6 +- 4 files changed, 81 insertions(+), 33 deletions(-) diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginInstallerImpl.kt b/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginInstallerImpl.kt index 40147f50fd1f..287978c54b3a 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginInstallerImpl.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginInstallerImpl.kt @@ -5,7 +5,7 @@ import com.intellij.openapi.application.EDT import com.intellij.openapi.components.SettingsCategory import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.extensions.PluginId -import com.intellij.openapi.progress.ProgressManager +import com.intellij.openapi.project.Project import com.intellij.openapi.project.ProjectManager import com.intellij.openapi.project.currentOrDefaultProject import com.intellij.openapi.updateSettings.impl.PluginDownloader @@ -25,36 +25,51 @@ internal open class SettingsSyncPluginInstallerImpl(private val notifyErrors: Bo override suspend fun installPlugins(pluginsToInstall: List) { if (pluginsToInstall.isEmpty()) return + + val project: Project? = ProjectManager.getInstanceIfCreated()?.openProjects?.firstOrNull() + val downloaders = withBackgroundProgress(currentOrDefaultProject(project), SettingsSyncBundle.message("installing.plugins.indicator")) { + createDownloaders(pluginsToInstall) + } + val remainingPluginIds = mutableSetOf(*pluginsToInstall.toTypedArray()) + downloaders.forEach { + remainingPluginIds.remove(it.id) + } + var settingsChanged = false + remainingPluginIds.forEach { + LOG.info("Cannot find compatible updates for $it. Will not try to install it again.") + disablePluginSync(it) + settingsChanged = true + } + installCollected(downloaders, settingsChanged) + } + + internal open suspend fun installCollected(installers: List, settingsAlreadyChanged: Boolean) { withModalProgress(ModalTaskOwner.guess(), SettingsSyncBundle.message("installing.plugins.indicator"), TaskCancellation.nonCancellable()) { - val downloaders = createDownloaders(pluginsToInstall) - installCollected(downloaders) + installCollected(installers, settingsAlreadyChanged) } } - private suspend fun installCollected(installers: List) { + internal suspend fun doInstallCollected(installers: List, settingsAlreadyChanged: Boolean) { val pluginsRequiredRestart = mutableListOf() - var settingsChanged = false - val settings = SettingsSyncSettings.getInstance() + var settingsChanged = settingsAlreadyChanged for (installer in installers) { - withContext(Dispatchers.EDT) { - try { - if (!install(installer)) { - pluginsRequiredRestart.add(installer.pluginName) - } - LOG.info("Setting sync installed plugin ID: ${installer.id.idString}") + try { + if (!install(installer)) { + pluginsRequiredRestart.add(installer.pluginName) } - catch (ex: Exception) { + LOG.info("Setting sync installed plugin ID: ${installer.id.idString}") + } + catch (ex: Exception) { - // currently, we don't install plugins that have missing dependencies. - // TODO: toposort plugin with dependencies. - // TODO: Skip installation dependent plugins, if any dependency fails to install. - LOG.warn("An exception occurred while installing plugin ${installer.id.idString}. Will disable syncing this plugin", ex) - settings.setSubcategoryEnabled(SettingsCategory.PLUGINS, installer.id.idString, false) - settingsChanged = true - } + // currently, we don't install plugins that have missing dependencies. + // TODO: toposort plugin with dependencies. + // TODO: Skip installation dependent plugins, if any dependency fails to install. + LOG.warn("An exception occurred while installing plugin ${installer.id.idString}. Will disable syncing this plugin", ex) + disablePluginSync(installer.id) + settingsChanged = true } } - if (settingsChanged){ + if (settingsChanged) { SettingsSyncEvents.getInstance().fireCategoriesChanged() } if (pluginsRequiredRestart.size > 0) { @@ -62,23 +77,26 @@ internal open class SettingsSyncPluginInstallerImpl(private val notifyErrors: Bo } } - open internal fun install(installer: PluginDownloader): Boolean = installer.installDynamically(null) + private fun disablePluginSync(pluginId: PluginId) { + SettingsSyncSettings.getInstance().setSubcategoryEnabled(SettingsCategory.PLUGINS, pluginId.idString, false) + } + + internal open suspend fun install(installer: PluginDownloader): Boolean { + return withContext(Dispatchers.EDT) { + installer.installDynamically(null) + } + } open internal fun createDownloaders(pluginIds: Collection): List { val compatibleUpdates = MarketplaceRequests.getLastCompatiblePluginUpdate(pluginIds.toSet()) val retval = arrayListOf() - val remainingPluginIds = mutableSetOf(*pluginIds.toTypedArray()) for (update in compatibleUpdates) { val pluginDescriptor = MarketplaceRequests.loadPluginDescriptor(update.pluginId, update) val downloader = PluginDownloader.createDownloader(pluginDescriptor) if (downloader.prepareToInstall(null)) { retval.add(downloader) - remainingPluginIds.remove(PluginId.getId(update.externalPluginId)) } } - if (remainingPluginIds.isNotEmpty()) { - LOG.info("Cannot find compatible updates for ${remainingPluginIds.joinToString()}") - } return retval } } \ No newline at end of file diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginManager.kt b/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginManager.kt index ad115c36fd62..b55f0ddd0c93 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginManager.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/plugins/SettingsSyncPluginManager.kt @@ -52,7 +52,7 @@ internal class SettingsSyncPluginManager(private val cs: CoroutineScope) : Dispo LOG.info("Plugins ${removedPluginIds.joinToString()} have been deleted from disk") for (pluginId in removedPluginIds) { val pluginData = newPlugins[pluginId] ?: continue - if (checkDependencies(pluginId, pluginData)) { + if (checkDependencies(pluginId, pluginData) && isPluginSynceable(pluginId)) { newPlugins.computeIfPresent(pluginId) { _, data -> PluginData(enabled = false, data.category, data.dependencies) } removed2disable.add(pluginId) } else { @@ -79,6 +79,7 @@ internal class SettingsSyncPluginManager(private val cs: CoroutineScope) : Dispo // also don't touch localization plugins as they become bundled in 242 and might cause issues: // see https://youtrack.jetbrains.com/issue/IJPL-157227/IDE-is-localized-after-Settings-Sync-between-2024.1-and-2024.2-if-language-plugins-had-updates + LOG.info("Plugin $id is not syncable!") } else if (shouldSaveState(plugin)) { newPlugins[id] = getPluginData(plugin) diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTest.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTest.kt index 94cb137e519d..46781bebca8c 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTest.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTest.kt @@ -1,7 +1,5 @@ package com.intellij.settingsSync -import com.intellij.ide.plugins.IdeaPluginDescriptor -import com.intellij.ide.plugins.PluginEnableStateChangedListener import com.intellij.idea.TestFor import com.intellij.openapi.components.SettingsCategory import com.intellij.settingsSync.config.BUNDLED_PLUGINS_ID @@ -11,7 +9,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest -import org.junit.Assert import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.api.Test import java.util.concurrent.atomic.AtomicReference @@ -364,7 +361,6 @@ class SettingsSyncPluginManagerTest : BasePluginManagerTest() { typengo(enabled = true) } assertPluginManagerState(pushedState) - Thread.sleep(100) assertFalse(SettingsSyncSettings.getInstance().isSubcategoryEnabled(SettingsCategory.PLUGINS, quickJump.idString)) } @@ -466,6 +462,35 @@ class SettingsSyncPluginManagerTest : BasePluginManagerTest() { } } + @Test + @TestFor(issues = ["IJPL-157266"]) + fun `disable syncing of incompatible plugin`(){ + val weirdPlugin = TestPluginDescriptor( + "org.intellij.weird" + ) + TestPluginDescriptor.ALL.remove(weirdPlugin.pluginId) + testPluginManager.addPluginDescriptors(git4idea) + pluginManager.updateStateFromIdeOnStart(state { + git4idea (enabled = true) // bundled + }) + assertPluginManagerState { + // empty + } + + pushToIdeAndWait(state { + git4idea(enabled = true) + weirdPlugin(enabled = true) + }) + + assertIdeState { + git4idea (enabled = true) + } + assertPluginManagerState { + weirdPlugin(enabled = true) + } + assertFalse(SettingsSyncSettings.getInstance().isSubcategoryEnabled(SettingsCategory.PLUGINS, weirdPlugin.idString)) + } + private fun restart_required_base(installedBefore: Boolean, enabledBefore: Boolean, enabledInPush: Boolean) = runTest { val restartRequiredRef = AtomicReference() SettingsSyncEvents.getInstance().addListener(object : SettingsSyncEventListener { diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTestUtil.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTestUtil.kt index a475e21b3ac7..98b7df1cc24a 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTestUtil.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncPluginManagerTestUtil.kt @@ -16,7 +16,11 @@ internal class TestPluginInstaller(private val afterInstallPluginCallback: (Plug // there's no marketplace to find plugin descriptors, so we'll just populate that in advance - override fun install(installer: PluginDownloader): Boolean { + override suspend fun installCollected(installers: List, settingsAlreadyChanged: Boolean) { + doInstallCollected(installers, settingsAlreadyChanged) + } + + override suspend fun install(installer: PluginDownloader): Boolean { val pluginId = installer.id val descriptor = TestPluginDescriptor.ALL[pluginId] as TestPluginDescriptor if (!descriptor.isDynamic)