Scripts to generate Android.mk for framework Skia.

platform_tools/android/bin/gyp_to_android.py

Index: platform_tools/android/bin/gyp_to_android.py
diff --git a/platform_tools/android/bin/gyp_to_android.py b/platform_tools/android/bin/gyp_to_android.py
new file mode 100644
index 0000000000000000000000000000000000000000..8fc9f43a9b27c119e8bb53ebf9da6c870b070d1d
--- /dev/null
+++ b/platform_tools/android/bin/gyp_to_android.py
@@ -0,0 +1,430 @@
+#!/usr/bin/python
+
+# Copyright 2014 Google Inc.
+#
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""
+    Script for generating the Android framework's verision of Skia from gyp

[epoger, 2014/01/23 18:57:58]

No need to indent these lines.

verision -> version

[scroggo, 2014/01/24 20:09:02]

Done.

+    files.
+"""
+
+import os
+
+# Folder containing all gyp files and generated gypd files.
+GYP_FOLDER = 'gyp'
+
+# FIXME: Currently the x86 specific files are not included. Include them.

[epoger, 2014/01/23 18:57:58]

I think we more commonly use TODO(username)

[scroggo, 2014/01/24 20:09:02]

Done.

+INCLUDE_X86_OPTS = False
+
+def WriteGroup(f, name, items, append):

[epoger, 2014/01/23 18:57:58]

Throughout:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Naming calls for
lower_with_under() naming for functions and methods.

Except when we are trying to maintain consistency within existing code, I think
we should follow the style guide.

[scroggo, 2014/01/24 20:09:02]

I would swear I found some style guide that told me to use the format I used,
but now I can't find it. Will fix after I merge in the tests.

Added a TODO for now.

[scroggo, 2014/01/31 22:16:24]

And thanks to Joe I now see where I found that:
https://engdoc.corp.google.com/eng/doc/devguide/py/styleguide.shtml?cl=head

Now we just need a guide to tell us which style guide to use...

+  """
+  Helper function to list all names passed to a variable.
+  @param f File open for writing (Android.mk)
+  @param name Name of the makefile variable (e.g. LOCAL_CFLAGS)
+  @param items list of strings to be passed to the variable.
+  @param append Whether to append to the variable or overwrite it.
+  """
+  if len(items) == 0:

[epoger, 2014/01/23 18:57:58]

mild preference for "if not items:", which will handle either a 0-length list or
None 

[scroggo, 2014/01/24 20:09:02]

That is clearer, I think. Done.

+    return
+  if append:
+    f.write('%s += \\\n' % name)
+  else:
+    f.write('%s := \\\n' % name)
+  for item in items:
+    f.write('\t%s \\\n' % item)

[epoger, 2014/01/23 18:57:58]

There's probably some easy way to make python do this for you, like

f.write(' '.join(items))

or something like that.
If you want to format them in a nice human-readable way, this would work:

f.write('%s ' % name)
if append:
  f.write('+=')
else:
  f.write(':=')
f.write(' \\\n\t')
f.write(' \\\n\t'.join(items))
f.write('\n')

Edit: having typed all that out, I think yours is clearer anyway.  But food for
thought.

[scroggo, 2014/01/24 20:09:02]

I think the choice between '+' and ':' is clearer the way I wrote it. I hear
there's a sneaky way to do <cond> ? <true branch> : <false branch> in Python
using 'and' and 'or', but it is prone to error so I think I'll save myself the
trouble....

As for the rest, I'm not being very Pythonic by using a for statement. I've
taken your suggestion a step further - if I create a new list with the first
entry being the name (and '+' or ':'), and then use ' \\\n\t'.join() for the
whole list, I think that ends up being cleaner. It also means my last line
doesn't end in a useless '\'.

+  # Add an extra blank line since the last line ended in a \
+  f.write('\n')
+
+class VarsDict(dict):
+  """
+  Custom dict of lists for each makefile variable. Does not allow insertion of

[epoger, 2014/01/23 18:57:58]

