Skip to content

Commit

Permalink
[build] Remove <uses-sdk> and specify [min|target|max]SdkVersion in GN
Browse files Browse the repository at this point in the history
GN already has the concept of a minSdkVersion. This CL adds target- and
maxSdkVersion and makes GN the source of truth by removing the
<uses-sdk> element from all manifests. There are three steps that
require [min|target|max]SdkVersion: linting, manifest merging and
resource compilation. Each of these steps get the versions passed from
GN. Furthermore, manifest merging removes the SDK version from the
output manifest so that GN stays the source of truth.

+ Add a manifest_utils to unify manifest parsing, rewriting, etc. in a
  single place.

+ Move manifest verification to compile resources steps, where the final
  manifest is written.

This CL adds some temporary workarounds to keep downstream working.
Those will be removed once downstream is updated.

Bug: 891996
Change-Id: If24be69d9c6b03a0d2423fad830a4da375d68505
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1650265
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Simeon Anfinrud <sanfin@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669810}
  • Loading branch information
Tibor Goldschwendt authored and Commit Bot committed Jun 17, 2019
1 parent d9e2ff8 commit 95db95d
Show file tree
Hide file tree
Showing 92 changed files with 439 additions and 323 deletions.
4 changes: 4 additions & 0 deletions android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,7 @@ android_library("android_webview_java") {
]

android_manifest_for_lint = system_webview_android_manifest
min_sdk_version = 21
}

android_library("android_webview_variations_utils_java") {
Expand Down Expand Up @@ -1088,6 +1089,7 @@ android_library("android_webview_services_java") {
":aw_variations_seed_server_aidl",
]
android_manifest_for_lint = system_webview_android_manifest
min_sdk_version = 21
}

