IJPL-157266 Exclude automatically disabled plugins from Settings Sync

GitOrigin-RevId: b66e2c30957490fa218178ec1951bdc62e021dc1

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<PluginId>) {
     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<PluginDownloader>, 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<PluginDownloader>) {
+  internal suspend fun doInstallCollected(installers: List<PluginDownloader>, settingsAlreadyChanged: Boolean) {
     val pluginsRequiredRestart = mutableListOf<String>()
-    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<PluginId>): List<PluginDownloader> {
     val compatibleUpdates = MarketplaceRequests.getLastCompatiblePluginUpdate(pluginIds.toSet())
     val retval = arrayListOf<PluginDownloader>()
-    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
   }
 }

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)

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<RestartReason>()
     SettingsSyncEvents.getInstance().addListener(object : SettingsSyncEventListener {

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<PluginDownloader>, 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)