Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5660: Ensure mobile-install works #5664

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ package_group(
]
]

# Define all binary flavors that can be built. Note that these are AABs, not APKs, and can be
# be installed on a local device or emulator using a 'bazel run' command like so:
# bazel run //:install_oppia_dev
# Define all binary flavors that can be built. Note that these are AABs and thus cannot be installed
# directly. However, their binaries can be installed using mobile-install like so:
# bazel mobile-install //:oppia_dev_binary
[
define_oppia_aab_binary_flavor(flavor = flavor)
for flavor in AVAILABLE_FLAVORS
Expand Down
8 changes: 4 additions & 4 deletions build_flavors.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Macros & definitions corresponding to Oppia binary build flavors.
"""

load("//:oppia_android_application.bzl", "declare_deployable_application", "oppia_android_application")
load("//:oppia_android_application.bzl", "generate_universal_apk", "oppia_android_application")
load("//:version.bzl", "MAJOR_VERSION", "MINOR_VERSION", "OPPIA_ALPHA_KITKAT_VERSION_CODE", "OPPIA_ALPHA_VERSION_CODE", "OPPIA_BETA_VERSION_CODE", "OPPIA_DEV_KITKAT_VERSION_CODE", "OPPIA_DEV_VERSION_CODE", "OPPIA_GA_VERSION_CODE")

# Defines the list of flavors available to build the Oppia app in. Note to developers: this list
Expand Down Expand Up @@ -242,7 +242,7 @@ def define_oppia_aab_binary_flavor(flavor):

This will define two targets:
- //:oppia_<flavor> (the AAB)
- //:install_oppia_<flavor> (the installable binary target--see declare_deployable_application
- //:oppia_<flavor>_universal_apk (the installable binary target--see generate_universal_apk
for details)

Args:
Expand Down Expand Up @@ -278,7 +278,7 @@ def define_oppia_aab_binary_flavor(flavor):
shrink_resources = True if len(_FLAVOR_METADATA[flavor]["proguard_specs"]) != 0 else False,
deps = _FLAVOR_METADATA[flavor]["deps"],
)
declare_deployable_application(
name = "install_oppia_%s" % flavor,
generate_universal_apk(
name = "oppia_%s_universal_apk" % flavor,
aab_target = ":oppia_%s" % flavor,
)
102 changes: 53 additions & 49 deletions oppia_android_application.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -158,61 +158,57 @@ def _package_metadata_into_deployable_aab_impl(ctx):
runfiles = ctx.runfiles(files = [output_aab_file]),
)

def _generate_apks_and_install_impl(ctx):
input_file = ctx.attr.input_file.files.to_list()[0]
def _generate_universal_apk_impl(ctx):
input_aab_file = ctx.attr.input_aab_file.files.to_list()[0]
output_apk_file = ctx.outputs.output_apk_file
debug_keystore_file = ctx.attr.debug_keystore.files.to_list()[0]
apks_file = ctx.actions.declare_file("%s_processed.apks" % ctx.label.name)
deploy_shell = ctx.actions.declare_file("%s_run.sh" % ctx.label.name)

# Reference: https://developer.android.com/studio/command-line/bundletool#generate_apks.
# Reference: https://developer.android.com/tools/bundletool#generate_apks.
# See also the Bazel BUILD file for the keystore for details on its password and alias.
generate_apks_arguments = [
generate_universal_apk_arguments = [
"build-apks",
"--bundle=%s" % input_file.path,
"--bundle=%s" % input_aab_file.path,
"--output=%s" % apks_file.path,
"--ks=%s" % debug_keystore_file.path,
"--ks-pass=pass:android",
"--ks-key-alias=androiddebugkey",
"--key-pass=pass:android",
"--mode=universal",
]

# bundletool only generats an APKs file, so the universal APK still needs to be extracted.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
# bundletool only generats an APKs file, so the universal APK still needs to be extracted.
# bundletool only generate an APKs file, so the universal APK still needs to be extracted.


# Reference: https://docs.bazel.build/versions/master/skylark/lib/actions.html#run.
ctx.actions.run(
outputs = [apks_file],
inputs = ctx.files.input_file + ctx.files.debug_keystore,
inputs = ctx.files.input_aab_file + ctx.files.debug_keystore,
tools = [ctx.executable._bundletool_tool],
executable = ctx.executable._bundletool_tool.path,
arguments = generate_apks_arguments,
mnemonic = "BuildApksFromDeployAab",
progress_message = "Preparing AAB deploy to device",
arguments = generate_universal_apk_arguments,
mnemonic = "GenerateUniversalAPk",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mnemonic = "GenerateUniversalAPk",
mnemonic = "GenerateUniversalAPK",

progress_message = "Generating universal APK from AAB",
)

# References: https://github.com/bazelbuild/bazel/issues/7390,
# https://developer.android.com/studio/command-line/bundletool#deploy_with_bundletool, and
# https://docs.bazel.build/versions/main/skylark/rules.html#executable-rules-and-test-rules.
# Note that the bundletool can be executed directly since Bazel creates a wrapper script that
# utilizes its own internal Java toolchain.
ctx.actions.write(
output = deploy_shell,
content = """
#!/bin/sh
{0} install-apks --apks={1}
echo The APK should now be installed
""".format(ctx.executable._bundletool_tool.short_path, apks_file.short_path),
is_executable = True,
command = """
# Extract APK to working directory.
unzip -q "$(pwd)/{0}" universal.apk
mv universal.apk "$(pwd)/{1}"
""".format(apks_file.path, output_apk_file.path)

# Reference: https://docs.bazel.build/versions/main/skylark/lib/actions.html#run_shell.
ctx.actions.run_shell(
outputs = [output_apk_file],
inputs = [apks_file],
tools = [],
command = command,
mnemonic = "ExtractUniversalAPk",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mnemonic = "ExtractUniversalAPk",
mnemonic = "ExtractUniversalAPK",

progress_message = "Extracting universal APK from .apks file",
)

# Reference for including necessary runfiles for Java:
# https://github.com/bazelbuild/bazel/issues/487#issuecomment-178119424.
runfiles = ctx.runfiles(
files = [
ctx.executable._bundletool_tool,
apks_file,
],
).merge(ctx.attr._bundletool_tool.default_runfiles)
return DefaultInfo(
executable = deploy_shell,
runfiles = runfiles,
files = depset([output_apk_file]),
runfiles = ctx.runfiles(files = [output_apk_file]),
)

_convert_apk_to_module_aab = rule(
Expand Down Expand Up @@ -303,10 +299,13 @@ _package_metadata_into_deployable_aab = rule(
implementation = _package_metadata_into_deployable_aab_impl,
)

_generate_apks_and_install = rule(
_generate_universal_apk = rule(
attrs = {
"input_file": attr.label(
allow_single_file = True,
"input_aab_file": attr.label(
allow_single_file = [".aab"],
mandatory = True,
),
"output_apk_file": attr.output(
mandatory = True,
),
"debug_keystore": attr.label(
Expand All @@ -319,14 +318,19 @@ _generate_apks_and_install = rule(
default = "//third_party:android_bundletool_binary",
),
},
executable = True,
implementation = _generate_apks_and_install_impl,
implementation = _generate_universal_apk_impl,
)

def oppia_android_application(name, config_file, proguard_generate_mapping, **kwargs):
"""
Creates an Android App Bundle (AAB) binary with the specified name and arguments.
This generates a mobile-installable target that ends in '_binary'. For example, if there's an
Oppia Android application defined with the name 'oppia_dev' then its APK binary can be
mobile-installed using:
bazel mobile-install //:oppia_dev_binary
Args:
name: str. The name of the Android App Bundle to build. This will corresponding to the name
of the generated .aab file.
Expand Down Expand Up @@ -390,30 +394,30 @@ def oppia_android_application(name, config_file, proguard_generate_mapping, **kw
tags = ["manual"],
)

def declare_deployable_application(name, aab_target):
def generate_universal_apk(name, aab_target):
"""
Creates a new target that can be run with 'bazel run' to install an AAB file.
Creates a new installable universal APK target for the provided AAB target.
This new target is installable via 'adb install'. This should generally never be used for direct
development installs as it will be far less performant than 'bazel mobile-install'.
Comment on lines +401 to +402
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this ambiguous, it is not clear to me whether the warning is against using 'adb install' or bazel mobile-install //:oppia_prod_universal_apk

Example:
declare_deployable_application(
name = "install_oppia_prod",
generate_universal_apk(
name = "oppia_prod_universal_apk",
aab_target = "//:oppia_prod",
)
$ bazel run //:install_oppia_prod
This will build (if necessary) and install the correct APK derived from the Android app bundle
on the locally attached emulator or device. Note that this command does not support targeting a
specific device so it will not work if more than one device is available via 'adb devices'.
$ bazel mobile-install //:oppia_prod_universal_apk
Args:
name: str. The name of the runnable target to install an AAB file on a local device.
aab_target: target. The target (declared via oppia_android_application) that should be made
installable.
"""
_generate_apks_and_install(
_generate_universal_apk(
name = name,
input_file = aab_target,
input_aab_file = aab_target,
output_apk_file = "%s.apk" % name,
debug_keystore = "@bazel_tools//tools/android:debug_keystore",
tags = ["manual"],
)
4 changes: 3 additions & 1 deletion wiki/Bazel-Setup-Instructions-for-Linux.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,7 @@ INFO: Build completed successfully, ...
Note also that the ``oppia_dev.aab`` under the ``bazel-bin`` directory of your local copy of Oppia Android should be a fully functioning development version of the app that can be installed using bundle-tool. However, it's recommended to deploy Oppia to an emulator or connected device using the following Bazel command:

```sh
bazel run //:install_oppia_dev
bazel mobile-install //:oppia_dev_binary
```

``mobile-install`` is much faster for local development (especially for the developer flavor of the app) because it does more sophisticated dex regeneration detection for faster incremental installs. See https://bazel.build/docs/mobile-install for details.
4 changes: 3 additions & 1 deletion wiki/Bazel-Setup-Instructions-for-Mac.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,7 @@ INFO: Build completed successfully, ...
Note also that the ``oppia_dev.aab`` under the ``bazel-bin`` directory of your local copy of Oppia Android should be a fully functioning development version of the app that can be installed using bundle-tool. However, it's recommended to deploy Oppia to an emulator or connected device using the following Bazel command:

```sh
bazel run //:install_oppia_dev
bazel mobile-install //:oppia_dev_binary
```

``mobile-install`` is much faster for local development (especially for the developer flavor of the app) because it does more sophisticated dex regeneration detection for faster incremental installs. See https://bazel.build/docs/mobile-install for details.
Loading