Add script to generate CronetChromeWebView.framework

ios/web_view/tools/package_ios.py

Index: ios/web_view/tools/package_ios.py
diff --git a/ios/web_view/tools/package_ios.py b/ios/web_view/tools/package_ios.py
new file mode 100755
index 0000000000000000000000000000000000000000..895beb16a7a8fd0895933161f28fbe45091e3c39
--- /dev/null
+++ b/ios/web_view/tools/package_ios.py
@@ -0,0 +1,94 @@
+#!/usr/bin/python
+# Copyright 2017 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.
+
+"""
+package_ios.py - Build and Package Release and Debug fat libraries for iOS.

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

Maybe it's better to say that this script is for CronetChromeWebView.framework?
The current description sounds like for more general purpose, but it actually
isn't.

[michaeldo, 2017/06/15 20:17:29]

Good point, updated.

+"""
+
+import argparse
+import os
+import shutil
+import sys
+
+def run(command, extra_options=''):

[Eugene But (OOO till 7-30), 2017/06/14 03:38:55]

Please add comments for every non-trivial function

[michaeldo, 2017/06/15 20:17:29]

Done.

+  command = command + ' ' + ' '.join(extra_options)
+  print command
+  return os.system(command)
+
+def package_ios_framework_using_gn(out_dir='out/Framework', extra_options=''):
+  print 'Building CronetChromeWebView Dynamic Framework...'
+
+  # Package all builds in the output directory
+  os.makedirs(out_dir)
+  build_dir = ''
+  for (build_config, gn_extra_args) in [('Debug', 'is_debug=true use_xcode_clang=true'),

[Eugene But (OOO till 7-30), 2017/06/14 03:38:55]

This for loop has only 2 iterations and will unlikely have more. Would it be
better to extract the code inside for-loop to a separate function and call that
function twice?

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

Why do you specify use_xcode_clang=true only for debug build?

[michaeldo, 2017/06/15 20:17:29]

I pulled the options from cronet package_ios.py. I don't know what this option
is here. I've never set it locally. I'll check with cronet folks after I upload
next patch.

The default gn args that are setup for ios use this line, which may explain it:

use_xcode_clang = is_official_build

Even though we are not setting is_official_build, maybe we actually want to set
use_xcode_clang=true for all configs.

[michaeldo, 2017/06/15 20:17:29]

I agree, I reformatted this into method calls.

+        ('Release', 'is_debug=false enable_stripping=true is_official_build=true')]:

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

is_official_build should be false even for release. Confirmed with eugenebut@.
Because this library is not a part of official Chrome release.

[michaeldo, 2017/06/15 20:17:29]

Done.

+    for (target_device, target_cpu, additional_cpu) in [('os', 'arm', 'arm64'),

[Eugene But (OOO till 7-30), 2017/06/14 03:38:55]

ditto

[michaeldo, 2017/06/15 20:17:29]

Done.

+        ('simulator', 'x86', 'x64')]:
+      target_dir = '%s-iphone%s' % (build_config, target_device)
+      build_dir = os.path.join("out", target_dir)
+      gn_args = 'target_os="ios" enable_websockets=false ' \

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

How about providing an option to use Goma? Otherwise it takes too long...

[michaeldo, 2017/06/15 20:17:29]

Great idea. Done.

+                'is_component_build=false ' \
+                'disable_file_support=true disable_ftp_support=true ' \
+                'disable_brotli_filter=true enable_dsyms=true ' \

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

Just curious, do you know if enable_dsyms affects binary size? I have set this
to false for release following your doc...

[michaeldo, 2017/06/15 20:17:29]

Removing enable_dsyms or setting it to true or false all do the same thing.
dsyms are always generated with this script. It's possible that another option
forces the dsym to be created regardless of this specific setting. I will remove
the option for now since it doesn't affect the build.

That said, no it doesn't affect the size, but that seems to be only because this
setting isn't doing anything.

+                'target_cpu="%s" additional_target_cpus = ["%s"] %s' % \
+                (target_cpu, additional_cpu, gn_extra_args)
+
+#is_cronet_build=true?

[Eugene But (OOO till 7-30), 2017/06/14 03:38:55]

Remove this?

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

Yeah I believe this should be set to false even for the combined library.
Otherwise it strips out some functionality in //net etc. which are required for
//ios/web_view. 

[michaeldo, 2017/06/15 20:17:29]

Yes, it should be false, but I left this here as a note to myself.

Setting is_cronet_build sets CRONET_BUILD which is used here:
https://cs.chromium.org/chromium/src/ios/net/cookies/cookie_store_ios.mm?rcl=...

We need to make sure this won't cause issues or we need to define CRONET_BUILD
ourselves.

+      print 'Generating Ninja ' + gn_args
+      gn_result = run('gn gen %s --args=\'%s\'' % (build_dir, gn_args))
+      if gn_result != 0:
+        return gn_result
+
+      print 'Building ' + build_dir
+      build_result = run('ninja -C %s ios/web_view:cronet_ios_web_view_package' % build_dir,

[Eugene But (OOO till 7-30), 2017/06/14 03:38:55]

Does it fit 80 symbols?

[michaeldo, 2017/06/15 20:17:29]

Done.

+                         extra_options)
+      if build_result != 0:
+        return build_result
+
+      # Copy framework.
+      shutil.copytree(os.path.join(build_dir, 'CronetChromeWebView.framework'),
+          os.path.join(out_dir, target_dir, 'CronetChromeWebView.framework'))
+      # Copy symbols.
+      shutil.copytree(os.path.join(build_dir, 'CronetChromeWebView.dSYM'),
+          os.path.join(out_dir, target_dir, 'CronetChromeWebView.framework.dSYM'))
+
+  # Copy common files from last built package.
+  package_dir = os.path.join(build_dir, 'cronet_ios_web_view')
+  shutil.copy2(os.path.join(package_dir, 'AUTHORS'), out_dir)
+  shutil.copy2(os.path.join(package_dir, 'LICENSE'), out_dir)
+  shutil.copy2(os.path.join(package_dir, 'VERSION'), out_dir)
+  # Copy the headers.

[Hiroshi Ichikawa, 2017/06/14 06:31:58]

Maybe you don't need to do this? We only expect headers to be in the framework
of each target.

[michaeldo, 2017/06/15 20:17:29]

Done. I agree, I used cronet script as a template, but I agree this isn't
necessary.

+  shutil.copytree(os.path.join(build_dir,
+                               'CronetChromeWebView.framework', 'Headers'),
+                  os.path.join(out_dir, 'Headers'))
+  print 'CronetChromeWebView framework is packaged into %s' % out_dir
+
+
+def main():
+  description = (
+    'To build CronetChromeWebView.framework call:\n'
+    'package_ios.py out/Frameworks\n'
+  )
+  parser = argparse.ArgumentParser(description=description)
+
+  parser.add_argument('out_dir', nargs=1, help='path to output directory')
+
+  options, extra_options_list = parser.parse_known_args()
+  print options
+  print extra_options_list
+
+  out_dir = options.out_dir[0]
+
+  # Make sure that the output directory does not exist
+  if os.path.exists(out_dir):
+    print >>sys.stderr, 'The output directory already exists: ' + out_dir
+    return 1
+
+  return package_ios_framework_using_gn(out_dir, extra_options_list)
+
+
+if __name__ == '__main__':
+  sys.exit(main())