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

Support overriding debuggable in AndroidManifest #13801

Conversation

benjaminRomano
Copy link
Contributor

@benjaminRomano benjaminRomano commented Aug 4, 2021

Background
I would like to add debuggable="false" into the AndroidManifest.xml, but specifying this directly in the AndroidManifest.xml does not take affect as --debug-mode is passed into aapt2 which overrides what developers set. ref

The only way to get a debuggable apk is to require --compilation-mode opt which will invalidate the entire dep graph which is not feasible for large apps.
#2575

Changes

  • Add if debuggable: "false" is specified in manifestValues of the android_binary target do not set --debug-mode true during resource processor step

Usage

android_binary(
    name = "app",
    dex_shards = 10,
    manifest = "src/bazel/AndroidManifest.xml",
    manifest_values = {
        "minSdkVersion": "19",
        "appName": "...",
        "applicationId": "...",
        "debuggable": "false",
    },
    multidex = "native",
    visibility = ["//visibility:public"],
    deps = ["..."],
)

@ahumesky
Copy link
Contributor

The way that the final "debuggable" value gets determined currently and with this change was fiddly enough that I made some tables. (I mostly reasoned about this in my head looking through the code, so it might not be perfect.)

Current:

--compilation_mode manifest_values debuggable AndroidManifest.xml debuggable final debuggable value
dbg/fastbuild (default) X X True
opt X False False
opt X True True
opt False (with substitution) X False
opt True (with substitution) X True

With this change:

--compilation_mode manifest_values debuggable AndroidManifest.xml debuggable final debuggable value
dbg/fastbuild False True True
dbg/fastbuild False False False
dbg/fastbuild True X True
dbg/fastbuild not set X True
opt X False False
opt X True True
opt False (with substitution) X False
opt True (with substitution) X True

(X = "don't care")

So the first thing is that it seems more difficult to reason about what's going on with this change, because there are more cases to think about.

The second is that it might not be trivial to release this change internally at Google, because I'm finding apps that might be affected by this (e.g., they have manifest_values = { "debuggable": "false" }, and that might not be doing anything right now, and with this change it might start doing something).

I think what would be preferable is to add a flag that just takes --compilation_mode (and aapt2's --debug-mode) out of the equation. Something like --android_opt_compilation_mode_sets_debuggable that can be put in a bazelrc. That way, the final value is simply either the value that's already in the manifest itself, or from the build file via value substitution:

BUILD:
manifest_values = {
   "debuggable": "true",
}

AndroidManifest.xml:
android:debuggable="${debuggable}"

It would also be possible to make this configurable on the command line:

load("@bazel_skylib//lib:dicts.bzl", "dicts")

config_setting(
  name = "android_debuggable",
  values = {
    "define": "android_debuggable=true",
  }
)

manifest_values = {
  "applicationId": "com.foo.app",
}

android_binary(
    name = "app",
    manifest = "AndroidManifest.xml",
    deps = [
        ":applib",
    ],
    manifest_values = select({
        ":android_debuggable": dicts.add(manifest_values, {
            "debuggable": "true",
        }),
        "//conditions:default": dicts.add(manifest_values, {
            "debuggable": "false",
        }),
    }),
)

@ahumesky ahumesky added the team-Android Issues for Android team label Apr 15, 2022
@sgowroji sgowroji added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 22, 2022
@ahumesky ahumesky closed this Aug 17, 2023
@ahumesky
Copy link
Contributor

ahumesky commented Aug 17, 2023

If there's still any interest in pursuing this, happy to reopen.

@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Aug 17, 2023
EdbertChan added a commit to EdbertChan/rules_android that referenced this pull request Jun 5, 2024
@EdbertChan
Copy link

@ahumesky Had a chat w Corbin about this actually.

bazelbuild/rules_android#238 (comment)

Copy pasting the original for convenience.

Per @restingbull (Corbin) offline conversation, the dependency on Compilation.OPT was a matter of convenience from inside of Google. Supposedly, few/nobody who was working on building with the debuggable flag build cared about release so full invalidation was not a problem.

If you weren't building an optimized binary, you should not be building a release anyway since release should be optimized.

Outside of Google, that may not be the case. Some developers very specifically want to work on performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants