diff --git a/python/pluginCore/resources/META-INF/plugin.xml b/python/pluginCore/resources/META-INF/plugin.xml index b570c939e745..38a1ffc6b2a6 100644 --- a/python/pluginCore/resources/META-INF/plugin.xml +++ b/python/pluginCore/resources/META-INF/plugin.xml @@ -646,6 +646,10 @@ The Python plug-in provides smart editing for Python scripts. The feature set of restartRequired="false" description="Disables automatic updating of PyPI package cache and ranking on project startup."/> + + + diff --git a/python/python-psi-impl/src/com/jetbrains/python/psi/PyUtil.java b/python/python-psi-impl/src/com/jetbrains/python/psi/PyUtil.java index 5a1761562fe6..4b0b13b9785f 100644 --- a/python/python-psi-impl/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/python-psi-impl/src/com/jetbrains/python/psi/PyUtil.java @@ -1024,17 +1024,30 @@ public final class PyUtil { () -> { final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); for (VirtualFile root : roots) { - boolean added = false; for (ContentEntry entry : model.getContentEntries()) { final VirtualFile file = entry.getFile(); if (file != null && VfsUtilCore.isAncestor(file, root, true)) { entry.addSourceFolder(root.getUrl(), JavaSourceRootType.SOURCE, true); - added = true; } } + } + model.commit(); + } + ); + } - if (!added) { - model.addContentEntry(root).addSourceFolder(root.getUrl(), JavaSourceRootType.SOURCE, true); + @RequiresEdt + public static void addModuleDependencies(@NotNull Module module, @NotNull Collection dependencies) { + if (dependencies.isEmpty()) { + return; + } + + ApplicationManager.getApplication().runWriteAction( + () -> { + ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); + for (Module dependency : dependencies) { + if (dependency != module && model.findModuleOrderEntry(dependency) == null) { + model.addModuleOrderEntry(dependency); } } model.commit(); @@ -1057,9 +1070,25 @@ public final class PyUtil { entry.removeSourceFolder(folder); } } + } + model.commit(); + } + ); + } - if (roots.contains(entry.getFile()) && entry.getSourceFolders().length == 0) { - model.removeContentEntry(entry); + @RequiresEdt + public static void removeModuleDependencies(@NotNull Module module, @NotNull Collection dependencies) { + if (dependencies.isEmpty()) { + return; + } + + ApplicationManager.getApplication().runWriteAction( + () -> { + final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); + for (Module dependency : dependencies) { + ModuleOrderEntry moduleOrderEntry = model.findModuleOrderEntry(dependency); + if (moduleOrderEntry != null) { + model.removeOrderEntry(moduleOrderEntry); } } model.commit(); diff --git a/python/src/com/jetbrains/python/sdk/PyTransferredSdkRoots.kt b/python/src/com/jetbrains/python/sdk/PyTransferredSdkRoots.kt index 9da6d7a2769e..c4e039490096 100644 --- a/python/src/com/jetbrains/python/sdk/PyTransferredSdkRoots.kt +++ b/python/src/com/jetbrains/python/sdk/PyTransferredSdkRoots.kt @@ -1,14 +1,19 @@ // Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.jetbrains.python.sdk +import com.google.common.collect.MultimapBuilder +import com.google.common.collect.SetMultimap import com.intellij.openapi.application.runInEdt import com.intellij.openapi.application.runReadAction import com.intellij.openapi.application.runWriteAction +import com.intellij.openapi.diagnostic.thisLogger import com.intellij.openapi.module.Module import com.intellij.openapi.module.ModuleManager import com.intellij.openapi.project.Project import com.intellij.openapi.projectRoots.Sdk import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.util.registry.Registry +import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.vfs.VirtualFile import com.jetbrains.python.psi.PyUtil @@ -26,6 +31,16 @@ fun removeTransferredRootsFromModulesWithSdk(project: Project, sdk: Sdk) { updateRootsForModulesWithSdk(project, sdk, ::removeTransferredRoots) } +private fun updateRootsForModulesWithSdk(project: Project, sdk: Sdk?, action: (Module, Sdk) -> Unit) { + if (sdk == null) { + return + } + + for (module in runReadAction { ModuleManager.getInstance(project).modules }) { + action(module, sdk) + } +} + /** * Applies [transferRoots] to all modules inheriting python sdk from the [project]. */ @@ -61,34 +76,66 @@ fun setPathsToTransfer(sdk: Sdk, roots: Set) { } /** - * Turns [getPathsToTransfer] result into [module] source roots if [module] python sdk is [sdk]. + * Turns [getPathsToTransfer] result into [module] source roots and dependencies if [module] python sdk is [sdk]. */ fun transferRoots(module: Module, sdk: Sdk?) { if (sdk != null && module.pythonSdk == sdk) { runInEdt { - PyUtil.addSourceRoots(module, getPathsToTransfer(sdk)) + val transferredRoots = getPathsToTransfer(sdk) + val newTransferredRoots = TransferredRootsDetector(module.project).detect(module, transferredRoots, transferredRoots) + addTransferredRoots(module, newTransferredRoots) } } } +private fun addTransferredRoots(module: Module, newTransferredRoots: ModuleTransferredRoots) { + ModuleRootAndDepOps.LOG.info("Adding source roots ${newTransferredRoots.sourceRoots} to module ${module}") + PyUtil.addSourceRoots(module, newTransferredRoots.sourceRoots) + ModuleRootAndDepOps.LOG.info("Adding dependencies ${newTransferredRoots.dependencies} to module ${module}") + PyUtil.addModuleDependencies(module, newTransferredRoots.dependencies) +} + /** - * Removes [getPathsToTransfer] result from [module] source roots if [module] python sdk is [sdk]. + * Removes [getPathsToTransfer] result from [module] source roots and dependencies if [module] python sdk is [sdk]. */ fun removeTransferredRoots(module: Module, sdk: Sdk?) { if (sdk != null && module.pythonSdk == sdk) { runInEdt { - PyUtil.removeSourceRoots(module, getPathsToTransfer(sdk)) + val transferredRoots = getPathsToTransfer(sdk) + val newTransferredRoots = TransferredRootsDetector(module.project).detect(module, transferredRoots, transferredRoots) + removeTransferredRoots(module, newTransferredRoots) } } } -private fun updateRootsForModulesWithSdk(project: Project, sdk: Sdk?, action: (Module, Sdk) -> Unit) { - if (sdk == null) { - return - } +private fun removeTransferredRoots(module: Module, newTransferredRoots: ModuleTransferredRoots) { + ModuleRootAndDepOps.LOG.info("Removing source roots ${newTransferredRoots.sourceRoots} from module ${module}") + PyUtil.removeSourceRoots(module, newTransferredRoots.sourceRoots) + ModuleRootAndDepOps.LOG.info("Removing dependencies ${newTransferredRoots.dependencies} from module ${module}") + PyUtil.removeModuleDependencies(module, newTransferredRoots.dependencies) +} - for (module in runReadAction { ModuleManager.getInstance(project).modules }) { - action(module, sdk) +fun updateTransferredRoots(project: Project, sdk: Sdk, newInProjectPaths: Set) { + val rootsDetector = TransferredRootsDetector(project) + val modulesWithThisSdk = rootsDetector.projectModules.filter { it.pythonSdk == sdk } + val oldTransferredRoots = getPathsToTransfer(sdk) + val oldTransferredRootsStructure = modulesWithThisSdk.associateWith { rootsDetector.detect(it, oldTransferredRoots, oldTransferredRoots) } + val newTransferredRootsStructure = modulesWithThisSdk.associateWith { rootsDetector.detect(it, newInProjectPaths, oldTransferredRoots) } + val newTransferredRoots = newTransferredRootsStructure.values + .flatMap { it.sourceRoots + it.dependencies.mapNotNull { it.baseDir } } + .toSet() + + if (oldTransferredRoots != newTransferredRoots) { + for ((module, newModuleRoots) in newTransferredRootsStructure.entries) { + runInEdt { + val oldModuleRoots = oldTransferredRootsStructure[module] + if (oldModuleRoots != null) { + removeTransferredRoots(module, oldModuleRoots) + } + addTransferredRoots(module, newModuleRoots) + } + } + setPathsToTransfer(sdk, newTransferredRoots) } } @@ -106,3 +153,48 @@ private fun updateRootsForModulesWithInheritedSdk(project: Project, sdk: Sdk?, a } } +private object ModuleRootAndDepOps { + val LOG = thisLogger() +} + +private data class ModuleTransferredRoots(val sourceRoots: Set, val dependencies: Set) + +private class TransferredRootsDetector(private val project: Project) { + val projectModules: List = runReadAction { ModuleManager.getInstance(project).modules }.toList() + val moduleToContentRoots: SetMultimap = MultimapBuilder.hashKeys().hashSetValues().build() + val moduleToSourceRoots: SetMultimap = MultimapBuilder.hashKeys().hashSetValues().build() + + init { + for (mod in projectModules) { + val moduleRootManager = ModuleRootManager.getInstance(mod) + moduleToContentRoots.putAll(mod, moduleRootManager.contentRoots.toList()) + moduleToSourceRoots.putAll(mod, moduleRootManager.sourceRoots.toList()) + } + } + + fun detect(module: Module, newPaths: Set, oldPaths: Set): ModuleTransferredRoots { + val sourceRoots = linkedSetOf() + val dependencies = linkedSetOf() + val manuallyMarkedModuleSourceRoots = moduleToSourceRoots[module] - oldPaths + val moduleContentRoots = moduleToContentRoots[module] + for (path in newPaths) { + val moduleForPath = moduleToContentRoots.entries() + .filter { VfsUtil.isAncestor(it.value, path, false) } + // Select the closest content root containing the file in case of nested modules + .maxByOrNull { it.value.path.length } + ?.key + if (moduleForPath == null) continue + if (moduleForPath == module) { + if (path !in moduleContentRoots && path !in manuallyMarkedModuleSourceRoots) { + sourceRoots.add(path) + } + } + else if (path == moduleForPath.baseDir) { + if (Registry.`is`("python.detect.cross.module.dependencies")) { + dependencies.add(moduleForPath) + } + } + } + return ModuleTransferredRoots(sourceRoots, dependencies) + } +} \ No newline at end of file diff --git a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java index 87f78dc33347..4a855a518648 100644 --- a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java +++ b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java @@ -28,6 +28,7 @@ import com.intellij.openapi.ui.Messages; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.io.FileUtilRt; +import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.StandardFileSystems; import com.intellij.openapi.vfs.VfsUtilCore; @@ -490,30 +491,39 @@ public final class PythonSdkUpdater { final List localSdkPaths = buildSdkPaths(sdk, sdkRoots.first, userAddedRoots.first); commitSdkPathsIfChanged(sdk, localSdkPaths, forceCommit); - final var pathsToTransfer = new HashSet(); - pathsToTransfer.addAll(sdkRoots.second); - pathsToTransfer.addAll(userAddedRoots.second); - // Presumably source and content roots that were configured manually by user, not set up automatically as "transferred" - HashSet nonTransferredModuleRoots = new HashSet<>(moduleRoots); - nonTransferredModuleRoots.removeAll(PyTransferredSdkRootsKt.getPathsToTransfer(sdk)); - pathsToTransfer.removeAll(nonTransferredModuleRoots); - - /* - PyTransferredSdkRootsKt#transferRoots and PyTransferredSdkRootsKt#removeTransferredRoots skip sdks - that are not equal to module one (editable as well). - - That's why roots changes were not applied but paths to transfer were successfully set. - - When current method was executed for original sdk, - roots changes were not applied since there were no changes in paths to transfer (they were shared with editable copy). - */ - if (!pathsToTransfer.equals(PyTransferredSdkRootsKt.getPathsToTransfer(sdk))) { + if (Registry.is("python.detect.cross.module.dependencies")) { + final var transferredPathCandidates = new HashSet(); + transferredPathCandidates.addAll(sdkRoots.second); + transferredPathCandidates.addAll(userAddedRoots.second); if (project != null) { - PyTransferredSdkRootsKt.removeTransferredRootsFromModulesWithSdk(project, sdk); + PyTransferredSdkRootsKt.updateTransferredRoots(project, sdk, transferredPathCandidates); } - PyTransferredSdkRootsKt.setPathsToTransfer(sdk, pathsToTransfer); - if (project != null) { - PyTransferredSdkRootsKt.transferRootsToModulesWithSdk(project, sdk); + } + else { + final var pathsToTransfer = new HashSet(); + pathsToTransfer.addAll(sdkRoots.second); + pathsToTransfer.addAll(userAddedRoots.second); + // Presumably source and content roots that were configured manually by user, not set up automatically as "transferred" + HashSet nonTransferredModuleRoots = new HashSet<>(moduleRoots); + nonTransferredModuleRoots.removeAll(PyTransferredSdkRootsKt.getPathsToTransfer(sdk)); + pathsToTransfer.removeAll(nonTransferredModuleRoots); + /* + PyTransferredSdkRootsKt#transferRoots and PyTransferredSdkRootsKt#removeTransferredRoots skip sdks + that are not equal to module one (editable as well). + + That's why roots changes were not applied but paths to transfer were successfully set. + + When current method was executed for original sdk, + roots changes were not applied since there were no changes in paths to transfer (they were shared with editable copy). + */ + if (!pathsToTransfer.equals(PyTransferredSdkRootsKt.getPathsToTransfer(sdk))) { + if (project != null) { + PyTransferredSdkRootsKt.removeTransferredRootsFromModulesWithSdk(project, sdk); + } + PyTransferredSdkRootsKt.setPathsToTransfer(sdk, pathsToTransfer); + if (project != null) { + PyTransferredSdkRootsKt.transferRootsToModulesWithSdk(project, sdk); + } } } } diff --git a/python/testSrc/com/jetbrains/python/sdk/PySdkPathsTest.kt b/python/testSrc/com/jetbrains/python/sdk/PySdkPathsTest.kt index e1c9e4312085..9f20d1389bc9 100644 --- a/python/testSrc/com/jetbrains/python/sdk/PySdkPathsTest.kt +++ b/python/testSrc/com/jetbrains/python/sdk/PySdkPathsTest.kt @@ -7,23 +7,26 @@ import com.intellij.openapi.application.runWriteActionAndWait import com.intellij.openapi.module.Module import com.intellij.openapi.projectRoots.ProjectJdkTable import com.intellij.openapi.projectRoots.Sdk +import com.intellij.openapi.roots.ModuleRootManager import com.intellij.openapi.roots.OrderRootType import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.io.FileUtil +import com.intellij.openapi.util.registry.Registry import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.vfs.VirtualFile import com.intellij.testFramework.* import com.intellij.testFramework.assertions.Assertions.assertThat import com.intellij.testFramework.rules.ProjectModelRule -import com.jetbrains.python.PyNames import com.jetbrains.python.PythonMockSdk import com.jetbrains.python.PythonPluginDisposable -import com.jetbrains.python.configuration.PyConfigurableInterpreterList import com.jetbrains.python.psi.LanguageLevel import com.jetbrains.python.psi.PyUtil import org.jetbrains.annotations.NotNull -import org.junit.* +import org.junit.Assume.assumeTrue +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test class PySdkPathsTest { @@ -220,43 +223,52 @@ class PySdkPathsTest { } @Test - fun sysPathEntryOutsideSdkAndModule1ButInsideModule2() { + fun sysPathEntryInsideAnotherModuleDoesNotConfigureSourceRootThere() { val (module1, moduleRoot1) = createModule("m1") - val (module2, moduleRoot2) = createModule("m2") - - val sdkDir = createVenvStructureInModule(moduleRoot1) + val (_, moduleRoot2) = createModule("m2") val entryPath1 = createSubdir(moduleRoot1) val entryPath2 = createSubdir(moduleRoot2) - val sdk = PythonMockSdk.create().let { - val properSdk = PythonMockSdk.create("Mock SDK without path", sdkDir.path, it.sdkType, LanguageLevel.getLatest()) - registerSdk(properSdk) - module1.pythonSdk = properSdk - module2.pythonSdk = properSdk - return@let properSdk + val sdk = PythonMockSdk.create(createVenvStructureInModule(moduleRoot1).path).also { + registerSdk(it) + module1.pythonSdk = it } sdk.putUserData(PythonSdkType.MOCK_SYS_PATH_KEY, listOf(sdk.homePath, entryPath1.path, entryPath2.path)) mockPythonPluginDisposable() updateSdkPaths(sdk) - checkRoots(sdk, module1, listOf(moduleRoot1, entryPath1, entryPath2), emptyList()) - checkRoots(sdk, module2, listOf(moduleRoot2, entryPath1, entryPath2), emptyList()) + checkRoots(sdk, module1, listOf(moduleRoot1, entryPath1), emptyList()) + } - val simpleSdk = PythonMockSdk.create().also { + @Test + fun sysPathEntryPointingToAnotherModuleRootConfiguresModuleDependency() { + assumeTrue("The registry key 'python.detect.cross.module.dependencies' is not enabled", + Registry.`is`("python.detect.cross.module.dependencies")) + + val (module1, moduleRoot1) = createModule("m1") + val (module2, moduleRoot2) = createModule("m2") + + val sdk = PythonMockSdk.create(createVenvStructureInModule(moduleRoot1).path).also { registerSdk(it) - removeTransferredRoots(module1, sdk) module1.pythonSdk = it - - removeTransferredRoots(module2, sdk) - module2.pythonSdk = it } + mockPythonPluginDisposable() + + sdk.putUserData(PythonSdkType.MOCK_SYS_PATH_KEY, listOf(moduleRoot2.path)) + updateSdkPaths(sdk) + checkRoots(sdk, module1, + moduleRoots = listOf(moduleRoot1), + sdkRoots = listOf(), + moduleDependencies = listOf(module2)) - updateSdkPaths(simpleSdk) - - checkRoots(simpleSdk, module1, listOf(moduleRoot1), emptyList()) - checkRoots(simpleSdk, module2, listOf(moduleRoot2), emptyList()) + sdk.putUserData(PythonSdkType.MOCK_SYS_PATH_KEY, listOf()) + updateSdkPaths(sdk) + checkRoots(sdk, module1, + moduleRoots = listOf(moduleRoot1), + sdkRoots = listOf(), + moduleDependencies = listOf()) } private fun registerSdk(it: Sdk) { @@ -310,7 +322,13 @@ class PySdkPathsTest { ApplicationManager.getApplication().invokeAndWait { PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue() } } - private fun checkRoots(sdk: Sdk, module: Module, moduleRoots: List, sdkRoots: List) { + private fun checkRoots( + sdk: Sdk, + module: Module, + moduleRoots: List, + sdkRoots: List, + moduleDependencies: List = emptyList(), + ) { assertThat(PyUtil.getSourceRoots(module)).containsExactlyInAnyOrder(*moduleRoots.toTypedArray()) val rootProvider = sdk.rootProvider @@ -318,6 +336,9 @@ class PySdkPathsTest { assertThat(classes).containsAll(sdkRoots) assertThat(classes).doesNotContain(*moduleRoots.toTypedArray()) assertThat(rootProvider.getFiles(OrderRootType.SOURCES)).isEmpty() + + val actualModuleDependencies = ModuleRootManager.getInstance(module).getModifiableModel().moduleDependencies + assertThat(actualModuleDependencies).isEqualTo(moduleDependencies.toTypedArray()) } private fun createInSdkRoot(sdk: Sdk, relativePath: String): VirtualFile {