[gms updater] Fixes and tests to prepare activation

build/android/play_services/update.py

 #!/usr/bin/env python
 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 '''
 Script to help uploading and downloading the Google Play services client
 library to and from a Google Cloud storage.
 '''
 
@@ skipping 19 matching lines @@
 import download_from_google_storage
 import upload_to_google_storage
 
 
 # Directory where the SHA1 files for the zip and the license are stored
 # It should be managed by git to provided information about new versions.
 SHA1_DIRECTORY = os.path.join(constants.DIR_SOURCE_ROOT, 'build', 'android',
                               'play_services')
 
 # Default bucket used for storing the files.
-GMS_CLOUD_STORAGE = 'chrome-sdk-extras'
+GMS_CLOUD_STORAGE = 'chromium-android-tools/play-services'
 
 # Path to the default configuration file. It exposes the currently installed
 # version of the library in a human readable way.
 CONFIG_DEFAULT_PATH = os.path.join(constants.DIR_SOURCE_ROOT, 'build',
                                    'android', 'play_services', 'config.json')
 
 LICENSE_FILE_NAME = 'LICENSE'
 LIBRARY_FILE_NAME = 'google_play_services_library.zip'
 GMS_PACKAGE_ID = 'extra-google-google_play_services'  # used by sdk manager
 
 LICENSE_PATTERN = re.compile(r'^Pkg\.License=(?P<text>.*)$', re.MULTILINE)
 
 
-def Main():
+def main(raw_args):
   parser = argparse.ArgumentParser(
       description=__doc__ + 'Please see the subcommand help for more details.',
       formatter_class=utils.DefaultsRawHelpFormatter)
   subparsers = parser.add_subparsers(title='commands')
 
   # Download arguments
   parser_download = subparsers.add_parser(
       'download',
       help='download the library from the cloud storage',
       description=Download.__doc__,
       formatter_class=utils.DefaultsRawHelpFormatter)
-  parser_download.add_argument('-f', '--force',
-                               action='store_true',
-                               help=('run even if the local version is '
-                                     'already up to date'))
   parser_download.set_defaults(func=Download)
-  AddCommonArguments(parser_download)
+  AddBasicArguments(parser_download)
+  AddBucketArguments(parser_download)
 
   # SDK Update arguments
   parser_sdk = subparsers.add_parser(
       'sdk',
       help='update the local sdk using the Android SDK Manager',
       description=UpdateSdk.__doc__,
       formatter_class=utils.DefaultsRawHelpFormatter)
-  parser_sdk.add_argument('--sdk-root',
-                          help=('base path to the Android SDK tools to use to '
-                                'update the library'),
-                          default=constants.ANDROID_SDK_ROOT)
-  parser_sdk.add_argument('-v', '--verbose',
-                          action='store_true',
-                          help='print debug information')
   parser_sdk.set_defaults(func=UpdateSdk)
+  AddBasicArguments(parser_sdk)
 
   # Upload arguments
   parser_upload = subparsers.add_parser(
       'upload',
       help='upload the library to the cloud storage',
       description=Upload.__doc__,
       formatter_class=utils.DefaultsRawHelpFormatter)
-  parser_upload.add_argument('-f', '--force',
-                             action='store_true',
-                             help=('run even if the checked in version is '
-                                   'already up to date'))
-  parser_upload.add_argument('--sdk-root',
-                             help=('base path to the Android SDK tools to use '
-                                   'to update the library'),
-                             default=constants.ANDROID_SDK_ROOT)
+
   parser_upload.add_argument('--skip-git',
                              action='store_true',
                              help="don't commit the changes at the end")
   parser_upload.set_defaults(func=Upload)
-  AddCommonArguments(parser_upload)
+  AddBasicArguments(parser_upload)
+  AddBucketArguments(parser_upload)
 
-  args = parser.parse_args()
+  args = parser.parse_args(raw_args)

[jbudorick, 2015/11/04 18:46:07]