I think the Python-y way of doing this is
http://docs.python.org/2/library/collections.html#collections.namedtuple , as
in:
MakefileVars = namedtuple('MakefileVars', ['cflags', 'cppflags', 'defines', ...
'known_targets'])

Then, you can access the fields by name:
WriteGroup(f, 'LOCAL_CFLAGS', makefile_vars.cflags, True)
WriteGroup(f, 'LOCAL_CPPFLAGS', makefile_vars.cppflags, append)
...

Edit: Based on usage of VarsDict throughout the rest of the module, I'm not sure
how the data structure is best arranged.

[scroggo, 2014/01/24 20:09:02]

Added a TODO. Will think on it.

[scroggo, 2014/01/31 22:16:24]

Patch set 13 uses a namedtuple. It has some downsides (documented elsewhere),
but I think overall it yields much clearer code. Thanks for the suggestion!

+  new keys.
+  """
+  def __init__(self):
+    dict.__init__(self)
+    self.__allow_insert = True
+    self['cflags'] = []
+    self['cppflags'] = []

[epoger, 2014/01/23 18:57:58]

Are these truly lists (ordered, allowing multiple occurrences of the same value)
or sets (unordered, no more than one occurrence of each value)?

See http://docs.python.org/2/tutorial/datastructures.html#sets , which includes
this tidbit that may be helpful to you:
"Set objects also support mathematical operations like union, intersection,
difference, and symmetric difference."

[scroggo, 2014/01/24 20:09:02]

Thanks for the tip. They're actually neither - they are ordered lists, with only
one occurrence of each value.

They are ordered for two reasons:
1. It will be easier to track changes in the resulting makefile.
2. For SkImageDecoders, we do care about the order (since the last one compiled
will be the first format checked for). We could change things so this does not
depend on the compile order (in a future change), which would eliminate this
case.

The intersect/difference operators would be useful though! I could write an
ordered set, but I think this would just be more complicated...

[scroggo, 2014/01/31 22:16:24]

Now they are OrderedSets. I don't think it was so complicated after all.

+    self['defines'] = []
+    self['sources'] = []
+    self['shared_libraries'] = []
+    self['static_libraries'] = []
+    self['include_dirs'] = []
+    self['export_include_dirs'] = []
+    self['known_targets'] = []
+    self.__allow_insert = False
+    self.__allow_convert = True
+
+  def __setitem__(self, k, v):
+    # Prevent the insertion of arbitrary keys.
+    assert self.__allow_insert
+    dict.__setitem__(self, k, v)
+
+  def ConvertDefines(self):
+    """
+    Add each string in 'defines' to 'cflags', prepended with '-D'. Can only

[epoger, 2014/01/23 18:57:58]

Alternatively, I guess you could add a GetCflags() method that would return
something based on cflags and defines fields.  I don't know which is more
appropriate for your use.

[scroggo, 2014/01/24 20:09:02]

Good thinking. I could also add defines directly to the cflags. Will think on
it.

