From 723925336525f29f71688259a53077428267e066 Mon Sep 17 00:00:00 2001 From: Raphael Moll Date: Mon, 17 Mar 2014 14:29:38 -0700 Subject: ADT: fix spurious SDK refresh when opening layout editor. SDK Bug: b.android.com/67084 Reason: we recently switched SDK Manager location to a File instead of string. ADT tries to automatically refresh the SDK when it has changed or its path has changed and the comparison between a File-to-path and the original path string from the pref now differed by an ending /. Also renames the legacy method to avoid confusion. Change-Id: Ie4349075528fb552efe10cac4b4462b4bfd3a13f --- .../src/com/android/ide/eclipse/adt/AdtPlugin.java | 31 ++++++++++++++-- .../adt/internal/actions/AddSupportJarAction.java | 6 ++-- .../adt/internal/actions/AvdManagerAction.java | 2 +- .../adt/internal/actions/SdkManagerAction.java | 4 +-- .../eclipse/adt/internal/build/BuildHelper.java | 6 ++-- .../eclipse/adt/internal/editors/Hyperlinks.java | 2 +- .../editors/layout/gle2/RenderService.java | 2 +- .../adt/internal/launch/DeviceChooserDialog.java | 2 +- .../adt/internal/launch/EmulatorConfigTab.java | 2 +- .../LibraryClasspathContainerInitializer.java | 2 +- .../android/ide/eclipse/adt/internal/sdk/Sdk.java | 42 +++++++++++++++++++--- .../wizards/exportgradle/BuildFileCreator.java | 2 +- 12 files changed, 81 insertions(+), 22 deletions(-) (limited to 'eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide') diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java index cbeb274..c53fdb0 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java @@ -1810,6 +1810,12 @@ public class AdtPlugin extends AbstractUIPlugin implements ILogger { * and updates opened projects. *

* The operation is asynchronous and happens in a background eclipse job. + *

+ * This operation is called in multiple places and should be reasonably + * cheap and conservative. The goal is to automatically refresh the SDK + * when it is obvious it has changed so when not sure the code should + * tend to not reload and avoid reloading too often (which is an expensive + * operation that has a lot of user impact.) */ public void refreshSdk() { // SDK can't have changed if we haven't loaded it yet. @@ -1822,8 +1828,29 @@ public class AdtPlugin extends AbstractUIPlugin implements ILogger { @Override protected IStatus run(IProgressMonitor monitor) { // SDK has changed if its location path is different. - boolean changed = sdk.getSdkLocation() == null || - !sdk.getSdkLocation().equals(AdtPrefs.getPrefs().getOsSdkFolder()); + File location = sdk.getSdkFileLocation(); + boolean changed = location == null || !location.isDirectory(); + + if (!changed) { + assert location != null; + File prefLocation = new File(AdtPrefs.getPrefs().getOsSdkFolder()); + changed = !location.equals(prefLocation); + + if (changed) { + // Basic file path comparison indicates they are not the same. + // Let's dig a bit deeper. + try { + location = location.getCanonicalFile(); + prefLocation = prefLocation.getCanonicalFile(); + changed = !location.equals(prefLocation); + } catch (IOException ignore) { + // There's no real reason for the canonicalization to fail + // if the paths map to actual directories. And if they don't + // this should have been caught above. + } + } + } + if (!changed) { // Check whether the target directories has potentially changed. changed = sdk.haveTargetsChanged(); diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java index 37559e9..2eeec5b 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java @@ -172,7 +172,7 @@ public class AddSupportJarAction implements IObjectActionDelegate { return null; } - String sdkLocation = sdk.getSdkLocation(); + String sdkLocation = sdk.getSdkOsLocation(); if (minimumRevision > 0) { File path = getSupportJarFile(); if (path != null) { @@ -229,7 +229,7 @@ public class AddSupportJarAction implements IObjectActionDelegate { public static int getInstalledRevision() { final Sdk sdk = Sdk.getCurrent(); if (sdk != null) { - String sdkLocation = sdk.getSdkLocation(); + String sdkLocation = sdk.getSdkOsLocation(); SdkManager manager = SdkManager.createManager(sdkLocation, NullLogger.getLogger()); Map versions = manager.getExtrasVersions(); Integer version = versions.get(VENDOR_ID + '/' + SUPPORT_ID); @@ -362,7 +362,7 @@ public class AddSupportJarAction implements IObjectActionDelegate { private static File getSupportPackageDir() { final Sdk sdk = Sdk.getCurrent(); if (sdk != null) { - String sdkLocation = sdk.getSdkLocation(); + String sdkLocation = sdk.getSdkOsLocation(); SdkManager manager = SdkManager.createManager(sdkLocation, NullLogger.getLogger()); Map versions = manager.getExtrasVersions(); Integer version = versions.get(VENDOR_ID + '/' + SUPPORT_ID); diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java index 2597090..86f2d3c 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java @@ -57,7 +57,7 @@ public class AvdManagerAction implements IWorkbenchWindowActionDelegate, IObject AvdManagerWindow window = new AvdManagerWindow( AdtPlugin.getShell(), new AdtConsoleSdkLog(), - sdk.getSdkLocation(), + sdk.getSdkOsLocation(), AvdInvocationContext.IDE); window.open(); } else { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java index 9d33230..9a7e7a9 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java @@ -147,7 +147,7 @@ public class SdkManagerAction implements IWorkbenchWindowActionDelegate, IObject // Get the SDK locatiom from the current SDK or as fallback // directly from the ADT preferences. Sdk sdk = Sdk.getCurrent(); - String osSdkLocation = sdk == null ? null : sdk.getSdkLocation(); + String osSdkLocation = sdk == null ? null : sdk.getSdkOsLocation(); if (osSdkLocation == null || !new File(osSdkLocation).isDirectory()) { osSdkLocation = AdtPrefs.getPrefs().getOsSdkFolder(); } @@ -272,7 +272,7 @@ public class SdkManagerAction implements IWorkbenchWindowActionDelegate, IObject // Do not show non-error/warning log in Eclipse. }; }, - sdk.getSdkLocation(), + sdk.getSdkOsLocation(), SdkInvocationContext.IDE); ISdkChangeListener listener = new ISdkChangeListener() { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java index 8e831b7..c981e90 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java @@ -581,9 +581,9 @@ public class BuildHelper { String[] envp = null; Map envMap = new TreeMap(System.getenv()); if (!envMap.containsKey("PROGUARD_HOME")) { //$NON-NLS-1$ - envMap.put("PROGUARD_HOME", Sdk.getCurrent().getSdkLocation() + //$NON-NLS-1$ - SdkConstants.FD_TOOLS + File.separator + - SdkConstants.FD_PROGUARD); + envMap.put("PROGUARD_HOME", Sdk.getCurrent().getSdkOsLocation() + //$NON-NLS-1$ + SdkConstants.FD_TOOLS + File.separator + + SdkConstants.FD_PROGUARD); envp = new String[envMap.size()]; int i = 0; for (Map.Entry entry : envMap.entrySet()) { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java index 1df2e4d..95cec47 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java @@ -445,7 +445,7 @@ public class Hyperlinks { */ private static URL getDocUrl(String relative) { // First try to find locally installed documentation - File sdkLocation = new File(Sdk.getCurrent().getSdkLocation()); + File sdkLocation = new File(Sdk.getCurrent().getSdkOsLocation()); File docs = new File(sdkLocation, FD_DOCS + File.separator + FD_DOCS_REFERENCE); try { if (docs.exists()) { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java index 3da6565..ffcb258 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java @@ -168,7 +168,7 @@ public class RenderService { if (RenderSecurityManager.RESTRICT_READS) { projectPath = AdtUtils.getAbsolutePath(mProject).toFile().getPath(); Sdk sdk = Sdk.getCurrent(); - sdkPath = sdk != null ? sdk.getSdkLocation() : null; + sdkPath = sdk != null ? sdk.getSdkOsLocation() : null; } RenderSecurityManager securityManager = new RenderSecurityManager(sdkPath, projectPath); securityManager.setLogger(AdtPlugin.getDefault()); diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java index 1b850c9..995ccdf 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java @@ -479,7 +479,7 @@ public class DeviceChooserDialog extends Dialog implements IDeviceChangeListener offsetComp.setLayout(layout); mPreferredAvdSelector = new AvdSelector(offsetComp, - mSdk.getSdkLocation(), + mSdk.getSdkOsLocation(), mSdk.getAvdManager(), new NonRunningAvdFilter(), DisplayMode.SIMPLE_SELECTION, diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java index 248cb7a..779dfa1 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java @@ -233,7 +233,7 @@ public class EmulatorConfigTab extends AbstractLaunchConfigurationTab { // displayed to ensure we have the latest one (dialog is reused but SDK could have // been changed in between. mPreferredAvdSelector = new AvdSelector(avdOffsetComp, - Sdk.getCurrent().getSdkLocation(), + Sdk.getCurrent().getSdkOsLocation(), null /* avd manager */, DisplayMode.SIMPLE_CHECK, new AdtConsoleSdkLog()); diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java index 914e1de..525e2fd 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java @@ -326,7 +326,7 @@ public class LibraryClasspathContainerInitializer extends BaseClasspathContainer // annotations support for older version of android if (state.getTarget() != null && state.getTarget().getVersion().getApiLevel() <= 15) { - File annotationsJar = new File(Sdk.getCurrent().getSdkLocation(), + File annotationsJar = new File(Sdk.getCurrent().getSdkOsLocation(), SdkConstants.FD_TOOLS + File.separator + SdkConstants.FD_SUPPORT + File.separator + SdkConstants.FN_ANNOTATIONS_JAR); diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java index 1a0e049..8e40f1c 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java @@ -321,15 +321,47 @@ public final class Sdk { } /** - * Returns the location (OS path) of the current SDK. + * Returns the location of the current SDK as an OS path string. + * Guaranteed to be terminated by a platform-specific path separator. + *

+ * Due to {@link File} canonicalization, this MAY differ from the string used to initialize + * the SDK path. + * + * @return The SDK OS path or null if no SDK is setup. + * @deprecated Consider using {@link #getSdkFileLocation()} instead. + * @see #getSdkFileLocation() */ - public String getSdkLocation() { - return mManager.getLocation(); + @Deprecated + @Nullable + public String getSdkOsLocation() { + String path = mManager == null ? null : mManager.getLocation(); + if (path != null) { + // For backward compatibility make sure it ends with a separator. + // This used to be the case when the SDK Manager was created from a String path + // but now that a File is internally used the trailing dir separator is lost. + if (path.length() > 0 && !path.endsWith(File.separator)) { + path = path + File.separator; + } + } + return path; + } + + /** + * Returns the location of the current SDK as a {@link File} or null. + * + * @return The SDK OS path or null if no SDK is setup. + */ + @Nullable + public File getSdkFileLocation() { + if (mManager == null || mManager.getLocalSdk() == null) { + return null; + } + return mManager.getLocalSdk().getLocation(); } /** * Returns a new {@link SdkManager} that can parse the SDK located - * at the current {@link #getSdkLocation()}. + * at the current {@link #getSdkOsLocation()}. *

* Implementation detail: The {@link Sdk} has its own internal manager with * a custom logger which is not designed to be useful for outsiders. Callers @@ -343,7 +375,7 @@ public final class Sdk { * @return A new {@link SdkManager} parsing the same location. */ public @Nullable SdkManager getNewSdkManager(@NonNull ILogger log) { - return SdkManager.createManager(getSdkLocation(), log); + return SdkManager.createManager(getSdkOsLocation(), log); } /** diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java index 9ee7e31..e749e71 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java @@ -110,7 +110,7 @@ public class BuildFileCreator { @NonNull Shell shell, @NonNull IProgressMonitor pm) { - File gradleLocation = new File(Sdk.getCurrent().getSdkLocation(), GRADLE_WRAPPER_LOCATION); + File gradleLocation = new File(Sdk.getCurrent().getSdkOsLocation(), GRADLE_WRAPPER_LOCATION); SubMonitor localmonitor = null; try { -- cgit v1.1