New issue
Advanced search Search tips

Issue 634430 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Feature



Sign in to add a comment

Add support for building incremental APKs by default

Project Member Reported by nyquist@chromium.org, Aug 4 2016

Issue description

It is easy to forget to build the incremental version of an APK, causing the installation of possibly old APKs to confuse engineers.

A subtly erroneous flow:
$ ninja foo_apk
$ out/Default/bin/install_foo_apk_incremental

The correct should have been:
$ ninja foo_apk_incremental
$ out/Default/bin/install_foo_apk_incremental

The proposal is to make building the incremental APK as default for developers. That makes it so that developers don't have to remember the weird syntax for building it.

Important note though is that we don't want to do this on the builders. They should still be able to build foo_apk and get the real APK.

After a discussion with agrieve@, this bug proposes that we add a GN arg that developers can set:
###
incremental_apk_by_default = true
###

This argument should also make it possible to build the non-incremental version of the APK, by adding a suffix such as _nonincremental:
$ ninja foo_apk_nonincremental

 
Cc: agrieve@chromium.org
Owner: estevenson@chromium.org
Status: Assigned (was: Untriaged)
Another erroneous flow:
$ ninja foo_test_apk_incremental
$ out/Default/bin/run_foo_test_apk  # stale script

With incremental_apk_by_default = true, developers can use the regular flow, but still take advantage of incremental installs:
$ ninja foo_unittests
$ out/Default/bin/run_foo_unittests

$ ninja foo_test_apk
$ out/Default/bin/run_foo_test_apk

$ ninja foo_apk
$ out/Default/bin/install_foo_apk


agrieve@ and I came to the conclusion that it probably isn't worth adding _nonincremental targets for now; it'll require a much larger change and the benefit gained from it is small. 

In progress CL: https://codereview.chromium.org/2562063003/
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce844392ab42112ce1fb92240de7ae102ed4c868

commit ce844392ab42112ce1fb92240de7ae102ed4c868
Author: estevenson <estevenson@chromium.org>
Date: Thu Dec 15 19:57:57 2016

Add incremental_apk_by_default GN arg.

Setting this arg allows _incremental targets and scripts to be used
without explicitly building them. With this arg set to true,
non-incremental targets will build their incremental counterparts if
they exist (i.e. the chrome_public_apk target will actually build
chrome_public_apk_incremental).

This works for .apks, test runner scripts, and install scripts for
instrumentation_test_apks, android_apks, and unittest_apks

BUG= 634430 

Review-Url: https://codereview.chromium.org/2562063003
Cr-Commit-Position: refs/heads/master@{#438901}

[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/build/android/gyp/create_test_runner_script.py
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/build/config/android/config.gni
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/build/config/android/internal_rules.gni
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/build/config/android/rules.gni
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/chrome/android/webapk/libs/runtime_library/javatests/apk_with_webapk_service/BUILD.gn
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/chrome/android/webapk/shell_apk/javatests/dex_optimizer/BUILD.gn
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/chrome/test/android/chrome_public_test_support/BUILD.gn
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/net/android/BUILD.gn
[modify] https://crrev.com/ce844392ab42112ce1fb92240de7ae102ed4c868/testing/test.gni

Status: Fixed (was: Assigned)

Sign in to add a comment