+    be done once.
+    """
+    assert self.__allow_convert
+    self.__allow_convert = False
+    for define in self['defines']:
+      define = '-D' + define

[epoger, 2014/01/23 18:57:58]

I think modifying the iterator is dangerous/confusing.  Instead, maybe:

for define in self['defines']:
  self['cflags'].append('-D' + define)

[scroggo, 2014/01/24 20:09:02]

Good point. I'll add that, and just know that ConvertDefines may go away
entirely anyway.

[scroggo, 2014/01/31 22:16:24]

As hinted at previously, patch set 13 eliminates ConvertDefines entirely.
Instead, defines are added (prepended with '-D') to LOCAL_CFLAGS to begin with.
The only thing gained by ConvertDefines was it grouped the defines at the end.
I'm not sure that's necessary (though it could also be done by sorting) and the
current way of doing things requires some extra smarts to make sure it's only
done once.

+      self['cflags'].append(define)
+
+def WriteLocalVars(f, var_dict, append):
+  """
+  Helper function to write all the members of var_dict to the makefile.
+  @param f File open for writing (Android.mk)
+  @param var_dict VarsDict holding the unique values for one configuration.
+  @param append Whether to append to each makefile variable or overwrite it.
+  """
+  var_dict.ConvertDefines()
+  # Always append LOCAL_CFLAGS. This allows us to define some early on in the
+  # makefile and not overwrite them.
+  WriteGroup(f, 'LOCAL_CFLAGS', var_dict['cflags'], True)

[epoger, 2014/01/23 18:57:58]

Maybe we should just use 'LOCAL_CFLAGS' as the key into var_dict, instead of
'cflags'?  Then your could set all the members like this:

for key in ['LOCAL_SRC_FILES', 'LOCAL_SHARED_LIBRARIES', ...]:
  WriteGroup(f, key, var_dict[key], append)

[scroggo, 2014/01/24 20:09:02]

I could potentially take that further and pass the dictionary itself. Added a
TODO for now, and will think on it.

[scroggo, 2014/01/31 22:16:24]

Done.

+  WriteGroup(f, 'LOCAL_CPPFLAGS', var_dict['cppflags'], append)
+  WriteGroup(f, 'LOCAL_SRC_FILES', var_dict['sources'], append)
+  WriteGroup(f, 'LOCAL_SHARED_LIBRARIES', var_dict['shared_libraries'], append)
+  WriteGroup(f, 'LOCAL_STATIC_LIBRARIES', var_dict['static_libraries'], append)
+  WriteGroup(f, 'LOCAL_C_INCLUDES', var_dict['include_dirs'], append)
+  WriteGroup(f, 'LOCAL_EXPORT_C_INCLUDE_DIRS', var_dict['export_include_dirs'],
+             append)
+
+AUTOGEN_WARNING = (
+"""
+###############################################################################
+#
+# THIS FILE IS AUTOGENERATED BY GYP_TO_ANDROID.PY. DO NOT EDIT.
+#
+###############################################################################
+
+"""
+)
+
+DEBUGGING_HELP = (
+"""
+###############################################################################
+#
+# PROBLEMS WITH SKIA DEBUGGING?? READ THIS...
+#
+# The debug build results in changes to the Skia headers. This means that those
+# using libskia must also be built with the debug version of the Skia headers.
+# There are a few scenarios where this comes into play:
+#
+# (1) You're building debug code that depends on libskia.
+#   (a) If libskia is built in release, then define SK_RELEASE when building
+#       your sources.
+#   (b) If libskia is built with debugging (see step 2), then no changes are
+#       needed since your sources and libskia have been built with SK_DEBUG.
+# (2) You're building libskia in debug mode.
+#   (a) RECOMMENDED: You can build the entire system in debug mode. Do this by
+#       updating your build/config.mk to include -DSK_DEBUG on the line that
+#       defines COMMON_GLOBAL_CFLAGS
+#   (b) You can update all the users of libskia to define SK_DEBUG when they are
+#       building their sources.
+#
+# NOTE: If neither SK_DEBUG or SK_RELEASE are defined then Skia checks NDEBUG to
+#       determine which build type to use.
+###############################################################################
+
+"""
+)
+
+def WriteAndroidMk(common, arm, armNeon, x86, default):
+  """
+  Given all the variables, write the final make file.
+  @param common VarsDict holding variables definitions common to all
+                configurations.
+  @param arm VarsDict holding variable definitions unique to arm. Will be
+             written to the makefile inside an 'ifeq ($(TARGET_ARCH), arm)'
+             block.
+  @param armNeon VarsDict holding variable definitions unique to arm with neon.
+                 Will be written inside an 'ifeq ($(ARCH_ARM_HAVE_NEON),true)'
+                 block nested inside an 'ifeq ($(TARGET_ARCH), arm)' block.
+  @param x86 VarsDict holding variable definitions unique to x86. Will be
+             written inside an 'ifeq ($(TARGET_ARCH),x86)' block.
+  @param default VarsDict holding variable definitions for an architecture
+                 without custom optimizations.
+  TODO: Add mips.
+  """
+  with open('Android.mk', 'w') as f:
+    f.write(AUTOGEN_WARNING)
+    f.write('BASE_PATH := $(call my-dir)\n')
+    f.write('LOCAL_PATH:= $(call my-dir)\n')
+
+    f.write(DEBUGGING_HELP)
+
+    f.write('include $(CLEAR_VARS)\n')
+
+    f.write('LOCAL_ARM_MODE := thumb\n')
+
+    # need a flag to tell the C side when we're on devices with large memory
+    # budgets (i.e. larger than the low-end devices that initially shipped)
+    f.write('ifeq ($(ARCH_ARM_HAVE_VFP),true)\n')
+    f.write('\tLOCAL_CFLAGS += -DANDROID_LARGE_MEMORY_DEVICE\n')
+    f.write('endif\n\n')
+
+    f.write('ifeq ($(TARGET_ARCH),x86)\n')
+    f.write('\tLOCAL_CFLAGS += -DANDROID_LARGE_MEMORY_DEVICE\n')
+    f.write('endif\n\n')
+
+    f.write('# used for testing\n')
+    f.write('#LOCAL_CFLAGS += -g -O0\n\n')
+
+    f.write('ifeq ($(NO_FALLBACK_FONT),true)\n')
+    f.write('\tLOCAL_CFLAGS += -DNO_FALLBACK_FONT\n')
+    f.write('endif\n\n')
+
+    WriteLocalVars(f, common, False)
+
+    f.write('ifeq ($(TARGET_ARCH),arm)\n')
+    f.write('ifeq ($(ARCH_ARM_HAVE_NEON),true)\n')
+    WriteLocalVars(f, armNeon, True)
+    f.write('endif\n\n')
+    WriteLocalVars(f, arm, True)
+
+    if INCLUDE_X86_OPTS:
+      f.write('else ifeq ($(TARGET_ARCH),x86)\n')
+      WriteLocalVars(f, x86, True)
+
+    f.write('else\n')
+    WriteLocalVars(f, default, True)
+    f.write('endif\n\n')
+
+    f.write('include external/stlport/libstlport.mk\n')
+    f.write('LOCAL_MODULE:= libskia\n')
+    f.write('include $(BUILD_SHARED_LIBRARY)\n')
+
+def AddToList(li, item):
+  """
+  Add item to li, but only if it is not already in li.

