Split apk-build.xml into 3 steps

Split apk-build.xml into 3 steps

In preparation for rewriting these actions in python, split apk-build.xml into smaller ant scripts. This should not change the functionality and mostly is just splitting the file and identifying the properties and defines required for each of the new files.

Some of the configuration is moved out of ant and into gyp (i.e. the android_manifest_path is now gyp-configurable).

Also, the ant scripts now use the correct basedir! That is, these ant scripts actually understand relative paths. To go along with this, resource_dir is the relative path to the resource directory (rather than a path relative to java_in_dir).

BUG=158821
TBR=torne@chromium.org, brettw@chromium.org
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190712

[shashi, 2013-03-22 04:18:05]

lgtm

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcode75
build/java_apk.gypi:75: 'asset_location%':
'<(intermediate_dir)/<(_target_name)/assets',
Does the default assets folder has target name twice
'<(PRODUCT_DIR)/<(_target_name)/<(_target_name)/assets'?

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcode78
build/java_apk.gypi:78: 'android_manifest':
'<(java_in_dir)/AndroidManifest.xml',
Nice, does this mean now it is possible to have different manifests for
different channels, rather than a custom on the fly script that does a git reset
--hard :).

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:224:
'-DANDROID_MANIFEST=<(intermediate_dir)/AndroidManifest.xml',
nit: do not need to pass ANDROID_MANIFEST here.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:252: '<(java_in_dir)/AndroidManifest.xml',
nit: Do not need dependency on AndroidManifest.xml since package seems to depend
on out.abs.manifest.file.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:276:
'-DANDROID_MANIFEST=<(intermediate_dir)/AndroidManifest.xml',
Same here.

[Yaron, 2013-03-22 22:32:35]

lgtm

https://codereview.chromium.org/12963009/diff/17001/android_webview/android_w...
File android_webview/android_webview_tests.gypi (left):

https://codereview.chromium.org/12963009/diff/17001/android_webview/android_w...
android_webview/android_webview_tests.gypi:17: 'java_in_dir':
'../android_webview/javatests',
Nice to see this fixed!

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-cod...
File build/android/ant/apk-codegen.xml (right):

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-cod...
build/android/ant/apk-codegen.xml:3: Copyright (C) 2005-2008 The Android Open
Source Project
Not sure if this is the right copyright header anymore.

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-pac...
File build/android/ant/apk-package.xml (right):

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-pac...
build/android/ant/apk-package.xml:42: <property name="configuration.name"
value="${CONFIGURATION_NAME}" />
Doesn't look like this is used. Either use the ant variable below or remove this
line

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi#newco...
build/java_apk.gypi:75: 'asset_location%':
'<(intermediate_dir)/<(_target_name)/assets',
+1 for Shashi's comment

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi#newco...
build/java_apk.gypi:180: '-DANDROID_SDK_ROOT=<(android_sdk_root)',
Probably keep common params in the same order so it's easy to compare (OUT_DIR
is different here from next)

[cjhopman, 2013-03-25 17:38:01]

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcode75
build/java_apk.gypi:75: 'asset_location%':
'<(intermediate_dir)/<(_target_name)/assets',
On 2013/03/22 04:18:06, shashi wrote:
> Does the default assets folder has target name twice
> '<(PRODUCT_DIR)/<(_target_name)/<(_target_name)/assets'?

Done.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcode78
build/java_apk.gypi:78: 'android_manifest':
'<(java_in_dir)/AndroidManifest.xml',
On 2013/03/22 04:18:06, shashi wrote:
> Nice, does this mean now it is possible to have different manifests for
> different channels, rather than a custom on the fly script that does a git
reset
> --hard :).
Yes, I believe so.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:224:
'-DANDROID_MANIFEST=<(intermediate_dir)/AndroidManifest.xml',
On 2013/03/22 04:18:06, shashi wrote:
> nit: do not need to pass ANDROID_MANIFEST here.

Done.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:252: '<(java_in_dir)/AndroidManifest.xml',
On 2013/03/22 04:18:06, shashi wrote:
> nit: Do not need dependency on AndroidManifest.xml since package seems to
depend
> on out.abs.manifest.file.

Done.

https://codereview.chromium.org/12963009/diff/7001/build/java_apk.gypi#newcod...
build/java_apk.gypi:276:
'-DANDROID_MANIFEST=<(intermediate_dir)/AndroidManifest.xml',
On 2013/03/22 04:18:06, shashi wrote:
> Same here.

Done.

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-cod...
File build/android/ant/apk-codegen.xml (right):

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-cod...
build/android/ant/apk-codegen.xml:3: Copyright (C) 2005-2008 The Android Open
Source Project
On 2013/03/22 22:32:35, Yaron wrote:
> Not sure if this is the right copyright header anymore.
I think the only thing that should maybe be changed would be the year, but our
documentation is unclear on it.

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-pac...
File build/android/ant/apk-package.xml (right):

