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

refactor(android): build with gradle #11339

Merged
merged 45 commits into from Dec 20, 2019
Merged

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Nov 14, 2019

JIRA:

Hyperloop Warning:
This PR will temporarily break hyperloop. Once this PR has been merged, we can then rebuild hyperloop module with gradle via the following private hyperloop PR...
tidev/hyperloop.next#329.

App Build Time Improvements:
Benchmarked with kitchensink-v2 on a 15" MacBook Pro 2015 with a solid state drive.

Non-encrypted emulator builds:
- Clean Build: 52s -> 20s  (2.6x faster)
- Incremental: 18s -> 7s   (2.6x faster)

Encrypted device/production builds:
- Clean Build: 53s -> 21s  (2.5x faster)
- Incremental: 50s -> 14s  (3.6x faster)

Notes:
- Benchmarks exclude gradle daemon startup of ~5s which is a one time hit.
- Titanium did not support encrypted incremental builds before. This is new.
- App gradle builds will be faster once we rebuild modules as AARs.

Summary of Changes:

  • Replaced "ant" tool usage with "gradle" for SDK, app, and module builds.
  • Updated min Android build-tools requirement from 25.0.0 to 28.0.3.
  • Titanium SDK Library:
    • Now built as a single AAR instead of individual JARs.
    • Can now easily open titanium_mobile/android directory in Android Studio.
    • Replaced titanium_mobile/android project TitaniumTest with app.
    • Supports building debug version of C/C++ .so libraries for asserts and debug logging.
    • Removed all Google JARs and res files and replaced with gradle/maven references.
    • Transpile/polyfill/rollup/snapshot of "ti.main.js" common folder is now done as a pre-build step of "titanium" library. Supports incremental builds when JS files change.
    • Jenkins/node-scons builds will now fail if unable to acquire V8 snapshot.
  • Apps:
    • Significantly improved build times. Now supports incremental device/production builds.
    • Now automatically digitally signs APK with signature scheme v2.
    • Now merges module/app "AndroidManifest.xml" files via gradle.
    • Now supports "AndroidManifest.xml" merge tools: attributes. See link
    • Added support for ./platform/android/build.gradle file for defining dependencies.
    • Build no longer post-fixes hashes to app's res/drawable image files. Now stored as-is.
    • Building for "development" or "test" now builds a debug version. Can be used to debug modules and SDK via Android Studio.
    • Added new build hook event build.android.requestResourcesDirPaths. To be used by plugins/modules (such as hyperloop) to add additional "Resources" files to app build.
  • Modules:
    • Are now built as AARs and installed as local maven repos for version management.
    • Modules built with 9.0.0 are not backward compatible. Must set "minsdk" to 9.0.0.
    • Now supports "Resources" folder whose files are copied into app as-is (like iOS).
    • Added support for ./platform/android/AndroidManifest.xml file.
  • Dropped support for appc config option android.mergeCustomAndroidManifest.
    • Must use XML tools:remove or tools:removeAll attributes instead which is Google's equivalent feature.

KitchenSink Test:
Test the below on Mac and Windows.

  1. Download the kitchensink-v2 app.
  2. Change its "tiapp.xml" file to reference this PR's SDK version.
  3. Build and run to the Android emulator.
  4. Verify app's top and bottom bars are RED. (Indicates custom theme XML file works.)
  5. Tap through all of the app's UI, verify that everything works. Particular Maps.
  6. Build and run to the emulator again. (Tests increment build.)
  7. Tap through all of the app's UI again.
  8. Run it on emulator via Appc Studio debugger. Verify break-points work.
  9. Run it on emulator via Appc Studio Live-View feature. Verify JS changes re-launches app.
  10. Build and run to an Android device. (Tests encrypted JS builds.)
  11. Tap through all of the app's UI again.
  12. Build and run to the device again. (Tests incremental encrypted builds.)
  13. Tap through all of the app's UI again.
  14. Run it on device via Appc Studio debugger. Verify break-points work.
  15. Run it on device via Appc Studio Live-View feature. Verify JS changes re-launches app.
  16. Build app for production. (This requires a keystore file for signing app.)
  17. Uninstall app from device/emultor.
  18. Install and run production app on device/emulator.
  19. Tap through all of the app's UI again.

APK Signature Scheme v2 Test:

  1. Following the test procedure attached to TIMOB-24517.
  2. Verify command line tool returns true for both v1 and v2 signatures.
    (Note that v3 should be false. Not currently supported.)

