[NaCl SDK] Prune old entries local naclsdk_manifest2.json

This prevents the local manifest from growing unboundedly
when the user removes SDKs manually rather than via the
uninstall command.

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk

Review URL: https://codereview.chromium.org/1419263002

Cr-Commit-Position: refs/heads/master@{#358898}

3 files changed, 37 insertions, 16 deletions

diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
index f61a9d2..9d69ce1 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
@@ -37,8 +37,8 @@ REVISION = '{REVISION}'
 GSTORE_URL = 'https://storage.googleapis.com/nativeclient-mirror'
 CONFIG_FILENAME = 'naclsdk_config.json'
 MANIFEST_FILENAME = 'naclsdk_manifest2.json'
-DEFAULT_SDK_ROOT = os.path.abspath(PARENT_DIR)
-USER_DATA_DIR = os.path.join(DEFAULT_SDK_ROOT, 'sdk_cache')
+SDK_ROOT = PARENT_DIR
+USER_DATA_DIR = os.path.join(SDK_ROOT, 'sdk_cache')
 
 
 def usage(more):
@@ -115,6 +115,7 @@ def LoadLocalManifest(raise_on_error=False):
       raise
     else:
       logging.warn(str(e))
+
   return manifest
 
 
@@ -166,6 +167,25 @@ def LoadCombinedRemoteManifest(default_manifest_url, cfg):
   return manifest
 
 
+def PruneLocalManifest(local_manifest, remote_manifest):
+  """Remove SDKs from the local manifest that don't exist remotely and
+  are not installed locally.
+
+  Without this the local manifest will grown unboundedly.
+  """
+  local_only_bundles = set([b.name for b in local_manifest.GetBundles()])
+  local_only_bundles -= set([b.name for b in remote_manifest.GetBundles()])
+  dirty = False
+  for bundle in local_only_bundles:
+    root = os.path.join(SDK_ROOT, bundle)
+    if not os.path.exists(root):
+      local_manifest.RemoveBundle(bundle)
+      dirty = True
+
+  if dirty:
+    WriteLocalManifest(local_manifest)
+
+
 # Commands #####################################################################
 
 
@@ -188,6 +208,7 @@ def CMDlist(parser, args):
   local_manifest = LoadLocalManifest()
   cfg = LoadConfig()
   remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
+  PruneLocalManifest(local_manifest, remote_manifest)
   command.list.List(remote_manifest, local_manifest, options.revision)
   return 0
 
@@ -206,9 +227,10 @@ def CMDupdate(parser, args):
   cfg = LoadConfig()
   remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
 
+  PruneLocalManifest(local_manifest, remote_manifest)
+
   try:
-    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR,
-                                                 DEFAULT_SDK_ROOT, cfg)
+    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR, SDK_ROOT, cfg)
     command.update.Update(delegate, remote_manifest, local_manifest,
                           options.bundles, options.force)
   finally:
@@ -237,7 +259,7 @@ def CMDuninstall(parser, args):
   parser.add_argument('bundles', nargs='+', help='bundles to uninstall')
   options = parser.parse_args(args)
   local_manifest = LoadLocalManifest()
-  command.uninstall.Uninstall(DEFAULT_SDK_ROOT, local_manifest, options.bundles)
+  command.uninstall.Uninstall(SDK_ROOT, local_manifest, options.bundles)
   WriteLocalManifest(local_manifest)
   return 0
 
@@ -255,8 +277,7 @@ def CMDreinstall(parser, args):
 
   cfg = LoadConfig()
   try:
-    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR,
-                                                 DEFAULT_SDK_ROOT, cfg)
+    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR, SDK_ROOT, cfg)
     command.update.Reinstall(delegate, local_manifest, options.bundles)
   finally:
     # Always write out the local manifest, we may have successfully updated one
@@ -342,8 +363,7 @@ def UpdateSDKTools(options, args):
   remote_manifest = LoadCombinedRemoteManifest(options.manifest_url, cfg)
 
   try:
-    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR,
-                                                 DEFAULT_SDK_ROOT, cfg)
+    delegate = command.update.RealUpdateDelegate(USER_DATA_DIR, SDK_ROOT, cfg)
     command.update.UpdateBundleIfNeeded(
         delegate,
         remote_manifest,
diff --git a/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py b/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
index 7dd9e61..d118186 100755
--- a/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
+++ b/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
@@ -156,12 +156,13 @@ class TestCommands(SdkToolsTestCase):
     self._WriteManifest()
     p23bundle = self._AddDummyBundle(self.manifest, 'pepper_23')
     self._WriteCacheManifest(self.manifest)
+    # Create pepper_23 directory so that manifest entry doesn't get purged
+    os.mkdir(os.path.join(self.nacl_sdk_base, 'pepper_23'))
     output = self._Run(['list', '-r'])
     message = 'Bundles installed locally that are not available remotely:'
-    message_loc = output.find(message)
-    self.assertNotEqual(message_loc, -1)
+    self.assertIn(message, output)
     # Make sure pepper_23 is listed after the message above.
-    self.assertTrue('pepper_23' in output[message_loc:])
+    self.assertTrue('pepper_23' in output[output.find(message):])
 
   def testSources(self):
     """The sources command should allow adding/listing/removing of sources.
diff --git a/native_client_sdk/src/build_tools/tests/sdktools_test.py b/native_client_sdk/src/build_tools/tests/sdktools_test.py
index 71891f6..66c041b 100755
--- a/native_client_sdk/src/build_tools/tests/sdktools_test.py
+++ b/native_client_sdk/src/build_tools/tests/sdktools_test.py
@@ -39,7 +39,8 @@ class SdkToolsTestCase(unittest.TestCase):
 
   def SetupWithBaseDirPrefix(self, basedir_prefix, tmpdir=None):
     self.basedir = tempfile.mkdtemp(prefix=basedir_prefix, dir=tmpdir)
-    self.cache_dir = os.path.join(self.basedir, 'nacl_sdk', 'sdk_cache')
+    self.nacl_sdk_base = os.path.join(self.basedir, 'nacl_sdk')
+    self.cache_dir = os.path.join(self.nacl_sdk_base, 'sdk_cache')
     # We have to make sure that we build our updaters with a version that is at
     # least as large as the version in the sdk_tools bundle. If not, update
     # tests may fail because the "current" version (according to the sdk_cache)
@@ -121,7 +122,7 @@ class SdkToolsTestCase(unittest.TestCase):
     return archive
 
   def _Run(self, args, expect_error=False):
-    naclsdk_shell_script = os.path.join(self.basedir, 'nacl_sdk', 'naclsdk')
+    naclsdk_shell_script = os.path.join(self.nacl_sdk_base, 'naclsdk')
     if getos.GetPlatform() == 'win':
       naclsdk_shell_script += '.bat'
     cmd = [naclsdk_shell_script]
@@ -214,8 +215,7 @@ class TestAutoUpdateSdkTools(SdkToolsTestCase):
     self.sdk_tools_bundle.revision = new_revision
     self._WriteManifest()
 
-    sdk_tools_update_dir = os.path.join(self.basedir, 'nacl_sdk',
-        'sdk_tools_update')
+    sdk_tools_update_dir = os.path.join(self.nacl_sdk_base, 'sdk_tools_update')
     self.assertFalse(os.path.exists(sdk_tools_update_dir))
     stdout = self._Run(['update', 'sdk_tools'])
     self.assertTrue(stdout.find('Ignoring manual update request.') != -1)