From 90bd2858517e1a4dc0f803002aa956631b963fe8 Mon Sep 17 00:00:00 2001 From: Andrey Vlasovskikh Date: Wed, 5 Mar 2014 15:13:28 +0400 Subject: [PATCH 1/2] Extracted getSdkRootVirtualFile() --- .../jetbrains/python/sdk/PythonSdkType.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/python/src/com/jetbrains/python/sdk/PythonSdkType.java b/python/src/com/jetbrains/python/sdk/PythonSdkType.java index 147c9a7c5b6f..b13cb762cae4 100644 --- a/python/src/com/jetbrains/python/sdk/PythonSdkType.java +++ b/python/src/com/jetbrains/python/sdk/PythonSdkType.java @@ -659,9 +659,9 @@ public class PythonSdkType extends SdkType { } public static void addSdkRoot(SdkModificator sdkModificator, String path) { - VirtualFile child = LocalFileSystem.getInstance().refreshAndFindFileByPath(path); - if (child != null) { - addSdkRoot(sdkModificator, child); + final VirtualFile file = LocalFileSystem.getInstance().refreshAndFindFileByPath(path); + if (file != null) { + addSdkRoot(sdkModificator, file); } else { LOG.info("Bogus sys.path entry " + path); @@ -669,19 +669,26 @@ public class PythonSdkType extends SdkType { } private static void addSdkRoot(@NotNull SdkModificator sdkModificator, @NotNull VirtualFile child) { - @NonNls String suffix = child.getExtension(); - if (suffix != null) suffix = suffix.toLowerCase(); // Why on earth empty suffix is null and not ""? - VirtualFile toAdd = child; - if ((!child.isDirectory()) && ("zip".equals(suffix) || "egg".equals(suffix))) { + // NOTE: Files marked as library sources are not considered part of project source. Since the directory of the project the + // user is working on is included in PYTHONPATH with many configurations (e.g. virtualenv), we must not mark SDK paths as + // library sources, only as classes. + sdkModificator.addRoot(getSdkRootVirtualFile(child), OrderRootType.CLASSES); + } + + @NotNull + public static VirtualFile getSdkRootVirtualFile(@NotNull VirtualFile path) { + String suffix = path.getExtension(); + if (suffix != null) { + suffix = suffix.toLowerCase(); // Why on earth empty suffix is null and not ""? + } + if ((!path.isDirectory()) && ("zip".equals(suffix) || "egg".equals(suffix))) { // a .zip / .egg file must have its root extracted first - toAdd = JarFileSystem.getInstance().getJarRootForLocalFile(child); - } - if (toAdd != null) { - // NOTE: Files marked as library sources are not considered part of project source. Since the directory of the project the - // user is working on is included in PYTHONPATH with many configurations (e.g. virtualenv), we must not mark SDK paths as - // library sources, only as classes. - sdkModificator.addRoot(toAdd, OrderRootType.CLASSES); + final VirtualFile jar = JarFileSystem.getInstance().getJarRootForLocalFile(path); + if (jar != null) { + return jar; + } } + return path; } public static String getSkeletonsPath(String basePath, String sdkHome) { From bc22d751e18ea3e5323ad387b304873c771e811c Mon Sep 17 00:00:00 2001 From: Andrey Vlasovskikh Date: Wed, 5 Mar 2014 15:19:09 +0400 Subject: [PATCH 2/2] Compare sys.path roots by virtual file, not by filename because of *.egg archives Added legacy duplicate roots deletion. Duplicates were the result of not filtering *.egg archives properly. Refacotred updateSysPath() to be more readable as well. --- .../python/sdk/PythonSdkUpdater.java | 86 +++++++++++++------ 1 file changed, 59 insertions(+), 27 deletions(-) diff --git a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java index 2cc749f72002..a964fa90246e 100644 --- a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java +++ b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java @@ -31,6 +31,7 @@ import com.intellij.openapi.projectRoots.SdkTypeId; import com.intellij.openapi.roots.OrderRootType; import com.intellij.openapi.startup.StartupActivity; import com.intellij.openapi.util.io.FileUtilRt; +import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VirtualFile; import com.jetbrains.python.PyBundle; import com.jetbrains.python.codeInsight.userSkeletons.PyUserSkeletonsUtil; @@ -38,7 +39,6 @@ import com.jetbrains.python.sdk.skeletons.PySkeletonRefresher; import org.jetbrains.annotations.NotNull; import java.io.File; -import java.io.IOException; import java.util.*; /** @@ -135,7 +135,7 @@ public class PythonSdkUpdater implements StartupActivity { } } - private static void updateSysPath(final Sdk sdk) throws InvalidSdkException { + private static void updateSysPath(@NotNull final Sdk sdk) throws InvalidSdkException { long start_time = System.currentTimeMillis(); final List sysPath = PythonSdkType.getSysPath(sdk.getHomePath()); final VirtualFile file = PyUserSkeletonsUtil.getUserSkeletonsDirectory(); @@ -151,9 +151,29 @@ public class PythonSdkUpdater implements StartupActivity { LOG.info("Updating sys.path took " + (System.currentTimeMillis() - start_time) + " ms"); } - private static void updateSdkPath(Sdk sdk, List sysPath) { + /** + * Updates SDK based on sys.path and cleans legacy information up. + */ + private static void updateSdkPath(@NotNull Sdk sdk, @NotNull List sysPath) { + final SdkModificator modificator = sdk.getSdkModificator(); + boolean changed = addNewSysPathEntries(sdk, modificator, sysPath); + changed = removeSourceRoots(sdk, modificator) || changed; + changed = removeDuplicateClassRoots(sdk, modificator) || changed; + if (changed) { + ApplicationManager.getApplication().runWriteAction(new Runnable() { + @Override + public void run() { + modificator.commitChanges(); + } + }); + } + } + + /** + * Adds new CLASSES entries found in sys.path. + */ + private static boolean addNewSysPathEntries(@NotNull Sdk sdk, @NotNull SdkModificator modificator, @NotNull List sysPath) { final List oldRoots = Arrays.asList(sdk.getRootProvider().getFiles(OrderRootType.CLASSES)); - final VirtualFile[] sourceRoots = sdk.getRootProvider().getFiles(OrderRootType.SOURCES); PythonSdkAdditionalData additionalData = sdk.getSdkAdditionalData() instanceof PythonSdkAdditionalData ? (PythonSdkAdditionalData)sdk.getSdkAdditionalData() : null; @@ -166,37 +186,49 @@ public class PythonSdkUpdater implements StartupActivity { newRoots.add(root); } } - if (!newRoots.isEmpty() || sourceRoots.length > 0) { - final SdkModificator modificator = sdk.getSdkModificator(); + if (!newRoots.isEmpty()) { for (String root : newRoots) { PythonSdkType.addSdkRoot(modificator, root); } - modificator.removeRoots(OrderRootType.SOURCES); - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - modificator.commitChanges(); - } - }); - } - } - - private static boolean wasOldRoot(String root, Collection virtualFiles) { - String rootPath = canonicalize(root); - for (VirtualFile virtualFile : virtualFiles) { - if (canonicalize(virtualFile.getPath()).equals(rootPath)) { - return true; - } + return true; } return false; } - private static String canonicalize(String path) { - try { - return new File(path).getCanonicalPath(); + /** + * Removes duplicate roots that have been added as the result of a bug with *.egg handling. + */ + private static boolean removeDuplicateClassRoots(@NotNull Sdk sdk, @NotNull SdkModificator modificator) { + final List sourceRoots = Arrays.asList(sdk.getRootProvider().getFiles(OrderRootType.CLASSES)); + final LinkedHashSet uniqueRoots = new LinkedHashSet(sourceRoots); + if (uniqueRoots.size() != sourceRoots.size()) { + modificator.removeRoots(OrderRootType.CLASSES); + for (VirtualFile root : uniqueRoots) { + modificator.addRoot(root, OrderRootType.CLASSES); + } + return true; } - catch (IOException e) { - return path; + return false; + } + + /** + * Removes legacy SOURCES entries in Python SDK tables (PY-2891). + */ + private static boolean removeSourceRoots(@NotNull Sdk sdk, @NotNull SdkModificator modificator) { + final VirtualFile[] sourceRoots = sdk.getRootProvider().getFiles(OrderRootType.SOURCES); + if (sourceRoots.length > 0) { + modificator.removeRoots(OrderRootType.SOURCES); + return true; } + return false; + } + + private static boolean wasOldRoot(@NotNull String root, @NotNull Collection oldRoots) { + final VirtualFile file = LocalFileSystem.getInstance().refreshAndFindFileByPath(root); + if (file != null) { + final VirtualFile rootFile = PythonSdkType.getSdkRootVirtualFile(file); + return oldRoots.contains(rootFile); + } + return false; } }