Pass correct parameters to gradle build #856

rosen-vladimirov · 2015-09-02T20:17:30Z

Pass correct parameters to gradle build

We should pass correct parameters to android build. We need:

Compile SDK - at least 21
BuildTools version - at least 22
AppCompat lib (Android Support) version - same as compile SDK
Target SDK - the selected target by user - there's no need to have it

Add AndroidToolsInfo class that calculates all required and installed versions of required Android tools. This class prints warnings when the public method getToolsInfo is called without parameters and prints errors when the showWarningsAsErrors parameter is set to true. When tns doctor command is called, the checks will print warnings and when tns platform add android or tns build/prepare... android commands are used, the AndroidToolsInfo will break the execution when incorrect Android setup is detected.

Fixes #840

ns-bot · 2015-09-02T20:21:13Z

✅

teobugslayer · 2015-09-03T12:49:01Z

docs/man_pages/project/testing/build-android.md


-<% if(isHtml) { %> 
+### Attributes
+* `<API Level>` is a valid Android API level. For example: 17, 19, MNC.<% if(isHtml) { %> For a complete list of the Android API levels and their corresponding Android versions, click [here](http://developer.android.com/guide/topics/manifest/uses-sdk-element.html#platform).<% } %>


We would not support MNC, perhaps it is best to avoid mentioning it?

ns-bot · 2015-09-03T15:27:23Z

✅

rosen-vladimirov · 2015-09-03T15:28:04Z

lib/services/android-project-service.ts

+				let newTarget = this.$options.sdk ? `${AndroidProjectService.ANDROID_TARGET_PREFIX}-${this.$options.sdk}` : targetFromAndroidManifest ? `${AndroidProjectService.ANDROID_TARGET_PREFIX}-${targetFromAndroidManifest}`: this.getLatestValidAndroidTarget().wait();
+				this.$logger.trace(`TargetSDK from AndroidManifest is ${targetFromAndroidManifest}. Selected target is: ${newTarget}.`);
+				if(!_.contains(this.SUPPORTED_TARGETS, newTarget)) {
+					let versionNumber = parseInt(_.last(newTarget.split("-")));


use parseAndroidSdkString method

ns-bot · 2015-09-05T14:03:33Z

✅

ns-bot · 2015-09-05T14:09:01Z

✅

ns-bot · 2015-09-07T06:26:21Z

✅

Mitko-Kerezov · 2015-09-07T06:34:54Z

lib/android-tools-info.ts

+"use strict";
+
+import path = require("path");
+import semver = require("semver");


ES6 Imports can be used here

rosen-vladimirov · 2015-09-07T06:39:31Z

ping @ikoevska for the messages

ns-bot · 2015-09-07T07:15:10Z

✅

Mitko-Kerezov · 2015-09-07T07:49:00Z

👍

teobugslayer · 2015-09-07T08:05:17Z

lib/android-tools-info.ts

+		if(additionalMsg) {
+			this.$logger.info(additionalMsg);
+		}
+	}


Perhaps

if (this.showWarningsAsErrors) { this.$errors.failWithoutHelp(msg); } else { this.$logger.warn(msg); } if(additionalMsg) { this.$logger.info(additionalMsg); }

or I am missing something?

you are correct :)

teobugslayer · 2015-09-07T08:09:39Z

lib/android-tools-info.ts

+
+	private getAndroidHome(): string {
+		let androidHomeEnvVar = process.env["ANDROID_HOME"];
+		if(!androidHomeEnvVar) {


Wouldn't this message be printed twice?

ns-bot · 2015-09-07T08:18:56Z

✅

teobugslayer · 2015-09-07T08:19:44Z

lib/declarations.ts

+	/**
+	 * The Android targetSdkVersion specified by the user.
+	 * In case it is not specified, compileSdkVersion will be used for targetSdkVersion.
+	 */


Comments are totally awesome!

Fatme · 2015-09-07T08:58:40Z

lib/android-tools-info.ts

+				let minSupportedVersion = this.parseAndroidSdkString(_.first(supportedVersions));
+				let maxSupportedVersion = this.parseAndroidSdkString(_.last(supportedVersions));
+				if(targetSdk && (targetSdk < minSupportedVersion)) {
+					this.printMessage(`The selected target SDK ${newTarget} is not supported. You should target at least ${minSupportedVersion}.`);


Shouldn't this be errors.failWithoutHelp?

In one case yes - in other - not :D
Check the description of the PR

ns-bot · 2015-09-07T11:10:26Z

✅

rosen-vladimirov · 2015-09-07T11:11:46Z

lib/android-tools-info.ts

+		private $logger: ILogger,
+		private $options: IOptions) {}
+
+	public getToolsInfo(options?: {showWarningsAsErrors: boolean}): IFuture<IAndroidToolsInfoData> {


no need of showWarningsAsErrors, I'll remove it

teobugslayer · 2015-09-07T11:34:27Z

👍 for the code

ikoevska · 2015-09-07T11:34:57Z

lib/android-tools-info.ts

+					if(targetSdk && (targetSdk < minSupportedVersion)) {
+						this.printMessage(`The selected target SDK ${newTarget} is not supported. You should target at least ${minSupportedVersion}.`);
+					} else if(!targetSdk || targetSdk > this.getMaxSupportedVersion()) {
+						this.$logger.warn(`The selected Android target '${newTarget}' is not verified as supported. Some functionality may not work as expected.`);


Support for the selected Android target SDK ${newTarget} is not verified. Your Android app might not work as expected.

We should pass correct parameters to android build. We need: * Compile SDK - at least 21 * BuildTools version - at least 22 * AppCompat lib (Android Support) version - same as compile SDK * Target SDK - the selected target by user - there's no need to have it Add AndroidToolsInfo class that calculates all required and installed versions of required Android tools. This class prints warnings when the public method getToolsInfo is called without parameters and prints errors when the showWarningsAsErrors parameter is set to true. When `tns doctor` command is called, the checks will print warnings and when `tns platform add android` or `tns build/prepare... android` commands are used, the AndroidToolsInfo will break the execution when incorrect Android setup is detected. Fixes #840

ns-bot · 2015-09-07T11:57:35Z

✅

…param Pass correct parameters to gradle build

rosen-vladimirov added the bug label Sep 2, 2015

rosen-vladimirov self-assigned this Sep 2, 2015

rosen-vladimirov added this to the 1.3.0 milestone Sep 2, 2015

teobugslayer reviewed Sep 3, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from 33da0ad to cf3f6f6 Compare September 3, 2015 15:23

rosen-vladimirov changed the title ~~Save the targetSdk in AndroidManifest.xml~~ Pass correct parameters to gradle build Sep 3, 2015

rosen-vladimirov reviewed Sep 3, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from cf3f6f6 to c428bc0 Compare September 5, 2015 14:00

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from c428bc0 to a67862d Compare September 5, 2015 14:06

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from a67862d to 6f0d343 Compare September 7, 2015 06:22

Mitko-Kerezov reviewed Sep 7, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from 6f0d343 to 308b3f3 Compare September 7, 2015 07:02

teobugslayer reviewed Sep 7, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from 308b3f3 to 8e886a5 Compare September 7, 2015 08:09

teobugslayer reviewed Sep 7, 2015
View reviewed changes

Fatme reviewed Sep 7, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from 8e886a5 to 671802d Compare September 7, 2015 11:07

rosen-vladimirov reviewed Sep 7, 2015
View reviewed changes

ikoevska reviewed Sep 7, 2015
View reviewed changes

rosen-vladimirov force-pushed the vladimirov/fix-android-sdk-param branch from 671802d to 23bdee0 Compare September 7, 2015 11:50

rosen-vladimirov added a commit that referenced this pull request Sep 7, 2015

Merge pull request #856 from NativeScript/vladimirov/fix-android-sdk-…

f7b0c13

…param Pass correct parameters to gradle build

rosen-vladimirov merged commit f7b0c13 into master Sep 7, 2015

rosen-vladimirov deleted the vladimirov/fix-android-sdk-param branch September 7, 2015 11:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass correct parameters to gradle build #856

Pass correct parameters to gradle build #856

rosen-vladimirov commented Sep 2, 2015

ns-bot commented Sep 2, 2015

teobugslayer Sep 3, 2015

ns-bot commented Sep 3, 2015

rosen-vladimirov Sep 3, 2015

ns-bot commented Sep 5, 2015

ns-bot commented Sep 5, 2015

ns-bot commented Sep 7, 2015

Mitko-Kerezov Sep 7, 2015

rosen-vladimirov commented Sep 7, 2015

ns-bot commented Sep 7, 2015

Mitko-Kerezov commented Sep 7, 2015

teobugslayer Sep 7, 2015

rosen-vladimirov Sep 7, 2015

teobugslayer Sep 7, 2015

ns-bot commented Sep 7, 2015

teobugslayer Sep 7, 2015

Fatme Sep 7, 2015

rosen-vladimirov Sep 7, 2015

ns-bot commented Sep 7, 2015

rosen-vladimirov Sep 7, 2015

teobugslayer commented Sep 7, 2015

ikoevska Sep 7, 2015

ns-bot commented Sep 7, 2015

Pass correct parameters to gradle build #856

Pass correct parameters to gradle build #856

Conversation

rosen-vladimirov commented Sep 2, 2015

ns-bot commented Sep 2, 2015

Choose a reason for hiding this comment

ns-bot commented Sep 3, 2015

Choose a reason for hiding this comment

ns-bot commented Sep 5, 2015

ns-bot commented Sep 5, 2015

ns-bot commented Sep 7, 2015

Choose a reason for hiding this comment

rosen-vladimirov commented Sep 7, 2015

ns-bot commented Sep 7, 2015

Mitko-Kerezov commented Sep 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ns-bot commented Sep 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ns-bot commented Sep 7, 2015

Choose a reason for hiding this comment

teobugslayer commented Sep 7, 2015

Choose a reason for hiding this comment

ns-bot commented Sep 7, 2015