diff options
4 files changed, 120 insertions, 47 deletions
diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogic.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogic.java index 871bf1b..bc3a3f4 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogic.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogic.java @@ -23,11 +23,13 @@ import com.android.sdklib.internal.repository.PlatformPackage; import com.android.sdklib.internal.repository.PlatformToolPackage; import com.android.sdklib.internal.repository.SdkSource; import com.android.sdklib.internal.repository.ToolPackage; +import com.android.sdklib.internal.repository.Package.UpdateInfo; import com.android.sdkuilib.internal.repository.UpdaterData; import com.android.sdkuilib.internal.repository.sdkman2.PkgItem.PkgState; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -312,27 +314,34 @@ class PackagesDiffLogic { assert newPackages.size() == packages.length; - // Upgrade 'new' items to 'installed' for any local package we already know about + // Upgrade NEW items to INSTALLED for any local package we already know about. + // We can't just change the state of the NEW item to INSTALLED, we also need its + // installed package/archive information and so we swap them in-place in the items list. + for (PkgCategory cat : op.getCategories()) { List<PkgItem> items = cat.getItems(); for (int i = 0; i < items.size(); i++) { PkgItem item = items.get(i); - if (item.hasUpdatePkg() && newPackages.contains(item.getUpdatePkg())) { - // This item has an update package that is now installed. - PkgItem installed = new PkgItem(item.getUpdatePkg(), PkgState.INSTALLED); - unusedPackages.remove(item.getUpdatePkg()); - item.removeUpdate(); - items.add(installed); - cat.setUnused(false); - hasChanged = true; + if (item.hasUpdatePkg()) { + Package newPkg = setContainsLocalPackage(newPackages, item.getUpdatePkg()); + if (newPkg != null) { + // This item has an update package that is now installed. + PkgItem installed = new PkgItem(newPkg, PkgState.INSTALLED); + removePackageFromSet(unusedPackages, newPkg); + item.removeUpdate(); + items.add(installed); + cat.setUnused(false); + hasChanged = true; + } } - if (newPackages.contains(item.getMainPackage())) { - unusedPackages.remove(item.getMainPackage()); + Package newPkg = setContainsLocalPackage(newPackages, item.getMainPackage()); + if (newPkg != null) { + removePackageFromSet(unusedPackages, newPkg); if (item.getState() == PkgState.NEW) { // This item has a main package that is now installed. - item.setState(PkgState.INSTALLED); + replace(items, i, new PkgItem(newPkg, PkgState.INSTALLED)); cat.setUnused(false); hasChanged = true; } @@ -340,13 +349,18 @@ class PackagesDiffLogic { } } - // Downgrade 'installed' items to 'new' if their package isn't listed anymore + // Downgrade INSTALLED items to NEW if their package isn't listed anymore for (PkgCategory cat : op.getCategories()) { - for (PkgItem item : cat.getItems()) { - if (item.getState() == PkgState.INSTALLED && - !newPackages.contains(item.getMainPackage())) { - item.setState(PkgState.NEW); - hasChanged = true; + List<PkgItem> items = cat.getItems(); + for (int i = 0; i < items.size(); i++) { + PkgItem item = items.get(i); + + if (item.getState() == PkgState.INSTALLED) { + Package newPkg = setContainsLocalPackage(newPackages, item.getMainPackage()); + if (newPkg == null) { + replace(items, i, new PkgItem(item.getMainPackage(), PkgState.NEW)); + hasChanged = true; + } } } } @@ -375,8 +389,72 @@ class PackagesDiffLogic { return hasChanged; } - /** Process all remote packages. Returns true if something changed. - * @param op */ + /** + * Replaces the item at {@code index} in {@code list} with the new {@code obj} element. + * This uses {@link ArrayList#set(int, Object)} if possible, remove+add otherwise. + * + * @return The old item at the same index position. + * @throws IndexOutOfBoundsException if index out of range (index < 0 || index >= size()). + */ + private <T> T replace(List<T> list, int index, T obj) { + if (list instanceof ArrayList<?>) { + return ((ArrayList<T>) list).set(index, obj); + } else { + T old = list.remove(index); + list.add(index, obj); + return old; + } + } + + /** + * Checks whether the {@code newPackages} set contains a package that is the + * same as {@code pkgToFind}. + * This is based on Package being the same from an install point of view rather than + * pure object equality. + * @return The matching package from the {@code newPackages} set or null if not found. + */ + private Package setContainsLocalPackage(Collection<Package> newPackages, Package pkgToFind) { + // Most of the time, local packages don't have the exact same hash code + // as new ones since the objects are similar but not exactly the same, + // for example their installed OS path cannot match (by definition) so + // their hash code do not match when used with Set.contains(). + + for (Package newPkg : newPackages) { + // Two packages are the same if they are compatible types, + // do not update each other and have the same revision number. + if (pkgToFind.canBeUpdatedBy(newPkg) == UpdateInfo.NOT_UPDATE && + pkgToFind.getRevision() == newPkg.getRevision()) { + return newPkg; + } + } + + return null; + } + + /** + * Removes the given package from the set. + * This is based on Package being the same from an install point of view rather than + * pure object equality. + */ + private void removePackageFromSet(Collection<Package> packages, Package pkgToRemove) { + // First try to remove the package based on its hash code. This can fail + // for a variety of reasons, as explained in setContainsLocalPackage(). + if (packages.remove(pkgToRemove)) { + return; + } + + for (Package pkg : packages) { + // Two packages are the same if they are compatible types, + // or not updates of each other and have the same revision number. + if (pkgToRemove.canBeUpdatedBy(pkg) == UpdateInfo.NOT_UPDATE && + pkgToRemove.getRevision() == pkg.getRevision()) { + packages.remove(pkg); + return; + } + } + } + + /** Process all remote packages. Returns true if something changed. */ private boolean processSource(UpdateOp op, SdkSource source, Package[] packages) { boolean hasChanged = false; // Note: unusedPackages must respect the original packages order. It can't be a set. @@ -396,22 +474,26 @@ class PackagesDiffLogic { if (!(itemSource == source || (source != null && source.equals(itemSource)))) { continue; } + // Installed items have been dealt with the local source, // so only change new items here - if (item.getState() == PkgState.NEW && - !newPackages.contains(item.getMainPackage())) { - // This package is no longer part of the source. - items.remove(i--); - hasChanged = true; - continue; + if (item.getState() == PkgState.NEW) { + Package newPkg = setContainsLocalPackage(newPackages, item.getMainPackage()); + if (newPkg == null) { + // This package is no longer part of the source. + items.remove(i--); + hasChanged = true; + continue; + } } cat.setUnused(false); - unusedPackages.remove(item.getMainPackage()); + removePackageFromSet(unusedPackages, item.getMainPackage()); if (item.hasUpdatePkg()) { - if (newPackages.contains(item.getUpdatePkg())) { - unusedPackages.remove(item.getUpdatePkg()); + Package newPkg = setContainsLocalPackage(newPackages, item.getUpdatePkg()); + if (newPkg != null) { + removePackageFromSet(unusedPackages, newPkg); } else { // This update is no longer part of the source item.removeUpdate(); diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesPage.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesPage.java index f703fcd..d5a6068 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesPage.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackagesPage.java @@ -868,13 +868,12 @@ public class PackagesPage extends UpdaterPage /** * Indicate an install/delete operation is pending. - * This disable the install/delete buttons. - * Use {@link #endOperationPending()} to revert. + * This disables the install/delete buttons. + * Use {@link #endOperationPending()} to revert, typically in a {@code try..finally} block. */ private void beginOperationPending() { mOperationPending = true; - mButtonInstall.setEnabled(false); - mButtonDelete.setEnabled(false); + updateButtonsState(); } private void endOperationPending() { @@ -883,10 +882,6 @@ public class PackagesPage extends UpdaterPage } private void updateButtonsState() { - if (mOperationPending) { - return; - } - boolean canInstall = false; int numPackages = 0; @@ -929,7 +924,7 @@ public class PackagesPage extends UpdaterPage } } - mButtonInstall.setEnabled(canInstall); + mButtonInstall.setEnabled(canInstall && !mOperationPending); mButtonInstall.setText( numPackages == 0 ? "Install packages..." : numPackages == 1 ? "Install 1 package..." : @@ -952,7 +947,7 @@ public class PackagesPage extends UpdaterPage } } - mButtonDelete.setEnabled(canDelete); + mButtonDelete.setEnabled(canDelete && !mOperationPending); mButtonDelete.setText( numPackages == 0 ? "Delete packages..." : numPackages == 1 ? "Delete 1 package..." : diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PkgItem.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PkgItem.java index 5bc0b46..2675d8d 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PkgItem.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PkgItem.java @@ -30,7 +30,7 @@ import com.android.sdklib.internal.repository.Package.UpdateInfo; * The state or update package can change later. */ public class PkgItem implements Comparable<PkgItem> { - private PkgState mState; + private final PkgState mState; private final Package mMainPkg; private Package mUpdatePkg; private boolean mChecked; @@ -102,10 +102,6 @@ public class PkgItem implements Comparable<PkgItem> { return mState; } - public void setState(PkgState state) { - mState = state; - } - public SdkSource getSource() { return mMainPkg.getParentSource(); } diff --git a/sdkmanager/libs/sdkuilib/tests/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogicTest.java b/sdkmanager/libs/sdkuilib/tests/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogicTest.java index 923a5fe..bbef6d3 100755 --- a/sdkmanager/libs/sdkuilib/tests/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogicTest.java +++ b/sdkmanager/libs/sdkuilib/tests/com/android/sdkuilib/internal/repository/sdkman2/PackagesDiffLogicTest.java @@ -182,9 +182,9 @@ public class PackagesDiffLogicTest extends TestCase { "-- <INSTALLED, pkg:MockEmptyPackage 'type1' rev=1, updated by:MockEmptyPackage 'type1' rev=2>\n", getTree(m, true /*displaySortByApi*/)); - // Now simulate a reload that clears the package list and create similar + // Now simulate a reload that clears the package list and creates similar // objects but not the same references. The only difference is that updateXyz - // returns false since they don't change anything. + // returns false since nothing changes. m.updateStart(); // First insert local packages @@ -285,7 +285,7 @@ public class PackagesDiffLogicTest extends TestCase { m.updateStart(); // No local packages assertTrue(m.updateSourcePackages(true /*sortByApi*/, null /*locals*/, new Package[0])); - assertTrue(m.updateSourcePackages(true /*sortByApi*/, src1, new Package[] { + assertFalse(m.updateSourcePackages(true /*sortByApi*/, src1, new Package[] { new MockEmptyPackage(src1, "type1", 1) })); |