diff --git a/python/python-sdk/BUILD.bazel b/python/python-sdk/BUILD.bazel index 11cb65734c09..ba9213e21558 100644 --- a/python/python-sdk/BUILD.bazel +++ b/python/python-sdk/BUILD.bazel @@ -241,6 +241,14 @@ jvm_library( "//platform/workspace/jps:jps_test_lib", "//platform/workspace/storage", "//platform/workspace/storage:storage_test_lib", + "//platform/testFramework/junit5", + "//platform/testFramework/junit5:junit5_test_lib", + "//python/python-test-env/junit5", + "//python/python-test-env/junit5:junit5_test_lib", + "//python/junit5Tests-framework:community-junit5Tests-framework", + "//python/junit5Tests-framework:community-junit5Tests-framework_test_lib", + "//libraries/kotlinx/coroutines/core", + "//libraries/kotlinx/coroutines/core:core_test_lib", ], ) ### auto-generated section `build intellij.python.sdk.tests` end diff --git a/python/python-sdk/intellij.python.sdk.tests.iml b/python/python-sdk/intellij.python.sdk.tests.iml index 94429f59939f..1ea78b462e02 100644 --- a/python/python-sdk/intellij.python.sdk.tests.iml +++ b/python/python-sdk/intellij.python.sdk.tests.iml @@ -45,6 +45,10 @@ + + + + \ No newline at end of file diff --git a/python/python-sdk/resources/messages/PySdkBundle.properties b/python/python-sdk/resources/messages/PySdkBundle.properties index fbd5ccc00d4a..fa32585817ff 100644 --- a/python/python-sdk/resources/messages/PySdkBundle.properties +++ b/python/python-sdk/resources/messages/PySdkBundle.properties @@ -72,6 +72,10 @@ python.configure.interpreter.action=Configure a Python interpreter\u2026 python.configuring.interpreter.progress=Configuring a python interpreter python.configuring.interpreter.progress.title=Configuring a Python Interpreter +# Rename interpreter +python.sdk.rename.interpreter.name.already.exists=An interpreter named ''{0}'' already exists +python.sdk.rename.interpreter.not.found=Interpreter ''{0}'' was not found + # Skeletons generator dialog.message.broken.home.path.for=Broken home path for {0} diff --git a/python/python-sdk/src/com/jetbrains/python/sdk/ProjectExt.kt b/python/python-sdk/src/com/jetbrains/python/sdk/ProjectExt.kt new file mode 100644 index 000000000000..61a104be70c8 --- /dev/null +++ b/python/python-sdk/src/com/jetbrains/python/sdk/ProjectExt.kt @@ -0,0 +1,74 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.jetbrains.python.sdk + +import com.intellij.openapi.module.ModuleManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.projectRoots.ProjectJdkTable +import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.roots.ModuleRootModificationUtil +import com.intellij.openapi.roots.ProjectRootManager +import com.intellij.util.concurrency.annotations.RequiresWriteLock +import com.jetbrains.python.Result +import com.jetbrains.python.errorProcessing.PyResult +import com.jetbrains.python.sdk.impl.PySdkBundle.message +import org.jetbrains.annotations.ApiStatus.Internal + +/** + * Renames the SDK currently registered as [oldName] to [newName] and keeps this project's references pointing at it. + * + * Returns a failure if [newName] is already used by another SDK; on success the project SDK and every module SDK dependency that + * referenced the SDK keep pointing at it. + * + * Renaming the SDK fires `jdkNameChanged`. When the SDK is referenced by BOTH the project SDK and a module SDK dependency, two + * listeners (`ProjectRootManagerImpl` + `ModuleDependencyIndexImpl`) each call `updateProjectModel`, and the multiverse context + * provider re-pumps the message bus between them, producing a recursive `updateProjectModel` that aborts the reference rewrite and + * drops the interpreter's association (PY-88229). To avoid that, the project SDK reference is detached for the duration of the rename + * so only one listener updates the project model; it is always restored afterwards (even if the rename fails), so the project never + * ends up without an interpreter. The project SDK and explicit module SDK references are then re-pointed to the renamed SDK. + */ +@Internal +@RequiresWriteLock +fun Project.renameSdk(oldName: String, newName: String): PyResult { + val jdkTable = ProjectJdkTable.getInstance() + val sdk = jdkTable.findJdk(oldName) + ?: return PyResult.localizedError(message("python.sdk.rename.interpreter.not.found", oldName)) + + if (oldName == newName) { + return PyResult.success(Unit) + } + + if (jdkTable.findJdk(newName) != null) { + return PyResult.localizedError(message("python.sdk.rename.interpreter.name.already.exists", newName)) + } + + + val projectRootManager = ProjectRootManager.getInstance(this) + // Capture modules whose explicit (non-inherited) SDK is this one; inherited modules follow the project SDK and need no action. + val modulesWithExplicitSdk = ModuleManager.getInstance(this).modules.filter { module -> + val rootManager = ModuleRootManager.getInstance(module) + !rootManager.isSdkInherited && rootManager.sdk === sdk + } + val isProjectSdk = projectRootManager.projectSdk?.name == oldName + if (isProjectSdk) { + projectRootManager.projectSdk = null + } + + try { + sdk.sdkModificator.let { + it.name = newName + it.commitChanges() + } + } + finally { + // Restore the project SDK reference: the renamed SDK on success, the unchanged one if the rename failed. + if (isProjectSdk) { + projectRootManager.projectSdk = sdk + } + } + + // Re-point the explicit module references to the SDK, which is now renamed in place. + for (module in modulesWithExplicitSdk) { + ModuleRootModificationUtil.setModuleSdk(module, sdk) + } + return Result.success(Unit) +} diff --git a/python/python-sdk/testResources/intellij.python.sdk.tests.xml b/python/python-sdk/testResources/intellij.python.sdk.tests.xml index d51f07172516..fadecf502a8d 100644 --- a/python/python-sdk/testResources/intellij.python.sdk.tests.xml +++ b/python/python-sdk/testResources/intellij.python.sdk.tests.xml @@ -2,12 +2,13 @@ - + + @@ -17,6 +18,8 @@ + + \ No newline at end of file diff --git a/python/python-sdk/tests/com/intellij/python/junit5Tests/env/PythonSdkRenameTest.kt b/python/python-sdk/tests/com/intellij/python/junit5Tests/env/PythonSdkRenameTest.kt new file mode 100644 index 000000000000..650e5ff01ec0 --- /dev/null +++ b/python/python-sdk/tests/com/intellij/python/junit5Tests/env/PythonSdkRenameTest.kt @@ -0,0 +1,151 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.python.junit5Tests.env + +import com.intellij.openapi.application.edtWriteAction +import com.intellij.openapi.projectRoots.ProjectJdkTable +import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.roots.ModuleRootModificationUtil +import com.intellij.openapi.roots.ProjectRootManager +import com.intellij.python.junit5Tests.framework.env.PyEnvTestCase +import com.intellij.python.junit5Tests.framework.env.pySdkFixture +import com.intellij.python.test.env.junit5.pyVenvFixture +import com.intellij.testFramework.junit5.fixture.moduleFixture +import com.intellij.testFramework.junit5.fixture.projectFixture +import com.intellij.testFramework.junit5.fixture.tempPathFixture +import com.jetbrains.python.sdk.renameSdk +import com.jetbrains.python.Result +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * Renaming an interpreter registered in the [ProjectJdkTable] must rename it in the table and keep every workspace-model reference + * (the project SDK and any module SDK dependencies) pointing at it (PY-88229: previously these references were left dangling and the + * interpreter showed up as "No interpreter"). + */ +@PyEnvTestCase +class PythonSdkRenameTest { + private val projectFixture = projectFixture() + private val moduleAFixture = projectFixture.moduleFixture(tempPathFixture(), addPathToSourceRoot = true) + private val moduleBFixture = projectFixture.moduleFixture(tempPathFixture(), addPathToSourceRoot = true) + + // A venv registered in the project JDK table but not associated with any module; each test wires up the references it needs. + private val venvFixture = pySdkFixture().pyVenvFixture(where = tempPathFixture(), addToSdkTable = true) + + // A second, independent venv used to check renaming onto an already used name. + private val secondVenvFixture = pySdkFixture().pyVenvFixture(where = tempPathFixture(), addToSdkTable = true) + + @Test + fun renameUpdatesProjectSdkReference(): Unit = runBlocking { + val project = projectFixture.get() + val sdk = venvFixture.get() + val oldName = sdk.name + val newName = "$oldName renamed" + edtWriteAction { ProjectRootManager.getInstance(project).projectSdk = sdk } + + edtWriteAction { project.renameSdk(oldName, newName) } + + assertSdkRenamedInTable(oldName, newName) + assertEquals(newName, ProjectRootManager.getInstance(project).projectSdk?.name, "Project SDK reference must follow the rename") + } + + @Test + fun renameUpdatesModuleSdkReferenceWithoutProjectSdk(): Unit = runBlocking { + val project = projectFixture.get() + val module = moduleAFixture.get() + val sdk = venvFixture.get() + val oldName = sdk.name + val newName = "$oldName renamed" + edtWriteAction { ModuleRootModificationUtil.setModuleSdk(module, sdk) } + + edtWriteAction { project.renameSdk(oldName, newName) } + + assertSdkRenamedInTable(oldName, newName) + assertEquals(newName, ModuleRootManager.getInstance(module).sdk?.name, "Module SDK reference must follow the rename") + assertNull(ProjectRootManager.getInstance(project).projectSdk, "Project SDK must stay unset") + } + + @Test + fun renameUpdatesBothProjectAndModuleReferences(): Unit = runBlocking { + val project = projectFixture.get() + val module = moduleAFixture.get() + val sdk = venvFixture.get() + val oldName = sdk.name + val newName = "$oldName renamed" + edtWriteAction { + ProjectRootManager.getInstance(project).projectSdk = sdk + ModuleRootModificationUtil.setModuleSdk(module, sdk) + } + + edtWriteAction { project.renameSdk(oldName, newName) } + + assertSdkRenamedInTable(oldName, newName) + assertEquals(newName, ProjectRootManager.getInstance(project).projectSdk?.name, "Project SDK reference must follow the rename") + assertEquals(newName, ModuleRootManager.getInstance(module).sdk?.name, "Module SDK reference must follow the rename") + } + + @Test + fun renameUpdatesAllModulesSharingTheSameSdk(): Unit = runBlocking { + val project = projectFixture.get() + val moduleA = moduleAFixture.get() + val moduleB = moduleBFixture.get() + val sdk = venvFixture.get() + val oldName = sdk.name + val newName = "$oldName renamed" + edtWriteAction { + ModuleRootModificationUtil.setModuleSdk(moduleA, sdk) + ModuleRootModificationUtil.setModuleSdk(moduleB, sdk) + } + + edtWriteAction { project.renameSdk(oldName, newName) } + + assertSdkRenamedInTable(oldName, newName) + assertEquals(newName, ModuleRootManager.getInstance(moduleA).sdk?.name, "Module A SDK reference must follow the rename") + assertEquals(newName, ModuleRootManager.getInstance(moduleB).sdk?.name, "Module B SDK reference must follow the rename") + } + + @Test + fun renamingToAnExistingNameDoesNotCreateDuplicate(): Unit = runBlocking { + val project = projectFixture.get() + val first = venvFixture.get() + val second = secondVenvFixture.get() + val firstName = first.name + val secondName = second.name + + // SDK names must stay unique in the project JDK table, so renaming the second interpreter to the first's name must be rejected + // and leave the table untouched. + val result = edtWriteAction { project.renameSdk(secondName, firstName) } + assertTrue(result is Result.Failure, "Renaming to an already used name must fail") + + val jdkTable = ProjectJdkTable.getInstance() + assertEquals(first, jdkTable.findJdk(firstName), "The existing interpreter must keep its name") + assertEquals(second, jdkTable.findJdk(secondName), "The interpreter being renamed must keep its original name") + } + + @Test + fun renamingToTheSameNameIsANoOp(): Unit = runBlocking { + val project = projectFixture.get() + val sdk = venvFixture.get() + val name = sdk.name + + val result = edtWriteAction { project.renameSdk(name, name) } + assertTrue(result is Result.Success, "Renaming to the same name must be a no-op success") + assertEquals(sdk, ProjectJdkTable.getInstance().findJdk(name), "The interpreter must be left unchanged") + } + + @Test + fun renamingAnUnknownSdkFails(): Unit = runBlocking { + val project = projectFixture.get() + + val result = edtWriteAction { project.renameSdk("no such interpreter", "another name") } + assertTrue(result is Result.Failure, "Renaming an interpreter that is not in the project JDK table must fail") + } + + private fun assertSdkRenamedInTable(oldName: String, newName: String) { + val jdkTable = ProjectJdkTable.getInstance() + assertNull(jdkTable.findJdk(oldName), "Old SDK name must be gone from the project JDK table") + assertEquals(newName, jdkTable.findJdk(newName)?.name, "Renamed SDK must be present under the new name") + } +} diff --git a/python/src/com/jetbrains/python/configuration/PythonInterpreterDetailsConfigurable.kt b/python/src/com/jetbrains/python/configuration/PythonInterpreterDetailsConfigurable.kt index 1abd0abd24d8..4fdf2fc4d9e2 100644 --- a/python/src/com/jetbrains/python/configuration/PythonInterpreterDetailsConfigurable.kt +++ b/python/src/com/jetbrains/python/configuration/PythonInterpreterDetailsConfigurable.kt @@ -1,7 +1,6 @@ // Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.jetbrains.python.configuration -import com.intellij.openapi.application.WriteAction import com.intellij.openapi.module.Module import com.intellij.openapi.options.Configurable import com.intellij.openapi.project.Project @@ -21,35 +20,22 @@ class PythonInterpreterDetailsConfigurable(project: Project, parentConfigurable: Configurable) : NamedConfigurable() { private val underlyingConfigurable: Configurable = createPythonInterpreterConfigurable(project, module, sdk, parentConfigurable) - private var initialSdkName: String = sdk.name - private var currentSdkName: @NlsSafe String = sdk.name - override fun isModified(): Boolean = initialSdkName != currentSdkName || underlyingConfigurable.isModified + override fun isModified(): Boolean = underlyingConfigurable.isModified override fun apply() { underlyingConfigurable.apply() - - if (currentSdkName != initialSdkName) { - WriteAction.run { - val sdkModificator = sdk.sdkModificator - sdkModificator.name = currentSdkName - sdkModificator.commitChanges() - initialSdkName = currentSdkName - } - } } - override fun getDisplayName(): @NlsSafe String { - return currentSdkName - } + override fun getDisplayName(): @NlsSafe String = sdk.name - override fun setDisplayName(name: String?) { - currentSdkName = name.orEmpty() - } + // The interpreter is renamed via PythonInterpreterMasterDetails' "Rename" action (which mutates the SDK directly), not by inline + // tree editing, so there is nothing to store here. + override fun setDisplayName(name: String?) {} override fun getEditableObject(): Sdk = sdk - override fun getBannerSlogan(): String = currentSdkName + override fun getBannerSlogan(): String = sdk.name override fun createOptionsPanel(): JComponent = underlyingConfigurable.createComponent()?.apply { setDefaultBorder() } ?: JPanel() diff --git a/python/src/com/jetbrains/python/configuration/PythonInterpreterMasterDetails.kt b/python/src/com/jetbrains/python/configuration/PythonInterpreterMasterDetails.kt index 36b05ce39140..abf3af674ecd 100644 --- a/python/src/com/jetbrains/python/configuration/PythonInterpreterMasterDetails.kt +++ b/python/src/com/jetbrains/python/configuration/PythonInterpreterMasterDetails.kt @@ -10,10 +10,12 @@ import com.intellij.openapi.actionSystem.CommonShortcuts import com.intellij.openapi.actionSystem.DefaultActionGroup import com.intellij.openapi.actionSystem.Presentation import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.diagnostic.thisLogger import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory import com.intellij.openapi.options.Configurable import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.DumbAwareToggleAction +import com.intellij.openapi.projectRoots.ProjectJdkTable import com.intellij.openapi.projectRoots.Sdk import com.intellij.openapi.roots.OrderRootType import com.intellij.openapi.ui.InputValidatorEx @@ -33,6 +35,8 @@ import com.jetbrains.python.sdk.customizeWithSdkValue import com.jetbrains.python.sdk.isAssociatedWithAnotherModule import com.jetbrains.python.sdk.legacy.PythonSdkUtil import com.jetbrains.python.sdk.noInterpreterMarker +import com.jetbrains.python.sdk.renameSdk +import com.jetbrains.python.onFailure import javax.swing.JTree import javax.swing.tree.DefaultMutableTreeNode import javax.swing.tree.DefaultTreeModel @@ -101,10 +105,7 @@ internal class PythonInterpreterMasterDetails(private val moduleOrProject: Modul ) { val configurable = (value as? DefaultMutableTreeNode)?.userObject as? PythonInterpreterDetailsConfigurable val sdk = configurable?.sdk - // The name might have been changed with "Rename" action and stored in `displayName`, while the change not being reflected in `sdk` - // instance yet - val currentSdkName = configurable?.displayName - customizeWithSdkValue(sdk, noInterpreterMarker, nullSdkValue = null, actualSdkName = currentSdkName) + customizeWithSdkValue(sdk, noInterpreterMarker, nullSdkValue = null) } } @@ -207,7 +208,8 @@ internal class PythonInterpreterMasterDetails(private val moduleOrProject: Modul override fun actionPerformed(e: AnActionEvent) { val selectedSdk = getSelectedSdk() ?: return val initialName = selectedSdk.name - val allNames: List = myRoot.children().asSequence().mapNotNull { (it as? MyNode)?.displayName }.toList() + // SDK names must be unique across the whole project JDK table (all SDK types), not only among the interpreters shown here. + val allNames: List = ProjectJdkTable.getInstance().allJdks.map { it.name } val name = Messages.showInputDialog( myTree, PyBundle.message("python.interpreters.rename.interpreter.dialog.message"), @@ -234,8 +236,23 @@ internal class PythonInterpreterMasterDetails(private val moduleOrProject: Modul ) // Skip changing the name if either the dialog is cancelled or the name is not changed if (name == null || name == initialName) return - // Delegate changing the name to the configurable - selectedConfigurable?.displayName = name + ApplicationManager.getApplication().runWriteAction { + // Rename the registered SDK in the project JDK table and re-point the project/module references to it. + project.renameSdk(initialName, name).onFailure { + // The rename dialog already rejects duplicate names, so this is only a defensive fallback. + thisLogger().warn("Cannot rename interpreter '$initialName' to '$name': $it") + return@runWriteAction + } + + // `renameSdk` renamed the registered SDK. For an existing interpreter the tree shows a separate editable copy from + // `ProjectSdksModel`; rename it too so that `ProjectSdksModel.apply()` does not revert the change. + if (selectedSdk.name == initialName) { + selectedSdk.sdkModificator.let { + it.name = name + it.commitChanges() + } + } + } (myTree.model as? DefaultTreeModel)?.nodeChanged(selectedNode) myTree.revalidate() }