https://codereview.chromium.org/12963009/diff/17001/build/android/ant/apk-pac...
build/android/ant/apk-package.xml:42: <property name="configuration.name"
value="${CONFIGURATION_NAME}" />
On 2013/03/22 22:32:35, Yaron wrote:
> Doesn't look like this is used. Either use the ant variable below or remove
this
> line

Done.

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi#newco...
build/java_apk.gypi:75: 'asset_location%':
'<(intermediate_dir)/<(_target_name)/assets',
On 2013/03/22 22:32:35, Yaron wrote:
> +1 for Shashi's comment

Done.

https://codereview.chromium.org/12963009/diff/17001/build/java_apk.gypi#newco...
build/java_apk.gypi:180: '-DANDROID_SDK_ROOT=<(android_sdk_root)',
On 2013/03/22 22:32:35, Yaron wrote:
> Probably keep common params in the same order so it's easy to compare (OUT_DIR
> is different here from next)

Done. Sorted.

[commit-bot: I haz the power, 2013-03-25 18:06:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12963009/24001

[commit-bot: I haz the power, 2013-03-25 18:07:05]

Presubmit check for 12963009-24001 failed and returned exit status 1.

INFO:root:Found 10 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    content/content_tests.gypi
    android_webview/android_webview_tests.gypi
    content/content_shell.gypi
    chrome/chrome_android.gypi

Presubmit checks took 2.9s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[cjhopman, 2013-03-25 18:51:33]

torne@, brettw@: TBR for OWNERS

[commit-bot: I haz the power, 2013-03-25 20:19:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12963009/33002

[commit-bot: I haz the power, 2013-03-26 07:08:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12963009/33002

[commit-bot: I haz the power, 2013-03-26 10:00:44]

Failed to apply patch for build/java_apk.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file build/java_apk.gypi
  Hunk #4 FAILED at 58.
  Hunk #7 FAILED at 125.
  Hunk #8 FAILED at 141.
  3 out of 8 hunks FAILED -- saving rejects to file build/java_apk.gypi.rej

Patch:       build/java_apk.gypi
Index: build/java_apk.gypi
diff --git a/build/java_apk.gypi b/build/java_apk.gypi
index
3f3e7065e403407013d9c2d1a514fe407779a06f..78a5bff44936fc4496bc748cc6c628fef18feb63
100644
--- a/build/java_apk.gypi
+++ b/build/java_apk.gypi
@@ -12,7 +12,7 @@
 #   'variables': {
 #     'apk_name': 'MyPackage',
 #     'java_in_dir': 'path/to/package/root',
-#     'resource_dir': 'res',
+#     'resource_dir': 'path/to/package/root/res',
 #   },
 #   'includes': ['path/to/this/gypi/file'],
 # }
@@ -29,8 +29,7 @@
 #    each directory in additional_res_dirs.
 #  additional_src_dirs - Additional directories with .java files to be compiled
 #    and included in the output of this target.
-#  asset_location - The directory where assets are located (default:
-#    <(ant_build_out)/<(_target_name)/assets).
+#  asset_location - The directory where assets are located.
 #  generated_src_dirs - Same as additional_src_dirs except used for .java files
 #    that are generated at build time. This should be set automatically by a
 #    target's dependencies. The .java files in these directories are not
@@ -52,7 +51,6 @@
 
 {
   'variables': {
-    'asset_location%': '',
     'additional_input_paths': [],
     'input_jars_paths': [],
     'additional_src_dirs': [],
@@ -60,17 +58,24 @@
     'app_manifest_version_name%': '<(android_app_version_name)',
     'app_manifest_version_code%': '<(android_app_version_code)',
     'proguard_enabled%': 'false',
-    'proguard_flags%': '',
+    'proguard_flags_path%': '<(DEPTH)/build/android/empty_proguard.flags',
     'native_libs_paths': [],
     'jar_name%': 'chromium_apk_<(_target_name).jar',
-    'resource_dir%':'',
+    'resource_dir%':'<(DEPTH)/build/android/ant/empty/res',
     'R_package%':'',
     'additional_res_dirs': [],
     'additional_res_packages': [],
     'is_test_apk%': 0,
     'java_strings_grd%': '',
     'grit_grd_file%': '',
-    'library_manifest_paths%' : [],
+    'library_manifest_paths' : [],
+    'resource_input_paths': [],
+    'intermediate_dir': '<(PRODUCT_DIR)/<(_target_name)',
+    'asset_location%': '<(intermediate_dir)/assets',
+    'codegen_stamp': '<(intermediate_dir)/codegen.stamp',
+    'compile_stamp': '<(intermediate_dir)/compile.stamp',
+    'android_manifest': '<(java_in_dir)/AndroidManifest.xml',
+    'codegen_input_paths': [],
   },
   'sources': [
       '<@(native_libs_paths)'
@@ -88,7 +93,7 @@
       'rule_name': 'copy_and_strip_native_libraries',
       'extension': 'so',
       'variables': {
-        'apk_libraries_dir':
'<(PRODUCT_DIR)/<(_target_name)/libs/<(android_app_abi)',
+        'apk_libraries_dir': '<(intermediate_dir)/libs/<(android_app_abi)',
         'stripped_library_path': '<(apk_libraries_dir)/<(RULE_INPUT_ROOT).so',
       },
       'inputs': [
@@ -107,6 +112,11 @@
     },
   ],
   'conditions': [
+    ['resource_dir!=""', {
+      'variables': {
+        'resource_input_paths': [ '<!@(find <(resource_dir) -name "*")' ]
+      },
+    }],
     ['R_package != ""', {
       'variables': {
         # We generate R.java in package R_package (in addition to the package
@@ -122,6 +132,12 @@
         'additional_res_dirs': ['<(out_res_dir)'],
         # grit_grd_file is used by grit_action.gypi, included below.
         'grit_grd_file': '<(java_in_dir)/strings/<(java_strings_grd)',
+        'resource_input_paths': [
+          # TODO(newt): replace this with .../values/strings.xml once
+          # the English strings.xml is generated as well? That would be
+          # simpler and faster and should be equivalent.
+          '<!@pymod_do_main(grit_info <@(grit_defines) --outputs
"<(out_res_dir)" <(grit_grd_file))',
+        ],
       },
       'actions': [
         {
@@ -138,88 +154,144 @@
   ],
   'actions': [
     {
-      'action_name': 'ant_<(_target_name)',
-      'message': 'Building <(_target_name).',
+      'action_name': 'ant_codegen_<(_target_name)',
+      'message': 'Generating R.java for <(_target_name)',
+      'conditions': [
+        ['is_test_apk == 1', {
+          'variables': {
+            'additional_res_dirs=': [],
+            'additional_res_packages=': [],
+          }
+        }],
+      ],
+      'inputs': [
+        '<(DEPTH)/build/android/ant/apk-codegen.xml',
+        '<(android_manifest)',
+        '>@(library_manifest_paths)'
+        '>@(codegen_input_paths)',
+        '>@(additional_input_paths)',
+      ],
+      'outputs': [
+        '<(codegen_stamp)',
+      ],
+      'action': [
+        'ant', '-quiet',
+        '-DADDITIONAL_RES_DIRS=>(additional_res_dirs)',
+        '-DADDITIONAL_RES_PACKAGES=>(additional_res_packages)',
+        '-DADDITIONAL_R_TEXT_FILES=>(additional_R_text_files)',
+        '-DANDROID_MANIFEST=<(android_manifest)',
+        '-DANDROID_SDK_JAR=<(android_sdk_jar)',
+        '-DANDROID_SDK_ROOT=<(android_sdk_root)',
+        '-DANDROID_SDK_VERSION=<(android_sdk_version)',
+        '-DLIBRARY_MANIFEST_PATHS=>(library_manifest_paths)',
+        '-DOUT_DIR=<(intermediate_dir)',
+        '-DRESOURCE_DIR=<(resource_dir)',
+
+        '-DSTAMP=<(codegen_stamp)',
+        '-Dbasedir=.',
+        '-buildfile',
+        '<(DEPTH)/build/android/ant/apk-codegen.xml',
+
+        # Add list of inputs to the command line, so if inputs change
+        # (e.g. if a Java file is removed), the command will be re-run.
+        # TODO(newt): remove this once crbug.com/177552 is fixed in ninja.
+        '-DTHIS_IS_IGNORED=>!(echo \'>(_inputs)\' | md5sum)',
+      ],
+    },
+    {
+      'action_name': 'ant_compile_<(_target_name)',
+      'message': 'Compiling java for <(_target_name)',
       'inputs': [
-        '<(java_in_dir)/AndroidManifest.xml',
-        '<(DEPTH)/build/android/ant/chromium-apk.xml',
-        '<(DEPTH)/build/android/ant/common.xml',
-        '<(DEPTH)/build/android/ant/apk-build.xml',
+        '<(DEPTH)/build/android/ant/apk-compile.xml',
+        '<(DEPTH)/build/android/ant/create-test-jar.js',
         # If there is a separate find for additional_src_dirs, it will find the
         # wrong .java files when additional_src_dirs is empty.
         '>!@(find >(java_in_dir) >(additional_src_dirs) -name "*.java")',
         '>@(input_jars_paths)',
+        '<(codegen_stamp)',
+        '<(proguard_flags_path)',
+      ],
+      'outputs': [
+        '<(compile_stamp)',
+      ],
+      'action': [
+        'ant', '-quiet',
+        '-DADDITIONAL_SRC_DIRS=>(additional_src_dirs)',
+        '-DANDROID_SDK_JAR=<(android_sdk_jar)',
+        '-DANDROID_SDK_ROOT=<(android_sdk_root)',
+        '-DANDROID_SDK_VERSION=<(android_sdk_version)',
+        '-DAPK_NAME=<(apk_name)',
+        '-DCREATE_TEST_JAR_PATH=<(DEPTH)/build/android/ant/create-test-jar.js',
+        '-DGENERATED_SRC_DIRS=>(generated_src_dirs)',
+        '-DINPUT_JARS_PATHS=>(input_jars_paths)',
+        '-DIS_TEST_APK=<(is_test_apk)',
+        '-DJAR_PATH=<(PRODUCT_DIR)/lib.java/<(jar_name)',
+        '-DOUT_DIR=<(intermediate_dir)',
+        '-DPROGUARD_ENABLED=<(proguard_enabled)',
+        '-DPROGUARD_FLAGS=<(proguard_flags_path)',
+        '-DSOURCE_DIR=<(java_in_dir)/src',
+        '-DTEST_JAR_PATH=<(PRODUCT_DIR)/test.lib.java/<(apk_name).jar',
+
+        '-DSTAMP=<(compile_stamp)',
+        '-Dbasedir=.',
+        '-buildfile',
+        '<(DEPTH)/build/android/ant/apk-compile.xml',
+
+        # Add list of inputs to the command line, so if inputs change
+        # (e.g. if a Java file is removed), the command will be re-run.
+        # TODO(newt): remove this once crbug.com/177552 is fixed in ninja.
+        '-DTHIS_IS_IGNORED=>!(echo \'>(_inputs)\' | md5sum)',
+      ],
+    },
+    {
+      'action_name': 'ant_package_<(_target_name)',
+      'message': 'Packaging <(_target_name).',
+      'inputs': [
+        '<(DEPTH)/build/android/ant/apk-package.xml',
+        #TODO(cjhopman): this should be the stripped library paths.
         '>@(native_libs_paths)',
-        '>@(additional_input_paths)',
-        '>@(library_manifest_paths)',
+        '<(codegen_stamp)',
+        '<(compile_stamp)',
       ],
       'conditions': [
-        ['resource_dir!=""', {
-          'inputs': ['<!@(find <(java_in_dir)/<(resource_dir) -name "*")']
-        }],
-        ['java_strings_grd != ""', {
-          'inputs': [
-            # TODO(newt): replace this with .../values/strings.xml once
-            # the English strings.xml is generated as well? That would be
-            # simpler and faster and should be equivalent.
-            '<!@pymod_do_main(grit_info <@(grit_defines) --outputs
"<(out_res_dir)" <(grit_grd_file))',
-          ],
-        }],
         ['is_test_apk == 1', {
           'variables': {
             'additional_res_dirs=': [],
             'additional_res_packages=': [],
           }
         }],
-        ['proguard_enabled == "true" and proguard_flags != ""', {
-          'inputs': ['<(java_in_dir)/<(proguard_flags)']
-        }],
       ],
       'outputs': [
         '<(PRODUCT_DIR)/apks/<(apk_name).apk',
       ],
       'action': [
         'ant', '-quiet',
-        '-DAPP_ABI=<(android_app_abi)',
-        '-DANDROID_GDBSERVER=<(android_gdbserver)',
-        '-DANDROID_SDK=<(android_sdk)',
+        '-DADDITIONAL_RES_DIRS=>(additional_res_dirs)',
+        '-DADDITIONAL_RES_PACKAGES=>(additional_res_packages)',
+        '-DADDITIONAL_R_TEXT_FILES=>(additional_R_text_files)',
+        '-DANDROID_SDK_JAR=<(android_sdk_jar)',
         '-DANDROID_SDK_ROOT=<(android_sdk_root)',
-        '-DANDROID_SDK_TOOLS=<(android_sdk_tools)',
         '-DANDROID_SDK_VERSION=<(android_sdk_version)',
-        '-DANDROID_TOOLCHAIN=<(android_toolchain)',
-        '-DCHROMIUM_SRC=<(ant_bui…
(message too large)

[commit-bot: I haz the power, 2013-03-26 17:28:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12963009/47001

[commit-bot: I haz the power, 2013-03-26 17:35:22]

Change committed as 190712