Keystore no longer a target, apks signed with SignApkTask.

This changes the Keystore from being a first-class target into
a regular object. Since pants does not actually build a keystore,
this is a better abstraction.

Instead of passing Keystore definitions in BUILD files, now there
is a keystore_config.ini which is passed as an option. Pants will
output one signed apk for every keystore definition in that config file.

The config file is in .ini syntax, to lower the barrier to entry
for casual or new users. Upon first build of an android target,
a default config file is put in ~/.pants.d/android. This is done
by the new AndroidConfigUtils class and points to a well-known
keystore installed by the Android SDK.

This change removes the build_type abstraction from tasks and targets.
Pants now figures out the build type from context. If pointed at a
release keystore definition, pants will be able to create a release
build with no further info.

Design Doc:
https://docs.google.com/document/d/1bhq4L4GW0Q-KMtIBy_AkEg4oWIX6pC5K-XUdMf2fVcg

Tests and integration tests for all new classes are included.

Fixes pantsbuild#490

Testing Done:
PANTS_DEV=1 ./pants test tests/python/pants_test
               SUCCESS

Travis passed: https://travis-ci.org/pantsbuild/pants/builds/49213471

Reviewed at https://rbcommons.com/s/twitter/r/1690/

.gitignore

@@ -22,9 +22,6 @@
 .factorypath
 .python
 .pydevproject
