[NaCl SDK] Some fixes to update_nacl_manifest.py.

Also modified naclsdk_manifest2.json to allow update_nacl_manifest to upload
pepper_20 bundles.

BUG=none
TEST=tests/test_update_manifest.py

Review URL: https://chromiumcodereview.appspot.com/10414011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138169 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 51 insertions, 44 deletions

diff --git a/native_client_sdk/src/build_tools/build_sdk.py b/native_client_sdk/src/build_tools/build_sdk.py
index 7efb156..0a215eb 100755
--- a/native_client_sdk/src/build_tools/build_sdk.py
+++ b/native_client_sdk/src/build_tools/build_sdk.py
@@ -693,9 +693,9 @@ def main(args):
                         'checksum': {'sha1': archive_sha1}})
       bundle = manifest_util.Bundle('pepper_' + pepper_ver)
       bundle.CopyFrom({
-          'revision': clnumber,
+          'revision': int(clnumber),
           'repath': 'pepper_' + pepper_ver,
-          'version': pepper_ver,
+          'version': int(pepper_ver),
           'description': 'Chrome %s bundle, revision %s' % (
               pepper_ver, clnumber),
           'stability': 'dev',
diff --git a/native_client_sdk/src/build_tools/json/naclsdk_manifest2.json b/native_client_sdk/src/build_tools/json/naclsdk_manifest2.json
index cecd7b9..2c0db96 100644
--- a/native_client_sdk/src/build_tools/json/naclsdk_manifest2.json
+++ b/native_client_sdk/src/build_tools/json/naclsdk_manifest2.json
@@ -198,30 +198,6 @@
       "version": 20,
       "repath": "pepper_20",
       "archives": [
-        {
-          "url": "https://commondatastorage.googleapis.com/nativeclient-mirror/nacl/nacl_sdk/trunk.134804/naclsdk_mac.bz2",
-          "checksum": {
-            "sha1": "d134ebad9887fe8f163f7599f136aa21cb5779a7"
-          },
-          "host_os": "mac",
-          "size": 123117593
-        },
-        {
-          "url": "https://commondatastorage.googleapis.com/nativeclient-mirror/nacl/nacl_sdk/trunk.134804/naclsdk_linux.bz2",
-          "checksum": {
-            "sha1": "f30ae1fe30208e52da0e26d262d42190b6575011"
-          },
-          "host_os": "linux",
-          "size": 127113829
-        },
-        {
-          "url": "https://commondatastorage.googleapis.com/nativeclient-mirror/nacl/nacl_sdk/trunk.134804/naclsdk_win.bz2",
-          "checksum": {
-            "sha1": "a4457505fe1c8ee12aaaa1e0c85eae6edf74656d"
-          },
-          "host_os": "win",
-          "size": 119404351
-        }
       ],
       "revision": 134804
     }
diff --git a/native_client_sdk/src/build_tools/tests/test_update_manifest.py b/native_client_sdk/src/build_tools/tests/test_update_manifest.py
index 3bcfce5..33e0f11 100755
--- a/native_client_sdk/src/build_tools/tests/test_update_manifest.py
+++ b/native_client_sdk/src/build_tools/tests/test_update_manifest.py
@@ -38,6 +38,10 @@ def GetArchiveUrl(host_os, version):
   return urlparse.urljoin(HTTPS_BASE_URL, posixpath.join(version, basename))
 
 
+def MakeGsUrl(rel_path):
+  return update_nacl_manifest.GS_BUCKET_PATH + rel_path
+
+
 def GetPathFromGsUrl(url):
   assert url.startswith(update_nacl_manifest.GS_BUCKET_PATH)
   return url[len(update_nacl_manifest.GS_BUCKET_PATH):]
@@ -77,10 +81,10 @@ def MakeNonPepperBundle(name, with_archives=False):
 
 
 def MakeBundle(major_version, revision, version=None, host_oses=None):
-  assert version is None or version.split('.')[0] == major_version
-  bundle_name = 'pepper_' + major_version
+  assert version is None or version.split('.')[0] == str(major_version)
+  bundle_name = 'pepper_' + str(major_version)
   bundle = manifest_util.Bundle(bundle_name)
-  bundle.version = int(major_version)
+  bundle.version = major_version
   bundle.revision = revision
   bundle.description = 'Chrome %s bundle, revision %s' % (major_version,
       revision)
@@ -156,7 +160,7 @@ class TestDelegate(update_nacl_manifest.Delegate):
     result = []
     for filename, _ in self.files.iteritems():
       if filename.startswith(path):
-        result.append(filename)
+        result.append(MakeGsUrl(filename))
     return result
 
   def GsUtil_cat(self, url):
@@ -186,12 +190,12 @@ V18_0_1025_175 = '18.0.1025.175'
 V18_0_1025_184 = '18.0.1025.184'
 V19_0_1084_41 = '19.0.1084.41'
 V19_0_1084_67 = '19.0.1084.67'
-B18_0_1025_163_R1_MLW = MakeBundle('18', '1', V18_0_1025_163, OS_MLW)
-B18_0_1025_184_R1_MLW = MakeBundle('18', '1', V18_0_1025_184, OS_MLW)
-B18_R1_NONE = MakeBundle('18', '1')
-B19_0_1084_41_R1_MLW = MakeBundle('19', '1', V19_0_1084_41, OS_MLW)
-B19_0_1084_67_R1_MLW = MakeBundle('19', '1', V19_0_1084_67, OS_MLW)
-B19_R1_NONE = MakeBundle('19', '1')
+B18_0_1025_163_R1_MLW = MakeBundle(18, 1, V18_0_1025_163, OS_MLW)
+B18_0_1025_184_R1_MLW = MakeBundle(18, 1, V18_0_1025_184, OS_MLW)
+B18_R1_NONE = MakeBundle(18, '1')
+B19_0_1084_41_R1_MLW = MakeBundle(19, 1, V19_0_1084_41, OS_MLW)
+B19_0_1084_67_R1_MLW = MakeBundle(19, 1, V19_0_1084_67, OS_MLW)
+B19_R1_NONE = MakeBundle(19, '1')
 NON_PEPPER_BUNDLE_NOARCHIVES = MakeNonPepperBundle('foo')
 NON_PEPPER_BUNDLE_ARCHIVES = MakeNonPepperBundle('bar', with_archives=True)
 
@@ -355,6 +359,22 @@ class TestUpdateManifest(unittest.TestCase):
     self._AssertUploadedManifestHasBundle(B18_0_1025_163_R1_MLW, BETA)
     self.assertEqual(len(self.uploaded_manifest.GetBundles()), 1)
 
+  def testSnippetWithStringRevisionAndVersion(self):
+    # This test exists because some manifest snippets were uploaded with
+    # strings for their revisions and versions. I want to make sure the
+    # resulting manifest is still consistent with the old format.
+    self.manifest = MakeManifest(B18_R1_NONE)
+    self.history.Add(OS_MLW, BETA, V18_0_1025_163)
+    bundle_string_revision = MakeBundle('18', '1234', V18_0_1025_163, OS_MLW)
+    self.files.Add(bundle_string_revision)
+    self._MakeDelegate()
+    self._Run(OS_MLW)
+    self._ReadUploadedManifest()
+    uploaded_bundle = self.uploaded_manifest.GetBundle(
+        bundle_string_revision.name)
+    self.assertEqual(uploaded_bundle.revision, 1234)
+    self.assertEqual(uploaded_bundle.version, 18)
+
 
 def main():
   suite = unittest.defaultTestLoader.loadTestsFromModule(sys.modules[__name__])
diff --git a/native_client_sdk/src/build_tools/update_nacl_manifest.py b/native_client_sdk/src/build_tools/update_nacl_manifest.py
index b72ef88..e3d89bd 100755
--- a/native_client_sdk/src/build_tools/update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/update_nacl_manifest.py
@@ -118,14 +118,18 @@ class Delegate(object):
     """Runs gsutil ls |url|
 
     Args:
-      url: The commondatastorage url to list."""
+      url: The commondatastorage url to list.
+    Returns:
+      A list of URLs, all with the gs:// schema."""
     raise NotImplementedError()
 
   def GsUtil_cat(self, url):
     """Runs gsutil cat |url|
 
     Args:
-      url: The commondatastorage url to read from."""
+      url: The commondatastorage url to read from.
+    Returns:
+      A string with the contents of the file at |url|."""
     raise NotImplementedError()
 
   def GsUtil_cp(self, src, dest, stdin=None):
@@ -176,7 +180,8 @@ class RealDelegate(Delegate):
 
   def GsUtil_cp(self, src, dest, stdin=None):
     """See Delegate.GsUtil_cp"""
-    return self._RunGsUtil(stdin, 'cp', '-a', 'public-read', src, dest)
+    # -p ensures we keep permissions when copying "in-the-cloud".
+    return self._RunGsUtil(stdin, 'cp', '-p', '-a', 'public-read', src, dest)
 
   def Print(self, *args):
     sys.stdout.write(' '.join(map(str, args)) + '\n')
@@ -306,10 +311,12 @@ class VersionFinder(object):
       version_string: The version to find archives for. (e.g. "18.0.1025.164")
     Returns:
       A list of strings, each of which is a platform-specific archive URL. (e.g.
-      "https://commondatastorage.googleapis.com/nativeclient_mirror/nacl/"
-      "nacl_sdk/18.0.1025.164/naclsdk_linux.bz2".
-    """
+      "gs://nativeclient_mirror/nacl/nacl_sdk/18.0.1025.164/naclsdk_linux.bz2").
+
+      All returned URLs will use the gs:// schema."""
     files = self.delegate.GsUtil_ls(GS_BUCKET_PATH + version_string)
+    assert all(file.startswith('gs://') for file in files)
+
     archives = [file for file in files if not file.endswith('.json')]
     manifests = [file for file in files if file.endswith('.json')]
 
@@ -356,14 +363,18 @@ class Updater(object):
        Bundle.
 
     Args:
-      archive: The URL of a platform-specific archive.
+      archive: A full URL of a platform-specific archive, using the gs schema.
     Returns:
       An object of type manifest_util.Bundle, read from a JSON file storing
       metadata for this archive.
     """
-    stdout = self.delegate.GsUtil_cat(GS_BUCKET_PATH + archive + '.json')
+    stdout = self.delegate.GsUtil_cat(archive + '.json')
     bundle = manifest_util.Bundle('')
     bundle.LoadDataFromString(stdout)
+    # Some snippets were uploaded with revisions and versions as strings. Fix
+    # those here.
+    bundle.revision = int(bundle.revision)
+    bundle.version = int(bundle.version)
     return bundle
 
   def _UploadManifest(self, manifest):