Skip to content

Commit fa290bd

Browse files
TsvetanMilanovTsvetanMilanov
committed
Use custom adb wrapper instead of child process directly
The adb commands should be executed through the custom adb wrapper to use the error handling.
1 parent ada7648 commit fa290bd

File tree

7 files changed

+60
-55
lines changed

7 files changed

+60
-55
lines changed

‎lib/android-tools-info.ts

Lines changed: 50 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
2121
private pathToAndroidExecutable: string;
2222
private _androidExecutableName: string;
2323
private get androidExecutableName(): string {
24-
if(!this._androidExecutableName) {
24+
if (!this._androidExecutableName) {
2525
this._androidExecutableName = "android";
26-
if(this.$hostInfo.isWindows) {
26+
if (this.$hostInfo.isWindows) {
2727
this._androidExecutableName += ".bat";
2828
}
2929
}
@@ -36,17 +36,18 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
3636
private $fs: IFileSystem,
3737
private $hostInfo: IHostInfo,
3838
private $logger: ILogger,
39-
private $options: IOptions) {}
39+
private $options: IOptions,
40+
private $adb: Mobile.IAndroidDebugBridge) { }
4041

41-
public getPathToAndroidExecutable(options?: {showWarningsAsErrors: boolean}): IFuture<string> {
42+
public getPathToAndroidExecutable(options?: { showWarningsAsErrors: boolean }): IFuture<string> {
4243
return ((): string => {
43-
if(options) {
44+
if (options) {
4445
this.showWarningsAsErrors = options.showWarningsAsErrors;
4546
}
4647
if (!this.pathToAndroidExecutable) {
47-
if(this.validateAndroidHomeEnvVariable(this.androidHome).wait()) {
48+
if (this.validateAndroidHomeEnvVariable(this.androidHome).wait()) {
4849
let androidPath = path.join(this.androidHome, "tools", this.androidExecutableName);
49-
if(!this.trySetAndroidPath(androidPath).wait() && !this.trySetAndroidPath(this.androidExecutableName).wait()) {
50+
if (!this.trySetAndroidPath(androidPath).wait() && !this.trySetAndroidPath(this.androidExecutableName).wait()) {
5051
this.printMessage(`Unable to find "${this.androidExecutableName}" executable file. Make sure you have set ANDROID_HOME environment variable correctly.`);
5152
}
5253
} else {
@@ -62,15 +63,15 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
6263
return ((): boolean => {
6364
let isAndroidPathCorrect = true;
6465
try {
65-
let result = this.$childProcess.spawnFromEvent(androidPath, ["--help"], "close", {}, {throwError: false}).wait();
66-
if(result && result.stdout) {
66+
let result = this.$adb.executeCommand(["--help"], { returnChildProcess: true }).wait();
67+
if (result && result.stdout) {
6768
this.$logger.trace(result.stdout);
6869
this.pathToAndroidExecutable = androidPath;
6970
} else {
7071
this.$logger.trace(`Unable to find android executable from '${androidPath}'.`);
7172
isAndroidPathCorrect = false;
7273
}
73-
} catch(err) {
74+
} catch (err) {
7475
this.$logger.trace(`Error occurred while checking androidExecutable from '${androidPath}'. ${err.message}`);
7576
isAndroidPathCorrect = false;
7677
}
@@ -81,7 +82,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
8182

8283
public getToolsInfo(): IFuture<IAndroidToolsInfoData> {
8384
return ((): IAndroidToolsInfoData => {
84-
if(!this.toolsInfo) {
85+
if (!this.toolsInfo) {
8586
let infoData: IAndroidToolsInfoData = Object.create(null);
8687
infoData.androidHomeEnvVar = this.androidHome;
8788
infoData.compileSdkVersion = this.getCompileSdk().wait();
@@ -96,57 +97,57 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
9697
}).future<IAndroidToolsInfoData>()();
9798
}
9899

99-
public validateInfo(options?: {showWarningsAsErrors: boolean, validateTargetSdk: boolean}): IFuture<boolean> {
100-
return (() : boolean => {
100+
public validateInfo(options?: { showWarningsAsErrors: boolean, validateTargetSdk: boolean }): IFuture<boolean> {
101+
return ((): boolean => {
101102
let detectedErrors = false;
102103
this.showWarningsAsErrors = options && options.showWarningsAsErrors;
103104
let toolsInfoData = this.getToolsInfo().wait();
104105
let isAndroidHomeValid = this.validateAndroidHomeEnvVariable(toolsInfoData.androidHomeEnvVar).wait();
105-
if(!toolsInfoData.compileSdkVersion) {
106+
if (!toolsInfoData.compileSdkVersion) {
106107
this.printMessage(`Cannot find a compatible Android SDK for compilation. To be able to build for Android, install Android SDK ${AndroidToolsInfo.MIN_REQUIRED_COMPILE_TARGET} or later.`,
107-
"Run `$ android` to manage your Android SDK versions.");
108+
"Run `$ android` to manage your Android SDK versions.");
108109
detectedErrors = true;
109110
}
110111

111-
if(!toolsInfoData.buildToolsVersion) {
112+
if (!toolsInfoData.buildToolsVersion) {
112113
let buildToolsRange = this.getBuildToolsRange();
113114
let versionRangeMatches = buildToolsRange.match(/^.*?([\d\.]+)\s+.*?([\d\.]+)$/);
114115
let message = `You can install any version in the following range: '${buildToolsRange}'.`;
115116

116117
// Improve message in case buildToolsRange is something like: ">=22.0.0 <=22.0.0" - same numbers on both sides
117-
if(versionRangeMatches && versionRangeMatches[1] && versionRangeMatches[2] && versionRangeMatches[1] === versionRangeMatches[2]) {
118+
if (versionRangeMatches && versionRangeMatches[1] && versionRangeMatches[2] && versionRangeMatches[1] === versionRangeMatches[2]) {
118119
message = `You have to install version ${versionRangeMatches[1]}.`;
119120
}
120121

121122
let invalidBuildToolsAdditionalMsg = 'Run `android` from your command-line to install required `Android Build Tools`.';
122-
if(!isAndroidHomeValid) {
123+
if (!isAndroidHomeValid) {
123124
invalidBuildToolsAdditionalMsg += ' In case you already have them installed, make sure `ANDROID_HOME` environment variable is set correctly.';
124125
}
125126

126127
this.printMessage("You need to have the Android SDK Build-tools installed on your system. " + message, invalidBuildToolsAdditionalMsg);
127128
detectedErrors = true;
128129
}
129130

130-
if(!toolsInfoData.supportRepositoryVersion) {
131+
if (!toolsInfoData.supportRepositoryVersion) {
131132
let invalidSupportLibAdditionalMsg = 'Run `$ android` to manage the Local Maven repository for Support Libraries.';
132-
if(!isAndroidHomeValid) {
133+
if (!isAndroidHomeValid) {
133134
invalidSupportLibAdditionalMsg += ' In case you already have it installed, make sure `ANDROID_HOME` environment variable is set correctly.';
134135
}
135136
this.printMessage(`You need to have Android SDK ${AndroidToolsInfo.MIN_REQUIRED_COMPILE_TARGET} or later and the latest Local Maven repository for Support Libraries installed on your system.`, invalidSupportLibAdditionalMsg);
136137
detectedErrors = true;
137138
}
138139

139-
if(options && options.validateTargetSdk) {
140+
if (options && options.validateTargetSdk) {
140141
let targetSdk = toolsInfoData.targetSdkVersion;
141142
let newTarget = `${AndroidToolsInfo.ANDROID_TARGET_PREFIX}-${targetSdk}`;
142-
if(!_.contains(AndroidToolsInfo.SUPPORTED_TARGETS, newTarget)) {
143+
if (!_.contains(AndroidToolsInfo.SUPPORTED_TARGETS, newTarget)) {
143144
let supportedVersions = AndroidToolsInfo.SUPPORTED_TARGETS.sort();
144145
let minSupportedVersion = this.parseAndroidSdkString(_.first(supportedVersions));
145146

146-
if(targetSdk && (targetSdk < minSupportedVersion)) {
147+
if (targetSdk && (targetSdk < minSupportedVersion)) {
147148
this.printMessage(`The selected Android target SDK ${newTarget} is not supported. You must target ${minSupportedVersion} or later.`);
148149
detectedErrors = true;
149-
} else if(!targetSdk || targetSdk > this.getMaxSupportedVersion()) {
150+
} else if (!targetSdk || targetSdk > this.getMaxSupportedVersion()) {
150151
this.$logger.warn(`Support for the selected Android target SDK ${newTarget} is not verified. Your Android app might not work as expected.`);
151152
}
152153
}
@@ -156,18 +157,18 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
156157
}).future<boolean>()();
157158
}
158159

159-
public validateJavacVersion(installedJavaVersion: string, options?: {showWarningsAsErrors: boolean}): IFuture<boolean> {
160+
public validateJavacVersion(installedJavaVersion: string, options?: { showWarningsAsErrors: boolean }): IFuture<boolean> {
160161
return ((): boolean => {
161162
let hasProblemWithJavaVersion = false;
162-
if(options) {
163+
if (options) {
163164
this.showWarningsAsErrors = options.showWarningsAsErrors;
164165
}
165166
let additionalMessage = "You will not be able to build your projects for Android." + EOL
166167
+ "To be able to build for Android, verify that you have installed The Java Development Kit (JDK) and configured it according to system requirements as" + EOL +
167168
" described in https://github.com/NativeScript/nativescript-cli#system-requirements.";
168169
let matchingVersion = (installedJavaVersion || "").match(AndroidToolsInfo.VERSION_REGEX);
169-
if(matchingVersion && matchingVersion[1]) {
170-
if(semver.lt(matchingVersion[1], AndroidToolsInfo.MIN_JAVA_VERSION)) {
170+
if (matchingVersion && matchingVersion[1]) {
171+
if (semver.lt(matchingVersion[1], AndroidToolsInfo.MIN_JAVA_VERSION)) {
171172
hasProblemWithJavaVersion = true;
172173
this.printMessage(`Javac version ${installedJavaVersion} is not supported. You have to install at least ${AndroidToolsInfo.MIN_JAVA_VERSION}.`, additionalMessage);
173174
}
@@ -182,7 +183,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
182183

183184
public getPathToAdbFromAndroidHome(): IFuture<string> {
184185
return (() => {
185-
if(this.androidHome) {
186+
if (this.androidHome) {
186187
let pathToAdb = path.join(this.androidHome, "platform-tools", "adb");
187188
try {
188189
this.$childProcess.execFile(pathToAdb, ["help"]).wait();
@@ -214,29 +215,29 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
214215
this.$logger.warn(msg);
215216
}
216217

217-
if(additionalMsg) {
218+
if (additionalMsg) {
218219
this.$logger.printMarkdown(additionalMsg);
219220
}
220221
}
221222

222223
private getCompileSdk(): IFuture<number> {
223224
return ((): number => {
224-
if(!this.selectedCompileSdk) {
225+
if (!this.selectedCompileSdk) {
225226
let userSpecifiedCompileSdk = this.$options.compileSdk;
226-
if(userSpecifiedCompileSdk) {
227+
if (userSpecifiedCompileSdk) {
227228
let installedTargets = this.getInstalledTargets().wait();
228229
let androidCompileSdk = `${AndroidToolsInfo.ANDROID_TARGET_PREFIX}-${userSpecifiedCompileSdk}`;
229-
if(!_.contains(installedTargets, androidCompileSdk)) {
230+
if (!_.contains(installedTargets, androidCompileSdk)) {
230231
this.$errors.failWithoutHelp(`You have specified '${userSpecifiedCompileSdk}' for compile sdk, but it is not installed on your system.`);
231232
}
232233

233234
this.selectedCompileSdk = userSpecifiedCompileSdk;
234235
} else {
235236
let latestValidAndroidTarget = this.getLatestValidAndroidTarget().wait();
236-
if(latestValidAndroidTarget) {
237+
if (latestValidAndroidTarget) {
237238
let integerVersion = this.parseAndroidSdkString(latestValidAndroidTarget);
238239

239-
if(integerVersion && integerVersion >= AndroidToolsInfo.MIN_REQUIRED_COMPILE_TARGET) {
240+
if (integerVersion && integerVersion >= AndroidToolsInfo.MIN_REQUIRED_COMPILE_TARGET) {
240241
this.selectedCompileSdk = integerVersion;
241242
}
242243
}
@@ -258,14 +259,14 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
258259
private getMatchingDir(pathToDir: string, versionRange: string): IFuture<string> {
259260
return ((): string => {
260261
let selectedVersion: string;
261-
if(this.$fs.exists(pathToDir).wait()) {
262+
if (this.$fs.exists(pathToDir).wait()) {
262263
let subDirs = this.$fs.readDirectory(pathToDir).wait();
263264
this.$logger.trace(`Directories found in ${pathToDir} are ${subDirs.join(", ")}`);
264265

265266
let subDirsVersions = subDirs
266267
.map(dirName => {
267268
let dirNameGroups = dirName.match(AndroidToolsInfo.VERSION_REGEX);
268-
if(dirNameGroups) {
269+
if (dirNameGroups) {
269270
return dirNameGroups[1];
270271
}
271272

@@ -274,7 +275,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
274275
.filter(dirName => !!dirName);
275276
this.$logger.trace(`Versions found in ${pathToDir} are ${subDirsVersions.join(", ")}`);
276277
let version = semver.maxSatisfying(subDirsVersions, versionRange);
277-
if(version) {
278+
if (version) {
278279
selectedVersion = _.find(subDirs, dir => dir.indexOf(version) !== -1);
279280
}
280281
}
@@ -290,7 +291,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
290291
private getBuildToolsVersion(): IFuture<string> {
291292
return ((): string => {
292293
let buildToolsVersion: string;
293-
if(this.androidHome) {
294+
if (this.androidHome) {
294295
let pathToBuildTools = path.join(this.androidHome, "build-tools");
295296
let buildToolsRange = this.getBuildToolsRange();
296297
buildToolsVersion = this.getMatchingDir(pathToBuildTools, buildToolsRange).wait();
@@ -304,7 +305,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
304305
return ((): string => {
305306
let compileSdkVersion = this.getCompileSdk().wait();
306307
let requiredAppCompatRange: string;
307-
if(compileSdkVersion) {
308+
if (compileSdkVersion) {
308309
requiredAppCompatRange = `>=${compileSdkVersion} <${compileSdkVersion + 1}`;
309310
}
310311

@@ -316,7 +317,7 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
316317
return ((): string => {
317318
let selectedAppCompatVersion: string;
318319
let requiredAppCompatRange = this.getAppCompatRange().wait();
319-
if(this.androidHome && requiredAppCompatRange) {
320+
if (this.androidHome && requiredAppCompatRange) {
320321
let pathToAppCompat = path.join(this.androidHome, "extras", "android", "m2repository", "com", "android", "support", "appcompat-v7");
321322
selectedAppCompatVersion = this.getMatchingDir(pathToAppCompat, requiredAppCompatRange).wait();
322323
}
@@ -342,15 +343,15 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
342343
if (!this.installedTargetsCache) {
343344
try {
344345
let pathToAndroidExecutable = this.getPathToAndroidExecutable().wait();
345-
if(pathToAndroidExecutable) {
346-
let result = this.$childProcess.spawnFromEvent(pathToAndroidExecutable, ["list", "targets"], "close", {}, {throwError: false}).wait();
347-
if(result && result.stdout) {
346+
if (pathToAndroidExecutable) {
347+
let result = this.$childProcess.spawnFromEvent(pathToAndroidExecutable, ["list", "targets"], "close", {}, { throwError: false }).wait();
348+
if (result && result.stdout) {
348349
this.$logger.trace(result.stdout);
349350
this.installedTargetsCache = [];
350-
result.stdout.replace(/id: \d+ or "(.+)"/g, (m:string, p1:string) => (this.installedTargetsCache.push(p1), m));
351+
result.stdout.replace(/id: \d+ or "(.+)"/g, (m: string, p1: string) => (this.installedTargetsCache.push(p1), m));
351352
}
352353
}
353-
} catch(err) {
354+
} catch (err) {
354355
this.$logger.trace("Unable to get Android targets. Error is: " + err);
355356
}
356357
}
@@ -365,14 +366,14 @@ export class AndroidToolsInfo implements IAndroidToolsInfo {
365366
private _cachedAndroidHomeValidationResult: boolean = null;
366367
private validateAndroidHomeEnvVariable(androidHomeEnvVar: string): IFuture<boolean> {
367368
return ((): boolean => {
368-
if(this._cachedAndroidHomeValidationResult === null) {
369+
if (this._cachedAndroidHomeValidationResult === null) {
369370
this._cachedAndroidHomeValidationResult = true;
370371
let expectedDirectoriesInAndroidHome = ["build-tools", "tools", "platform-tools", "extras"];
371-
if(!androidHomeEnvVar || !this.$fs.exists(androidHomeEnvVar).wait()) {
372+
if (!androidHomeEnvVar || !this.$fs.exists(androidHomeEnvVar).wait()) {
372373
this.printMessage("The ANDROID_HOME environment variable is not set or it points to a non-existent directory. You will not be able to perform any build-related operations for Android.",
373374
"To be able to perform Android build-related operations, set the `ANDROID_HOME` variable to point to the root of your Android SDK installation directory.");
374375
this._cachedAndroidHomeValidationResult = false;
375-
} else if(!_.any(expectedDirectoriesInAndroidHome.map(dir => this.$fs.exists(path.join(androidHomeEnvVar, dir)).wait()))) {
376+
} else if (!_.any(expectedDirectoriesInAndroidHome.map(dir => this.$fs.exists(path.join(androidHomeEnvVar, dir)).wait()))) {
376377
this.printMessage("The ANDROID_HOME environment variable points to incorrect directory. You will not be able to perform any build-related operations for Android.",
377378
"To be able to perform Android build-related operations, set the `ANDROID_HOME` variable to point to the root of your Android SDK installation directory, " +
378379
"where you will find `tools` and `platform-tools` directories.");

‎lib/common

‎lib/config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export class StaticConfig extends StaticConfigBase implements IStaticConfig {
4646

4747
public get SYS_REQUIREMENTS_LINK(): string {
4848
let linkToSysRequirements: string;
49-
switch(process.platform) {
49+
switch (process.platform) {
5050
case "linux":
5151
linkToSysRequirements = "http://docs.nativescript.org/setup/ns-cli-setup/ns-setup-linux.html#system-requirements";
5252
break;
@@ -77,13 +77,13 @@ export class StaticConfig extends StaticConfigBase implements IStaticConfig {
7777
return path.join(__dirname, "..", "package.json");
7878
}
7979

80-
public get PATH_TO_BOOTSTRAP() : string {
80+
public get PATH_TO_BOOTSTRAP(): string {
8181
return path.join(__dirname, "bootstrap");
8282
}
8383

8484
public getAdbFilePath(): IFuture<string> {
8585
return (() => {
86-
if(!this._adbFilePath) {
86+
if (!this._adbFilePath) {
8787
let androidToolsInfo: IAndroidToolsInfo = this.$injector.resolve("androidToolsInfo");
8888
this._adbFilePath = androidToolsInfo.getPathToAdbFromAndroidHome().wait() || super.getAdbFilePath().wait();
8989
}

‎lib/providers/device-app-data-provider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as deviceAppDataBaseLib from "../common/mobile/device-app-data/device-a
44
import Future = require("fibers/future");
55
import * as path from "path";
66
import {AndroidDeviceHashService} from "../common/mobile/android/android-device-hash-service";
7-
import {AndroidDebugBridge} from "../common/mobile/android/android-debug-bridge";
7+
import {DeviceAndroidDebugBridge} from "../common/mobile/android/device-android-debug-bridge";
88

99
const SYNC_DIR_NAME = "sync";
1010
const FULLSYNC_DIR_NAME = "fullsync";
@@ -64,7 +64,7 @@ export class AndroidAppIdentifier extends deviceAppDataBaseLib.DeviceAppDataBase
6464

6565
private getSyncFolderName(): IFuture<string> {
6666
return ((): string =>{
67-
let adb = this.$injector.resolve(AndroidDebugBridge, { identifier: this.device.deviceInfo.identifier });
67+
let adb = this.$injector.resolve(DeviceAndroidDebugBridge, { identifier: this.device.deviceInfo.identifier });
6868
let deviceHashService = this.$injector.resolve(AndroidDeviceHashService, {adb: adb, appIdentifier: this.appIdentifier});
6969
let hashFile = this.$options.force ? null : deviceHashService.doesShasumFileExistsOnDevice().wait();
7070
return this.$options.watch || hashFile ? SYNC_DIR_NAME : FULLSYNC_DIR_NAME;

‎test/ios-project-service.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ function createTestInjector(projectPath: string, projectName: string): IInjector
4545
testInjector.register("config", ConfigLib.Configuration);
4646
testInjector.register("errors", ErrorsLib.Errors);
4747
testInjector.register("fs", FileSystemLib.FileSystem);
48+
testInjector.register("adb", {});
4849
testInjector.register("hostInfo", HostInfoLib.HostInfo);
4950
testInjector.register("injector", testInjector);
5051
testInjector.register("iOSEmulatorServices", {});

‎test/npm-support.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ let assert = require("chai").assert;
4141
function createTestInjector(): IInjector {
4242
let testInjector = new yok.Yok();
4343
testInjector.register("fs", FsLib.FileSystem);
44+
testInjector.register("adb", {});
4445
testInjector.register("options", OptionsLib.Options);
4546
testInjector.register("errors", ErrorsLib.Errors);
4647
testInjector.register("staticConfig", StaticConfigLib.StaticConfig);

‎test/plugins-service.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ function createTestInjector() {
4747
testInjector.register("messagesService", MessagesService);
4848
testInjector.register("npm", NodePackageManager);
4949
testInjector.register("fs", FileSystem);
50+
testInjector.register("adb", {});
51+
testInjector.register("androidDebugBridgeResultHandler", {});
5052
testInjector.register("projectData", ProjectData);
5153
testInjector.register("platforsmData", stubs.PlatformsDataStub);
5254
testInjector.register("childProcess", ChildProcess);

0 commit comments

Comments
 (0)