-3rdparty/android/authentication/keys/release/*
-# allow only the readme under 'release'
-!3rdparty/android/authentication/keys/release/readme.txt
 build/
 codegen/classes/
 htmlcov/

examples/src/android/example/BUILD

@@ -8,7 +8,6 @@ android_binary(
   manifest='AndroidManifest.xml',
   dependencies = [
     ':resources',
-    '3rdparty/android/authentication/keys/debug:debug'
   ],
 )
 

pants.ini

@@ -185,3 +185,9 @@ packages: [
     "internal_backend.repositories",
     "internal_backend.sitegen"
   ]
+
+
+[android-keystore-location]
+# Default to debug keystore installed with SDK.
+# You can change this to point to a config.ini holding the definition of your keys.
+keystore_config_location: %(pants_bootstrapdir)s/android/keystore/default_config.ini

src/python/pants/backend/android/BUILD

@@ -14,9 +14,22 @@ python_library(
 )
 
 python_library(
-  name = 'android_distribution',
-  sources = globs('distribution/*.py'),
+  name = 'android_config_util',
+  sources =['android_config_util.py'],
   dependencies = [
-    '3rdparty/python/twitter/commons:twitter.common.log',
+    'src/python/pants/util:dirutil',
   ]
 )
+
+python_library(
+  name = 'android_distribution',
+  sources = ['distribution/android_distribution.py'],
+)
+
+python_library(
+  name = 'keystore_resolver',
+  sources = ['keystore/keystore_resolver.py'],
+  dependencies = [
+    'src/python/pants/base:config',
+  ]
+)

src/python/pants/backend/android/android_config_util.py

@@ -0,0 +1,58 @@
+# coding=utf-8
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
+                        print_function, unicode_literals)
+
+import os
+import textwrap
+
+from pants.util.dirutil import safe_open
+
+
+class AndroidConfigUtil(object):
+  """Utility methods for Pants-specific Android configurations."""
+
+  class AndroidConfigError(Exception):
+    """Indicate an error reading Android config files."""
+
+  @classmethod
+  def setup_keystore_config(cls, config):
+    """Create a config file for Android keystores and seed with the default keystore.
+
+    :param string config: Desired location for the new .ini config file.
+    """
+
+    # Unless the config file in ~/.pants.d/android is deleted, this method should only run once,
+    # the first time an android_target is built. What I don't like about this is that the
+    # example config is only generated after the first time an android_target is built,
+    # instead of being available beforehand.
+
+    ini = textwrap.dedent(
+      """
+      # Android Keystore definitions. Follow this format when adding a keystore definition.
+      # Each keystore has an arbitrary name and is required to have all five fields below.
+
+      # These definitions can go anywhere in your file system, passed to pants as the option
+      # '--keystore-config-location' in pants.ini or on the CLI.
+
+      # The 'default-debug' definition is a well-known key installed along with the Android SDK.
+
+      [default-debug]
+
+      build_type: debug
+      keystore_location: %(homedir)s/.android/debug.keystore
+      keystore_alias: androiddebugkey
+      keystore_password: android
+      key_password: android
+      """
+    )
+
+    config = os.path.expanduser(config)
+
+    try:
+      with safe_open(config, 'w') as config_file:
+        config_file.write(ini)
+    except OSError as e:
+      raise cls.AndroidConfigError("Problem creating Android keystore config file: {0}".format(e))

src/python/pants/backend/android/keystore/keystore_resolver.py

@@ -0,0 +1,102 @@
+# coding=utf-8
+# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
+                        print_function, unicode_literals)
+
+import os
+
+from pants.base.config import Config, SingleFileConfig
+
+
+class KeystoreResolver(object):
+  """
+  Read a keystore config.ini file and instantiate Keystore objects with the info.
+
+  A Keystore config is an .ini file with valid syntax as parsed by Python's ConfigParser.
+  Each definition requires an arbitrary [name] section followed by the following five fields:
+  build_type, keystore_location, keystore_alias, keystore_password, key_password.
+
+  The specs of these fields can be seen below in the Keystore object docstring.
+  """
+
+  class Error(Exception):
+    """Indicates an invalid android distribution."""
+
+  _CONFIG_SECTION = 'android-keystore-location'
+
+  @classmethod
+  def resolve(cls, config_file):
+    """Parse a keystore config file and return a list of Keystore objects."""
+
+    config = Config.create_parser()
+    try:
+      with open(config_file, 'rb') as keystore_config:
+        config.readfp(keystore_config)
+    except IOError:
+      raise KeystoreResolver.Error("The \'--{0}\' option must point at a valid .ini file holding "
+                                   "keystore definitions.".format(cls._CONFIG_SECTION))
+    parser = SingleFileConfig(config_file, config)
+    key_names = config.sections()
+    keys = []
+
+    def create_key(key_name):
+      """Instantiate Keystore objects."""
+      keystore = Keystore(keystore_name=key_name,
+                          build_type=parser.get_required(key_name, 'build_type'),
+                          keystore_location=parser.get_required(key_name, 'keystore_location'),
+                          keystore_alias=parser.get_required(key_name, 'keystore_alias'),
+                          keystore_password=parser.get_required(key_name, 'keystore_password'),
+                          key_password=parser.get_required(key_name, 'key_password'))
+      return keystore
+
+    for name in key_names:
+      try:
+        keys.append(create_key(name))
+      except Config.ConfigError as e:
+        raise KeystoreResolver.Error(e)
+    return keys
+
+
+class Keystore(object):
+  """Represents a keystore configuration."""
+
+  def __init__(self,
+               keystore_name=None,
+               build_type=None,
+               keystore_location=None,
+               keystore_alias=None,
+               keystore_password=None,
+               key_password=None):
+    """
+    :param string name: Arbitrary name of keystore. This is the [section] of the .ini config file.
+    :param string build_type: The build type of the keystore. One of (debug, release).
+    :param string keystore_location: path/to/keystore.
+    :param string keystore_alias: The alias of this keystore.
+    :param string keystore_password: The password for the keystore.
+    :param string key_password: The password for the key.
+    """
+
+    self._type = None
+    self._build_type = build_type
+
+    self.keystore_name = keystore_name
+    # The os call is robust against None b/c it was validated in KeyResolver with get_required().
+    self.keystore_location = os.path.expandvars(keystore_location)
+    self.keystore_alias = keystore_alias
+    self.keystore_password = keystore_password
+    self.key_password = key_password
+
+  @property
+  def build_type(self):
+    """Return the build type of the keystore. Required to be either 'debug' or 'release'."""
+    # The build_type does not get validated until this property is called.
+    if self._type is None:
+      keystore_type = self._build_type.lower()
+      if keystore_type not in ('release', 'debug'):
+        raise ValueError(self, "The 'build_type' must be one of (debug, release)"
+                               " instead of: '{0}'.".format(self._build_type))
+      else:
+        self._type = keystore_type
+    return self._type

src/python/pants/backend/android/register.py

@@ -7,11 +7,10 @@
 
 from pants.backend.android.targets.android_binary import AndroidBinary
 from pants.backend.android.targets.android_resources import AndroidResources
-from pants.backend.android.targets.keystore import Keystore
 from pants.backend.android.tasks.aapt_gen import AaptGen
 from pants.backend.android.tasks.aapt_builder import AaptBuilder
 from pants.backend.android.tasks.dx_compile import DxCompile
-from pants.backend.android.tasks.jarsigner_task import JarsignerTask
+from pants.backend.android.tasks.sign_apk import SignApkTask
 from pants.base.build_file_aliases import BuildFileAliases
 from pants.goal.task_registrar import TaskRegistrar as task
 
@@ -21,12 +20,11 @@ def build_file_aliases():
     targets={
       'android_binary': AndroidBinary,
       'android_resources': AndroidResources,
-      'keystore': Keystore,
     }
   )
 
 def register_goals():
   task(name='aapt', action=AaptGen).install('gen')
-  task(name='dex', action=DxCompile).install('dex')
+  task(name='dex', action=DxCompile).install()
   task(name='apk', action=AaptBuilder).install('bundle')
-  task(name='sign', action=JarsignerTask).install('sign')
+  task(name='sign', action=SignApkTask).install()

src/python/pants/backend/android/targets/BUILD

@@ -15,10 +15,9 @@ python_library(
     'android_binary.py',
     'android_resources.py',
     'android_target.py',
-    'build_type_mixin.py',
-    'keystore.py',
   ],
   dependencies = [
+    'src/python/pants/backend/jvm/targets:jvm',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:exceptions',
     'src/python/pants/base:payload_field',

src/python/pants/backend/android/targets/android_binary.py

@@ -6,27 +6,7 @@
                         print_function, unicode_literals)
 
 from pants.backend.android.targets.android_target import AndroidTarget
-from pants.backend.android.targets.build_type_mixin import BuildTypeMixin
 
 
-class AndroidBinary(AndroidTarget, BuildTypeMixin):
+class AndroidBinary(AndroidTarget):
   """Produces an Android binary."""
-
-  def __init__(self,
-               build_type=None,
-               *args,
-               **kwargs):
-    """
-    :param string build_type: One of [debug, release]. The keystore to sign the package with.
-      Set as 'debug' by default.
-    """
-    super(AndroidBinary, self).__init__(*args, **kwargs)
-    self._build_type = None
-    # default to 'debug' builds for now.
-    self._keystore = build_type if build_type else 'debug'
-
-  @property
-  def build_type(self):
-    if self._build_type is None:
-      self._build_type = self.get_build_type(self._keystore)
-    return self._build_type

src/python/pants/backend/android/targets/android_target.py

@@ -16,7 +16,8 @@ class AndroidTarget(JvmTarget):
   """A base class for all Android targets."""
 
   # Missing attributes from the AndroidManifest would eventually error in the compilation process.
-  # But since the error would raise here in the target definition, we are catching the exception
+  # But since the error would raise here in the target definition, we are catching the exception.
+
   class BadManifestError(Exception):
     """Indicates an invalid android manifest."""
 
@@ -32,10 +33,8 @@ def __init__(self,
       Defaults to the latest full release.
     :param manifest: path/to/file of 'AndroidManifest.xml' (required name). Paths are relative
       to the BUILD file's directory.
-      Set as 'debug' by default.
     """
     super(AndroidTarget, self).__init__(address=address, **kwargs)
-
     self.add_labels('android')
 
     # TODO(pl): These attributes should live in the payload
@@ -54,9 +53,9 @@ def __init__(self,
     # If unable to parse application name, silently falls back to target name.
     self.app_name = self.get_app_name() if self.get_app_name() else self.name
 
-  # TODO(mateor) Peel parsing into a ManifestParser class to ensure it's robust against bad input
+  # TODO(mateor) Peel parsing into a ManifestParser class to ensure it's robust against bad input.
   # Parsing as in Android Donut's testrunner:
-  # https://github.com/android/platform_development/blob/master/testrunner/android_manifest.py
+  # https://github.com/android/platform_development/blob/master/testrunner/android_manifest.py.
   def get_package_name(self):
     """Return the package name of the Android target."""
     tgt_manifest = parse(self.manifest).getElementsByTagName('manifest')
@@ -76,5 +75,8 @@ def get_target_sdk(self):
   def get_app_name(self):
     """Return a string with the application name of the package, return None if not found."""
     tgt_manifest = parse(self.manifest).getElementsByTagName('activity')
-    package_name = tgt_manifest[0].getAttribute('android:name')
-    return package_name.split(".")[-1]
+    try:
+      package_name = tgt_manifest[0].getAttribute('android:name')
+      return package_name.split(".")[-1]
+    except:
+      return None