[epoger, 2014/01/23 18:57:58]

sounds like a set, not a list

[scroggo, 2014/01/24 20:09:02]

As stated above, I do care about order.

+  """
+  if item not in li:
+    li.append(item)
+
+def GetMembers(d, name):

[epoger, 2014/01/23 18:57:58]

I think this function can be replaced by calls to

d.get(name, [])

See http://www.tutorialspoint.com/python/dictionary_get.htm

[scroggo, 2014/01/24 20:09:02]

Done. Thanks!

+  """
+  Get the list corresponding to name in dictionary d.
+  """
+  if not name in d:
+    return []
+  return d[name]
+
+def ParseDictionary(var_dict, d, current_target_name):
+  """
+  Helper function to get the meaningful entries in a dictionary.
+  @param var_dict VarsDict object for storing the results of the parsing.
+  @param d Dictionary object to parse.
+  @param current_target_name The current target being parsed. If this
+                             dictionary is a target, this will be its entry
+                             'target_name'. Otherwise, this will be the name of
+                             the target which contains this dictionary.
+  """
+  for source in GetMembers(d, 'sources'):
+    if source.endswith('.h'):
+      # Android.mk does not need the header files.
+      continue
+    if source.endswith('gypi'):
+      # The gypi files are included in sources, but the sources they included
+      # are also included. No need to parse them again.
+      continue
+    # The path is relative to the gyp folder, but Android wants the path
+    # relative to the root.
+    source = source.replace('../src', 'src', 1)
+    AddToList(var_dict['sources'], source)
+
+  for lib in GetMembers(d, 'libraries'):
+    if lib.endswith('.a'):
+      # Remove the '.a'
+      lib = lib[:-2]
+      # Add 'lib', if necessary
+      if not lib.startswith('lib'):
+        lib = 'lib' + lib
+      AddToList(var_dict['static_libraries'], lib)
+    else:
+      # lib will be in the form of '-l<name>'. Change it to 'lib<name>'
+      lib = lib.replace('-l', 'lib', 1)
+      AddToList(var_dict['shared_libraries'], lib)
+
+  for dependency in GetMembers(d, 'dependencies'):
+    # Each dependency is listed as
+    #   <path_to_file>:<target>#target
+    li = dependency.split(':')
+    assert(len(li) <= 2 and len(li) >= 1)
+    sub_targets = []
+    if len(li) == 2 and li[1] != '*':
+      sub_targets.append(li[1].split('#')[0])
+    sub_path = li[0]
+    assert(sub_path.endswith('.gyp'))
+    # Although the original reference is to a .gyp, parse the corresponding
+    # gypd file, which was constructed by gyp.
+    sub_path = sub_path + 'd'
+    ParseGypd(var_dict, sub_path, sub_targets)
+
+  if 'default_configuration' in d:
+    config_name = d['default_configuration']
+    # default_configuration is meaningless without configurations
+    assert('configurations' in d)
+    config = d['configurations'][config_name]
+    ParseDictionary(var_dict, config, current_target_name)
+
+  for flag in GetMembers(d, 'cflags'):
+    AddToList(var_dict['cflags'], flag)
+  for flag in GetMembers(d, 'cflags_cc'):
+    AddToList(var_dict['cppflags'], flag)
+
+  for include in GetMembers(d, 'include_dirs'):
+    # The input path will be relative to gyp/, but Android wants relative to
+    # LOCAL_PATH
+    include = include.replace('..', '$(LOCAL_PATH)', 1)
+    # Remove a trailing slash, if present.
+    if include.endswith('/'):
+      include = include[:-1]
+    AddToList(var_dict['include_dirs'], include)
+    # For the top level, libskia, include directories should be exported.
+    if current_target_name == 'libskia':
+      AddToList(var_dict['export_include_dirs'], include)
+
+  for define in GetMembers(d, 'defines'):
+    AddToList(var_dict['defines'], define)
+
+def ParseGypd(var_dict, path, desired_targets=None):
+  """
+  Parse a gypd file.
+  @param var_dict VarsDict object for storing the result of the parse.
+  @param path Path to gypd file.
+  @param desired_targets List of targets to be parsed from this file. If empty,
+                         parse all targets.
+  """
+  d = {}
+  with open(path, 'r') as f:
+    # Read the entire file as a dictionary
+    d = eval(f.read())
+  # The gypd file is structured such that the top level dictionary has an entry
+  # named 'targets'
+  if 'targets' not in d:
+    return
+  targets = d['targets']
+  for target in targets:
+    target_name = target['target_name']
+    if target_name in var_dict['known_targets']:
+      # Avoid circular dependencies
+      continue
+    if desired_targets and target_name not in desired_targets:
+      # Our caller does not depend on this one
+      continue
+    # Add it to our known targets so we don't parse it again
+    var_dict['known_targets'].append(target_name)
+
+    ParseDictionary(var_dict, target, target_name)
+
+def Intersect(var_dict_list):

[epoger, 2014/01/23 18:57:58]

I think the VarsDict class, and functions that operate on VarsDict objects,
should be extracted into a separate module and unittested.

There are plenty of ways to get these implementations wrong, and they will cause
mysterious failures.

[scroggo, 2014/01/24 20:09:02]

Added a TODO. Will do in a followup patch set.

[scroggo, 2014/01/31 22:16:24]

Done.

+  """
+  Find the intersection of a list of VarsDicts and trim each input to its
+  unique entries.
+  @param var_dict_list list of VarsDicts.
+  @return VarsDict containing list entries common to all VarsDicts in
+          var_dict_list
+  """
+  intersection = VarsDict()
+  # First VarsDict
+  var_dictA = var_dict_list[0]
+  # The rest.
+  otherVarDicts = var_dict_list[1:]
+
+  for key in var_dictA.keys():
+    # Copy A's list, so we can continue iterating after modifying the original.
+    aList = list(var_dictA[key])
+    for item in aList:
+      # If item is in all lists, add to intersection, and remove from all.
+      in_all_lists = True
+      for var_dict in otherVarDicts:
+        # Each dictionary must be a VarsDict, which always contains the same
+        # set of keys.
+        if not item in var_dict[key]:
+          in_all_lists = False
+          break
+      if in_all_lists:
+        intersection[key].append(item)
+        for var_dict in var_dict_list:
+          var_dict[key].remove(item)
+  return intersection
+
+def CleanGypdFiles():
+  """
+  Remove the gypd files generated by android_framework_gyp.main().
+  """
+  assert os.path.isdir(GYP_FOLDER)
+  files = os.listdir(GYP_FOLDER)
+  for f in files:
+    if f.endswith('gypd'):
+      os.remove(os.path.join(GYP_FOLDER, f))
+
+import android_framework_gyp
+
+def main():
+  # Move up to top of trunk
+  script_dir = os.path.dirname(__file__)
+  skia_dir = os.path.normpath(os.path.join(script_dir, os.pardir, os.pardir,
+                                           os.pardir))
+  os.chdir(skia_dir)
+
+  main_gypd_file = os.path.join(GYP_FOLDER, 'android_framework_lib.gypd')
+
+  print 'Creating Android.mk',
+
+  # Call the script to generate gypd files, first using a non-existant archtype.
+  android_framework_gyp.main('other', False)
+  # And store its variables in the default dict
+  default_var_dict = VarsDict()
+  ParseGypd(default_var_dict, main_gypd_file)
+  CleanGypdFiles()
+
+  print '.',
+
+  # Now do the same for Arm. arm_var_dict will contain all the variable
+  # definitions needed to build with arm (meaning that it will share most
+  # with default_var_dict).
+  android_framework_gyp.main('arm', False)
+  arm_var_dict = VarsDict()
+  ParseGypd(arm_var_dict, main_gypd_file)
+  CleanGypdFiles()
+
+  print '.',
+
+  # Now do the same for Arm/Neon.
+  android_framework_gyp.main('arm', True)
+  arm_neon_var_dict = VarsDict()
+  ParseGypd(arm_neon_var_dict, main_gypd_file)
+  CleanGypdFiles()
+
+  print '.'
+
+  # Now do the same for x86.
+  x86_var_dict = VarsDict()
+  if INCLUDE_X86_OPTS:
+    android_framework_gyp.main('x86', False)
+    ParseGypd(x86_var_dict, main_gypd_file)
+    CleanGypdFiles()
+
+  # Compute the intersection of all targets. All the files in the intersection
+  # should be part of the makefile always. Each dict will now contain trimmed
+  # lists containing only variable definitions specific to that configuration.
+  var_dict_list = [default_var_dict, arm_var_dict, arm_neon_var_dict]
+  if INCLUDE_X86_OPTS:
+    var_dict_list.append(x86_var_dict)
+  common = Intersect(var_dict_list)
+
+  # Further trim arm_neon_var_dict with arm_var_dict. After this call,
+  # arm_var_dict (which will now be the intersection) includes all definitions
+  # used by both arm and arm + neon, and arm_neon_var_dict will only contain
+  # those specific to arm + neon.
+  arm_var_dict = Intersect([arm_var_dict, arm_neon_var_dict])
+
+  WriteAndroidMk(common, arm_var_dict, arm_neon_var_dict, x86_var_dict,
+                 default_var_dict)
+
+if __name__ == '__main__':
+  main()