parse_args() will pick up sys.argv[1:] and parse those by default, so the only
reason to pass something to parse_args is if you're doing something unusual
(which doesn't appear to be the case here).

[dgn, 2015/11/05 12:15:10]

It's the case in update_test.py, where I call main() with command line
arguments:
https://codereview.chromium.org/1418833005/diff/140001/build/android/play_ser...

I could call the underlying functions directly, but I preferred this way since
it tests the argument parsing, is easier to write (avoid having to rebuild the
args object), and I believe is also more useful for demo/doc purposes. Should I
change that?

[jbudorick, 2015/11/05 16:03:02]

oooooh I missed that. This is fine, then.

   if args.verbose:
     logging.basicConfig(level=logging.DEBUG)
-  logging_utils.ColorStreamHandler.MakeDefault()
+  logging_utils.ColorStreamHandler.MakeDefault(not _IsBotEnvironment())
   return args.func(args)
 
 
-def AddCommonArguments(parser):
+def AddBasicArguments(parser):
   '''
   Defines the common arguments on subparser rather than the main one. This
   allows to put arguments after the command: `foo.py upload --debug --force`
   instead of `foo.py --debug upload --force`
   '''
 
-  parser.add_argument('--bucket',
-                      help='name of the bucket where the files are stored',
-                      default=GMS_CLOUD_STORAGE)
-  parser.add_argument('--config',
-                      help='JSON Configuration file',
-                      default=CONFIG_DEFAULT_PATH)
-  parser.add_argument('--dry-run',
-                      action='store_true',
-                      help=('run the script in dry run mode. Files will be '
-                            'copied to a local directory instead of the cloud '
-                            'storage. The bucket name will be as path to that '
-                            'directory relative to the repository root.'))
+  parser.add_argument('--sdk-root',
+                      help=('base path to the Android SDK tools to use when'
+                            'updating the library'),
+                      default=constants.ANDROID_SDK_ROOT)
+
   parser.add_argument('-v', '--verbose',
                       action='store_true',
                       help='print debug information')
 
 
+def AddBucketArguments(parser):
+  parser.add_argument('--bucket',
+                      help='name of the bucket where the files are stored',
+                      default=GMS_CLOUD_STORAGE)
+
+  parser.add_argument('--config',
+                      help='JSON Configuration file',
+                      default=CONFIG_DEFAULT_PATH)
+
+  parser.add_argument('--dry-run',
+                      action='store_true',
+                      help=('run the script in dry run mode. Files will be '
+                            'copied to a local directory instead of the '
+                            'cloud storage. The bucket name will be as path '
+                            'to that directory relative to the repository '
+                            'root.'))
+
+  parser.add_argument('-f', '--force',
+                      action='store_true',
+                      help=('run even if the library is already '
+                            ' up to date'))
+
+
 def Download(args):
   '''
   Downloads the Google Play services client library from a Google Cloud Storage
   bucket and installs it to
   //third_party/android_tools/sdk/extras/google/google_play_services.
 
   A license check will be made, and the user might have to accept the license
   if that has not been done before.
   '''
 
-  paths = _InitPaths(constants.ANDROID_SDK_ROOT)
+  if not os.path.isdir(args.sdk_root):
+    logging.debug('Did not find the android sdk directory at "%s".',
+                  args.sdk_root)
+    if not args.force:
+      logging.info('Skipping, not on an android checkout.')
+      return 0
+
+  paths = _InitPaths(args.sdk_root)
+
+  if os.path.isdir(paths.package) and not os.access(paths.package, os.W_OK):
+    logging.error('Failed updating the Google Play Services library. '
+                  'The location is not writable. Please remove the '
+                  'directory (%s) and try again.', paths.package)
+    return -2
 
   new_lib_zip_sha1 = os.path.join(SHA1_DIRECTORY, LIBRARY_FILE_NAME + '.sha1')
   old_lib_zip_sha1 = os.path.join(paths.package, LIBRARY_FILE_NAME + '.sha1')
 
   logging.debug('Comparing library hashes: %s and %s', new_lib_zip_sha1,
                 old_lib_zip_sha1)
   if utils.FileEquals(new_lib_zip_sha1, old_lib_zip_sha1) and not args.force:
-    logging.debug('The Google Play services library is up to date.')
+    logging.info('Skipping, the Google Play services library is up to date.')
     return 0
 
   config = utils.ConfigParser(args.config)
   bucket_path = _VerifyBucketPathFormat(args.bucket,
                                         config.version_number,
                                         args.dry_run)
 
   tmp_root = tempfile.mkdtemp()
   try:
-    if not os.environ.get('CHROME_HEADLESS'):
-      if not os.path.isdir(paths.package):
-        os.makedirs(paths.package)
+    # setup the destination directory
+    if not os.path.isdir(paths.package):
+      os.makedirs(paths.package)
 
-      # download license file from bucket/{version_number}/license.sha1
-      new_license = os.path.join(tmp_root, LICENSE_FILE_NAME)
-      old_license = os.path.join(paths.package, LICENSE_FILE_NAME)
+    # download license file from bucket/{version_number}/license.sha1
+    new_license = os.path.join(tmp_root, LICENSE_FILE_NAME)
+    old_license = os.path.join(paths.package, LICENSE_FILE_NAME)
 
-      license_sha1 = os.path.join(SHA1_DIRECTORY, LICENSE_FILE_NAME + '.sha1')
-      _DownloadFromBucket(bucket_path, license_sha1, new_license,
-                          args.verbose, args.dry_run)
-      if not _CheckLicenseAgreement(new_license, old_license):
+    license_sha1 = os.path.join(SHA1_DIRECTORY, LICENSE_FILE_NAME + '.sha1')
+    _DownloadFromBucket(bucket_path, license_sha1, new_license,
+                        args.verbose, args.dry_run)
+
+    if (not _IsBotEnvironment() and
+        not _CheckLicenseAgreement(new_license, old_license,
+                                   config.version_number)):
         logging.warning('Your version of the Google Play services library is '
                         'not up to date. You might run into issues building '
                         'or running the app. Please run `%s download` to '
                         'retry downloading it.', __file__)
         return 0
 
     new_lib_zip = os.path.join(tmp_root, LIBRARY_FILE_NAME)
     _DownloadFromBucket(bucket_path, new_lib_zip_sha1, new_lib_zip,
                         args.verbose, args.dry_run)
 
-    # We remove only the library itself. Users having a SDK manager installed
+    # We remove the library itself. Users having a SDK manager installed
     # library before will keep the documentation and samples from it.
     shutil.rmtree(paths.lib, ignore_errors=True)
     os.makedirs(paths.lib)

[jbudorick, 2015/11/04 18:48:10]

In light of the root ownership issue, we might want to give users some hints
here if one of these two calls fails.

[dgn, 2015/11/05 12:15:10]

Done. I also wrapped the calls below, as the license and sha1 files don't go
under that path.

 
+    # We also remove source.properties so that the SDK manager doesn't fail
+    # recognizing that it didn't install the local version.
+    source_prop = os.path.join(paths.package, 'source.properties')
+    if os.path.isfile(source_prop):
+      os.remove(source_prop)
+
     logging.debug('Extracting the library to %s', paths.lib)
     with zipfile.ZipFile(new_lib_zip, "r") as new_lib_zip_file:
       new_lib_zip_file.extractall(paths.lib)
 
     logging.debug('Copying %s to %s', new_license, old_license)
     shutil.copy(new_license, old_license)
 
     logging.debug('Copying %s to %s', new_lib_zip_sha1, old_lib_zip_sha1)
     shutil.copy(new_lib_zip_sha1, old_lib_zip_sha1)
 
+    logging.info('Update complete.')
   finally:
     shutil.rmtree(tmp_root)
 
   return 0
 
 
 def UpdateSdk(args):
   '''
   Uses the Android SDK Manager to update or download the local Google Play
   services library. Its usual installation path is
@@ skipping 160 matching lines @@
 
   match = LICENSE_PATTERN.search(prop_file_content)
   if not match:
     raise AttributeError('The license was not found in ' +
                          os.path.abspath(prop_file_path))
 
   with open(license_path, 'w') as license_file:
     license_file.write(match.group('text'))
 
 
-def _CheckLicenseAgreement(expected_license_path, actual_license_path):
+def _CheckLicenseAgreement(expected_license_path, actual_license_path,
+                           version_number):
   '''
   Checks that the new license is the one already accepted by the user. If it
   isn't, it prompts the user to accept it. Returns whether the expected license
   has been accepted.
   '''
 
   if utils.FileEquals(expected_license_path, actual_license_path):
     return True
 
   with open(expected_license_path) as license_file:
+    # Uses plain print rather than logging to make sure this is not formatted
+    # by the logger.
+    print ('Updating the Google Play services SDK to '
+           'version %d.' % version_number)
+
     # The output is buffered when running as part of gclient hooks. We split
     # the text here and flush is explicitly to avoid having part of it dropped
     # out.
     # Note: text contains *escaped* new lines, so we split by '\\n', not '\n'.
     for license_part in license_file.read().split('\\n'):
-      # Uses plain print rather than logging to make sure this is not formatted
-      # by the logger.
       print license_part
       sys.stdout.flush()
 
   # Need to put the prompt on a separate line otherwise the gclient hook buffer
   # only prints it after we received an input.
   print 'Do you accept the license? [y/n]: '
   sys.stdout.flush()
   return raw_input('> ') in ('Y', 'y')
 
 
+def _IsBotEnvironment():
+  return bool(os.environ.get('CHROME_HEADLESS'))
+
+
 def _VerifyBucketPathFormat(bucket_name, version_number, is_dry_run):
   '''
   Formats and checks the download/upload path depending on whether we are
   running in dry run mode or not. Returns a supposedly safe path to use with
   Gsutil.
   '''
 
   if is_dry_run:
     bucket_path = os.path.abspath(os.path.join(bucket_name,
                                                str(version_number)))
@@ skipping 25 matching lines @@
   def call(self, *args):
     logging.debug('Calling command "%s"', str(args))
     return cmd_helper.GetCmdStatusOutputAndError(args)
 
   def check_call(self, *args):
     logging.debug('Calling command "%s"', str(args))
     return cmd_helper.GetCmdStatusOutputAndError(args)
 
 
 if __name__ == '__main__':
-  sys.exit(Main())
+  sys.exit(main(sys.argv[1:]))