android_aidl("crash_receiver_aidl") {
Expand All @@ -1110,6 +1112,8 @@ if (public_android_sdk) {
android_manifest_dep = ":system_webview_manifest"
deps = upstream_only_webview_deps
apk_name = "SystemWebView"
min_sdk_version = 21
target_sdk_version = 28
}

android_resource_sizes_test("resource_sizes_system_webview_apk") {
Expand Down
6 changes: 0 additions & 6 deletions android_webview/apk/java/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@
xmlns:tools="http://schemas.android.com/tools"
package="{{manifest_package|default('com.android.webview')}}"
tools:ignore="MissingLeanbackLauncher">
<uses-sdk android:minSdkVersion="{{min_sdk_version|default(21)}}"
android:targetSdkVersion="{{target_sdk_version|default(28)}}"
{% if max_sdk_version is defined %}
android:maxSdkVersion="{{max_sdk_version}}"
{% endif %}>
</uses-sdk>

<uses-feature android:name="android.hardware.touchscreen"
android:required="false" />
Expand Down
1 change: 1 addition & 0 deletions android_webview/glue/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,5 @@ android_library("glue") {
]

android_manifest_for_lint = system_webview_android_manifest
min_sdk_version = 21
}
1 change: 0 additions & 1 deletion android_webview/javatests/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
in the LICENSE file. -->
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.chromium.android_webview.test">
<uses-sdk android:minSdkVersion="21" android:targetSdkVersion="23" />
<instrumentation android:name="org.chromium.base.test.BaseChromiumAndroidJUnitRunner"
android:targetPackage="org.chromium.android_webview.shell"
android:label="Tests for org.chromium.android_webview"/>
Expand Down

This file was deleted.

6 changes: 4 additions & 2 deletions android_webview/support_library/boundary_interfaces/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ android_library("boundary_interface_java") {
deps = []

# This is to verify the boundary interfaces compile and lint correctly against
# the minSdkVersion of the webkit support library module.
android_manifest_for_lint = "AndroidManifest.xml"
# the minSdkVersion of the webkit support library module. As the minSdkVersion
# of the support library increases, so should this value. See
# http://crbug.com/828184 for more details.
min_sdk_version = 14
}
1 change: 1 addition & 0 deletions android_webview/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ instrumentation_test_apk("webview_instrumentation_test_apk") {
apk_name = "WebViewInstrumentationTest"
apk_under_test = ":webview_instrumentation_apk"
android_manifest = "../javatests/AndroidManifest.xml"
min_sdk_version = 21
deps = [
"//android_webview:android_webview_java",
"//android_webview:android_webview_platform_services_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
xmlns:tools="http://schemas.android.com/tools"
package="org.chromium.android_webview.test.support">

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.INTERNET"/>

Expand Down
1 change: 0 additions & 1 deletion android_webview/test/shell/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.chromium.android_webview.shell">
<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />

<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
xmlns:android="http://schemas.android.com/apk/res/android"
package="org.chromium.webview_ui_test.test">

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />

<uses-permission android:name="android.permission.RUN_INSTRUMENTATION" />

<!-- We add an application tag here just so that we can indicate that this
Expand Down
1 change: 1 addition & 0 deletions android_webview/tools/system_webview_shell/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ android_apk("system_webview_shell_apk") {
"apk/src/org/chromium/webview_shell/WebViewTracingActivity.java",
]
android_manifest = "apk/AndroidManifest.xml"
target_sdk_version = 28
deps = [
":system_webview_shell_apk_resources",
"//base:base_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="28" />

<!-- "Normal" permissions which do not require user prompt -->
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.ACCESS_WIFI_STATE" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />

<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />

<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.ACCESS_WIFI_STATE" />
<uses-permission android:name="android.permission.RUN_INSTRUMENTATION" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ found in the LICENSE file. -->
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="21" android:targetSdkVersion="28" />

<application
android:label="WebView Log Verbosifier"
android:icon="@mipmap/ic_launcher"
Expand Down
2 changes: 2 additions & 0 deletions android_webview/tools/webview_log_verbosifier/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ android_apk("webview_log_verbosifier_apk") {
deps = [
":webview_log_verbosifier_resources",
]
min_sdk_version = 21
target_sdk_version = 28
}

android_resources("webview_log_verbosifier_resources") {
Expand Down
1 change: 0 additions & 1 deletion base/android/jni_generator/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="org.jni.generator">

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="24" />
<application></application>

</manifest>
1 change: 1 addition & 0 deletions base/android/jni_generator/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ shared_library("jni_sample_lib") {
android_apk("sample_jni_apk") {
apk_name = "SampleJni"
android_manifest = "AndroidManifest.xml"
target_sdk_version = 24
deps = [
":jni_sample_java",
"//base:base_java",
Expand Down
4 changes: 0 additions & 4 deletions build/android/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@
This is a dummy manifest which is required by:
1. aapt when generating R.java in java.gypi:
Nothing in the manifest is used, but it is still required by aapt.
2. lint: [min|target]SdkVersion are required by lint and should
be kept up to date.
-->
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.dummy">

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="24" />

</manifest>
64 changes: 59 additions & 5 deletions build/android/gyp/compile_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
from xml.etree import ElementTree

from util import build_utils
from util import diff_utils
from util import manifest_utils
from util import resource_utils

# Name of environment variable that can be used to force this script to
Expand Down Expand Up @@ -67,6 +69,17 @@ def _ParseArgs(args):
'--aapt2-path', required=True, help='Path to the Android aapt2 tool.')
input_opts.add_argument(
'--android-manifest', required=True, help='AndroidManifest.xml path.')
input_opts.add_argument(
'--android-manifest-expected',
help='Expected contents for the final manifest.')
input_opts.add_argument(
'--android-manifest-normalized', help='Normalized manifest.')
input_opts.add_argument(
'--fail-if-unexpected-android-manifest',
action='store_true',
help=
'Fail if expected manifest contents do not match final manifest contents.'
)
group = input_opts.add_mutually_exclusive_group()
group.add_argument(
'--shared-resources',
Expand Down Expand Up @@ -132,6 +145,14 @@ def _ParseArgs(args):

input_opts.add_argument('--version-code', help='Version code for apk.')
input_opts.add_argument('--version-name', help='Version name for apk.')
input_opts.add_argument(
'--min-sdk-version', required=True, help='android:minSdkVersion for APK.')
input_opts.add_argument(
'--target-sdk-version',
required=True,
help="android:targetSdkVersion for APK.")
input_opts.add_argument(
'--max-sdk-version', help="android:maxSdkVersion for APK.")

input_opts.add_argument(
'--locale-whitelist',
Expand Down Expand Up @@ -435,9 +456,20 @@ def maybe_extract_version(j):
version_code, version_name = successful_extractions.pop()[:2]

debug_manifest_path = os.path.join(temp_dir, 'AndroidManifest.xml')
doc, manifest_node, app_node = resource_utils.ParseAndroidManifest(
doc, manifest_node, app_node = manifest_utils.ParseManifest(
options.android_manifest)

manifest_utils.AssertNoUsesSdk(manifest_node)

uses_sdk_attributes = [
('android:minSdkVersion', options.min_sdk_version),
('android:targetSdkVersion', options.target_sdk_version),
]
if options.max_sdk_version:
uses_sdk_attributes += [('android:maxSdkVersion', options.max_sdk_version)]
uses_sdk_node = manifest_utils.MakeElement('uses-sdk', uses_sdk_attributes)
manifest_node.insert(0, uses_sdk_node)

manifest_node.set('platformBuildVersionCode', version_code)
manifest_node.set('platformBuildVersionName', version_name)

Expand All @@ -446,15 +478,28 @@ def maybe_extract_version(j):
manifest_node.set('package', options.arsc_package_name)

if options.debuggable:
app_node.set('{%s}%s' % (resource_utils.ANDROID_NAMESPACE, 'debuggable'),
app_node.set('{%s}%s' % (manifest_utils.ANDROID_NAMESPACE, 'debuggable'),
'true')

with open(debug_manifest_path, 'w') as debug_manifest:
debug_manifest.write(ElementTree.tostring(doc.getroot(), encoding='UTF-8'))

manifest_utils.SaveManifest(doc, debug_manifest_path)
return debug_manifest_path, orig_package


def _VerifyManifest(actual_manifest, expected_manifest, normalized_manifest,
fail_if_unexpected_manifest):
with build_utils.AtomicOutput(normalized_manifest) as normalized_output:
normalized_output.write(manifest_utils.NormalizeManifest(actual_manifest))
msg = diff_utils.DiffFileContents(expected_manifest, normalized_manifest)
if msg:
sys.stderr.write("""\
AndroidManifest.xml expectations file needs updating. For details see:
https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md
""")
sys.stderr.write(msg)
if fail_if_unexpected_manifest:
sys.exit(1)


def _ResourceNameFromPath(path):
return os.path.splitext(os.path.basename(path))[0]

Expand Down Expand Up @@ -682,6 +727,10 @@ def _PackageApk(options, build):
'link',
'--auto-add-overlay',
'--no-version-vectors',
'--min-sdk-version',
options.min_sdk_version,
'--target-sdk-version',
options.target_sdk_version,
]

for j in options.include_resources:
Expand Down Expand Up @@ -717,6 +766,11 @@ def _PackageApk(options, build):
options, build.temp_dir)
if options.rename_manifest_package:
desired_manifest_package_name = options.rename_manifest_package
if options.android_manifest_expected:
_VerifyManifest(fixed_manifest, options.android_manifest_expected,
options.android_manifest_normalized,
options.fail_if_unexpected_android_manifest)

link_command += [
'--manifest', fixed_manifest, '--rename-manifest-package',
desired_manifest_package_name
Expand Down
2 changes: 2 additions & 0 deletions build/android/gyp/compile_resources.pydeps
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@
compile_resources.py
util/__init__.py
util/build_utils.py
util/diff_utils.py
util/manifest_utils.py
util/md5_check.py
util/resource_utils.py
Loading

0 comments on commit 95db95d

Please sign in to comment.