"AndroidManifest.xml" Error Handling Test:

  1. Follow test procedure attached to TIMOB-27325.
  2. Build the app.
  3. Verify that app build fails and it logs what's wrong with the XML. (This is a good thing.)

Custom "build.gradle" Test:

  1. Create a Classic app.
  2. Copy the below app.js to the project.
  3. Copy the below build.gradle file to project's ./platform/android folder.
  4. Build and run on Android.
  5. Verify black "play" button is shown onscreen. (Comes from gradle file's referenced library.)
  6. In build log, verify you see message: @@@ [build.gradle] JavaCompile.doFirst

./Resources/app.js

var window = Ti.UI.createWindow({
	backgroundColor: "white",
});
window.add(Ti.UI.createImageView({
	image: "android.resource://" + Ti.App.id + "/drawable/exo_notification_small_icon"
}));
window.open();

./platform/android/build.gradle

repositories {
	google()
	jcenter()
}
tasks.withType(JavaCompile) {
	doFirst {
		println('@@@ [build.gradle] JavaCompile.doFirst')
	}
}
dependencies {
	implementation 'com.google.android.exoplayer:exoplayer-ui:2.8.0'
}

- Replaced "ant" with "gradle" when building the Titanium SDK and Titanium apps.
- Dropped "ant" build tool requirement. No longer used.
- Updated min Android build-tools requirement from 25.0.0 to 28.0.3.
  * This is the min version required by Android gradle tool.
- Now building Titanium as an AAR instead of individual JARs.
  * Core modules no longer built as separate JARs, but 1 monolitch JAR internally.
- Replaced SDK's internal "android/TitaniumTest" project with "android/app" project.
- Removed all Google library JARs and res files and replaced with gradle/maven repo references.
- Titanium's generated "build.properties" moved from JAR resources to APK assets.
- No longer post-fixing hash to app's res/drawable image files. Now stored/loaded as-is.
- Removed res/values/strings.xml key "app_name" from all Titanium libraries.
  * This should only be defined by the app project.
- Optimized @Kroll annotation process "kroll-apt".
  * Now only executed once during SDK build. (Old behavior loaded it 3 times.)
  * Now generates C++ V8 binding files in addition to binding JSON file.
- Fixed TiUILabel to not access private API isSingleLine() when targeting Android Q.
- Fixed C++ compiler errors and asserts when doing a "debug" build.
- Removed "-a" API Level command line arg support from "node scons" build tools.
  * API Level is configured in "android/titanium/build.gradle" file.
- Removed node module dependencies:
  * appc-aar-tools
  * archiver
- C/C++ makefile would fail during a clean if V8 library hasn't be downloaded yet.
- Now downloads V8 when doing a "clean" or "build".
- Rewrote AndroidManifest.xml file merge handling.
  * Deleted "android/cli/lib/AndroidManifest.js" file.
  * Replaced with new "android/cli/lib/android-manifest.js" file.
  * Now blindly adds any XML element/attribute app developer sets. We no longer filter out nodes we don't know. (Future proof.)
  * Now supports "tools:" attributes to replace/remove XML elements/attributes.
  * Now auto-adds "tools:replace" attributes if "tiapp.xml" file's attributes collides with library's defined attributes. Avoids build failures.
  * Now preserves order of XML elements defined by app developer. This is important when using "tools:remove" feature.
  * Now inserts/merges <uses-permission/> elements above <application/> element to avoid gradle linting warning.
- Added new "ti.constants.gradle" file:
  * Defines library versions used by Titanium. To be referenced by SDK builds, app builds, and module builds.
  * Defines manifest placeholder "tiActivityConfigChanges" which populates "android:configChanges" in <activity/> element.
  * Used by SDK builds, app builds, and module builds.
- Added support for legacy manifest placeholder ${localApplicationId}.
  * Only used by "ti.zdetection" modules. Needed or else a gradle build failure will occur.
- Improved error handling of build. Especially for unhandled promise rejections.
- Modified some Android app build tasks to be done in parallel via Promise.all().
- Dropped support for appc config option "android.mergeCustomAndroidManifest".
  * Must use XML "tools:remove" or "tools:removeAll" attributes instead which is Google's equivalent feature.
- Added Android module "Resources" folder support.
  * Copies these files to APK "assets/Resources" folder.
- Fixed app build failures when module manifest field contains double quotes. (Now properly escaped.)
- Change was lost while merging with gradle changes. Re-integrated the feature.
appc.subprocess.run('appc', [ '-q', 'config', 'get', 'proxyServer' ], runOptions, (exitCode, out) => {
try {
if (!exitCode && out && (out.length > 0)) {
proxyUrl = url.parse(out.trim());
Copy link
Contributor

Choose a reason for hiding this comment

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

  • ⚠️ android/cli/lib/gradle-wrapper.js line 214 – 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead. (node/no-deprecated-api)

@build
Copy link
Contributor

build commented Nov 14, 2019

Warnings
⚠️

Changes were made to package.json, but not to package-lock.json.
Perhaps you need to run npm install?

Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 6550 tests are passing.
(There are 700 skipped tests not included in that total)

Dependencies removed: appc-aar-tools.

Generated by 🚫 dangerJS against ee08945

jquick-axway and others added 9 commits November 14, 2019 16:24
- Jenkins build provides Android SDK/NDK paths when doing "node scons build", but not for "node scons package". Was causing build failure.
- Modified to be more lenient and allow build to continue if Android "local.properties" file was already generated. (Logs warning instead.)
- Appcelerator Studio is hardcoded to find JS file in "build/android/assets" directory when debugging an Alloy project.
  * Caused by changing "assets" directory to "ti-assets" for gradle. Changed it back to "assets".
- Restored old behavior where we did not allow app dev's "AndroidManifest.xml" to override a Titanium <activity/> "android:configChanges" attribute.
  * Change was lost when changing build system to gradle.
- Now extracts Titanium C/C++ *.so libraries from AAR after gradle publishing step.
  * Unfortunately, Google does not provide an official means of changing the output directory for built *.so files.
@sgtcoolguy
Copy link
Contributor

sgtcoolguy commented Nov 19, 2019

So to review what we discussed on Teams/call. There's a mismatch in that the app's build.gradle file is specifying a dependency on org.appcelerator:titanium:9.0.0 but the AAR is actually versioned something like 9.0.0v201911191234567. The options are to:

  • update the generation of the app's build.gradle to inject the proper version (using something like this.titaniumSdkName in place of this.titaniumSdkVersion). If we go down this road, the AAR has the very specific version, but we'd likely have to alter our release scripts to manipulate that version in the AAR/POM/etc to the short GA name (like 9.0.0.GA) since our release takes an existing built sdk zip and alters the internal folder name (i.e. we don't rebuild from source with a special versionTag).
  • generate AARs with the "base" version without the tag/qualifier on it. (i.e. 9.0.0). The AAR won't have the fully qualified version number in it, but then I don't think we'd need to alter any of the AAR/POM/etc when we release.

Jenkinsfile Outdated Show resolved Hide resolved
- iOS' logged errors during clean was causing Jenkins to fail the build. Now ignored.
- C/C++ makefile $(shell) command fails on Windows if built with "-n" dry-run flag.
  * Dry-run does not actually build anything and only fetches tasks to be completed. So, modified to skip fetch V8 source/lib files.
  * Thanks goes to Gary Mathews for isolating this.
- Added new environment variable TI_SDK_BUILD_REQUIRES_V8_SNAPSHOTS.
  * When set, will purposely trigger a build failure if failed to generate V8 snapshot C++ header file. (No fallbacks.)
  * Set by SDK build scripts under "titanium_mobile/build" directory.
  * Idea is to not let Jenkins release SDK builds missing V8 snapshots. But okay to use fallback for local Android Studio builds.
- Cleaned up build scripts.
- No longer needed due to TIMOB-26910 which moved them to APK "assets" or "res" folders instead as an optimization.
lokeshchdhry and others added 10 commits December 16, 2019 08:09
- Now checks if environment variable TI_SDK_BUILD_REQUIRES_V8_SNAPSHOTS before "node scons" build overwrites it.
- Would cause build failures if target API Level was less than 26.
- Will use "en-US" language fallback in case "en" is missing. Minimizes gradle build warnings.
- Titanium's "kroll-apt.jar" annotation processor was not accepting all Kroll annotations defined, causing harmless errors to be logged.
- Removed usage of $(shell) in makefile which caused build failures on Windows for dry-run builds.
  * Dry-runs are needed to make C/C++ intellisense work in Android Studio. Used to pick up headers and libraries.
- Now generates a git-ignored "V8Settings.mk" file storing version/mode of V8 to be used. Is included-in by main V8 makefile.
- Added gradle incremental task which updates "V8Settings.mk" when "package.json" V8 settings change or if makefile is missing.
@lokeshchdhry
Copy link
Contributor

Please do not merge until Win android FR is done.

FR Passed for MAC android.

Ran below checks using Studio & CLI

  • Ran tests mentioned above.

  • KS build & runs fine.

  • Ran tests in smoke test & they look good.

  • Checked creation & build of default alloy & classic apps & it looks good.

  • Checked min-sdk & target-sdk in tiapp.xml & works as expected.

  • TIMOB-24517 - Works as expected.

  • TIMOB-27325 - Works as expected.
    If <uses-library> is inside manifest it will throw error now:

[ERROR] :  [GRADLE] FAILURE: Build failed with an exception.
[ERROR] :  [GRADLE] 
[ERROR] :  [GRADLE] * What went wrong:
[ERROR] :  [GRADLE] Execution failed for task ':app:processDebugResources'.
[ERROR] :  [GRADLE] > Android resource linking failed
[ERROR] :  [GRADLE]   /Users/lchoudhary/Desktop/workspaces/workspace_2019/ZZZgradleclassic/build/android/app/build/intermediates/merged_manifests/debug/AndroidManifest.xml:11: AAPT: error: unexpected element <uses-library> found in <manifest>.
[ERROR] :  [GRADLE]       
[ERROR] :  [GRADLE] 
[ERROR] :  [GRADLE] * Try:
[ERROR] :  [GRADLE] Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
[ERROR] :  [GRADLE] 
[ERROR] :  [GRADLE] * Get more help at https://help.gradle.org
[ERROR] :  [GRADLE] 
[ERROR] :  [GRADLE] BUILD FAILED in 3s
  • TIMOB-25240 - Incremental builds are significantly faster now.

  • Checked if modules can be created & build successfully with gradle via CLI & Studio. Looks good via CLI found an issue with studio: https://jira.appcelerator.org/browse/TISTUD-9204.

  • Checked if old modules built with non gradle SDK i.e <9.0 work with 9.0 for backward compatibility & they work fine.

  • Checked modules created with 9.0 have min-sdk as 9.0 & looks good.

I will add more if I remember something later .......

@sgtcoolguy
Copy link
Contributor

@jquick-axway When this PR is eventually ready to merge, what is your expectation for the merge itself? i.e. do you plan to do a lot of rebasing locally to clean it all up and rebase on top of tip of master branch? Should it all be squashed into a single merge commit? Because I think 44 commits is a bit much.

On larger PRs I tend to push commits until the CR is done and then manually rebase interactively (locally) to squash down to a smaller more coherent set of commits that could be rebased onto master.

@jquick-axway
Copy link
Contributor Author

@sgtcoolguy, I'd rather we merge it as-is. I'm not a fan of squashing commits... unless the prior commits were horribly broken (in this case they're not). In fact, I purposely split some of my changes into separate commits so that they can be easily understood from a history/blame perspective.

@ssekhri
Copy link

ssekhri commented Dec 20, 2019

FR Passed on windows machine as well running both android device and emulators.
Performed the tests as mentioned by @lokeshchdhry for mac testing.

@lokeshchdhry lokeshchdhry merged commit f687e3b into tidev:master Dec 20, 2019
@drauggres
Copy link
Contributor

@lokeshchdhry
cc @jquick-axway

FYI kitchensink-v2 is failed to build on linux:

Error: ENOENT: no such file or directory, open '<TI_SDK_PATH>/modules/android/facebook/8.0.0/facebook.jar'

because the file name is actually Facebook.jar

@jquick-axway
Copy link
Contributor Author

jquick-axway commented Jan 7, 2020

@drauggres, yeah, I see the issue. Thanks. From looking at the Titanium 8_3_X branch via blame, it looks like this has been an issue for over 2 years. I can see our build script doing a toLowerCase() on the JAR filename in the link below (and I blindly followed it).
https://github.com/appcelerator/titanium_mobile/blob/243afd00e0760f2060e797312942ee65d47b9f5f/android/cli/commands/_build.js#L1575

The difference was that the old build script would log the following warning and carry on. The negative impact being that the Facebook APIs won't work since it'll fail to include the JAR. (This is bad.)

[WARN]  Module facebook version 8.0.0 does not have a main jar file

Our new build script rightfully logs it as an error now.
I wrote up a ticket for it here: TIMOB-27706
Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants