From 6539f8660d9c8fd3b56d4888e65dadbd0a3cab97 Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Sun, 8 Jul 2018 14:52:06 +0300 Subject: [PATCH 1/2] feat: Speed up adding native platform The current logic for adding native platform is: 1. Install the runtime package (tns-ios or tns-android) as a dependency of the project 2. Copy all files from the `/node_modules//framework` to `/plaforms//...` (specific places there). 3. Uninstall the runtime package, so it is no longer dependency of the project. This solution becomes really slow when the project has a lot of dependencies. For example, for `tns-template-master-detail-ng`, all these steps take around 2 mins and 5 seconds on Windows (without SSD) and around 40 seconds on macOS (with SSD). In order to speed up this process, remove the installing and uninstalling of the runtime package. Instead of this, use the `pacote` package to extract the runtime to a directory in the `temp` and copy files from there to `platforms` directory. As we do not execute `npm install` and `npm uninstall` with this solution, the operation is not dependant on the number of dependencies used in the project. It takes around 5-9 second on both Windows and macOS. Also remove the usage of `$options` from `npmInstallationManager` - its purpose was to work with `--frameworkPath` on `tns platform add ` command, but we no longer call `npmInstallationManager` for this case. Fix the tests for `platorm-service`'s addPlatform method as they were incorrectly passing. --- lib/npm-installation-manager.ts | 4 +- lib/services/android-project-service.ts | 16 +-- lib/services/platform-service.ts | 37 +++--- test/npm-support.ts | 3 + test/platform-commands.ts | 3 + test/platform-service.ts | 159 +++++++++++------------- 6 files changed, 103 insertions(+), 119 deletions(-) diff --git a/lib/npm-installation-manager.ts b/lib/npm-installation-manager.ts index a9ae2b162f..b3501286ca 100644 --- a/lib/npm-installation-manager.ts +++ b/lib/npm-installation-manager.ts @@ -6,7 +6,6 @@ export class NpmInstallationManager implements INpmInstallationManager { constructor(private $npm: INodePackageManager, private $childProcess: IChildProcess, private $logger: ILogger, - private $options: IOptions, private $settingsService: ISettingsService, private $fs: IFileSystem, private $staticConfig: IStaticConfig, @@ -39,9 +38,8 @@ export class NpmInstallationManager implements INpmInstallationManager { return maxSatisfying || latestVersion; } - public async install(packageName: string, projectDir: string, opts?: INpmInstallOptions): Promise { + public async install(packageToInstall: string, projectDir: string, opts?: INpmInstallOptions): Promise { try { - const packageToInstall = this.$options.frameworkPath || packageName; const pathToSave = projectDir; const version = (opts && opts.version) || null; const dependencyType = (opts && opts.dependencyType) || null; diff --git a/lib/services/android-project-service.ts b/lib/services/android-project-service.ts index 87df48fc29..e78a5ae4ac 100644 --- a/lib/services/android-project-service.ts +++ b/lib/services/android-project-service.ts @@ -144,7 +144,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject const targetSdkVersion = androidToolsInfo && androidToolsInfo.targetSdkVersion; this.$logger.trace(`Using Android SDK '${targetSdkVersion}'.`); - this.isAndroidStudioTemplate = this.isAndroidStudioCompatibleTemplate(projectData); + this.isAndroidStudioTemplate = this.isAndroidStudioCompatibleTemplate(projectData, frameworkVersion); if (this.isAndroidStudioTemplate) { this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "*", "-R"); } else { @@ -703,20 +703,12 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject } } - private isAndroidStudioCompatibleTemplate(projectData: IProjectData): boolean { + private isAndroidStudioCompatibleTemplate(projectData: IProjectData, frameworkVersion?: string): boolean { const currentPlatformData: IDictionary = this.$projectDataService.getNSValue(projectData.projectDir, constants.TNS_ANDROID_RUNTIME_NAME); - let platformVersion = currentPlatformData && currentPlatformData[constants.VERSION_STRING]; + const platformVersion = (currentPlatformData && currentPlatformData[constants.VERSION_STRING]) || frameworkVersion; if (!platformVersion) { - const tnsAndroidPackageJsonPath = path.join(projectData.projectDir, constants.NODE_MODULES_FOLDER_NAME, constants.TNS_ANDROID_RUNTIME_NAME, constants.PACKAGE_JSON_FILE_NAME); - if (this.$fs.exists(tnsAndroidPackageJsonPath)) { - const projectPackageJson: any = this.$fs.readJson(tnsAndroidPackageJsonPath); - if (projectPackageJson && projectPackageJson.version) { - platformVersion = projectPackageJson.version; - } - } else { - return true; - } + return true; } if (platformVersion === constants.PackageVersion.NEXT || platformVersion === constants.PackageVersion.LATEST || platformVersion === constants.PackageVersion.RC) { diff --git a/lib/services/platform-service.ts b/lib/services/platform-service.ts index 272590e8a9..55f863842b 100644 --- a/lib/services/platform-service.ts +++ b/lib/services/platform-service.ts @@ -36,11 +36,12 @@ export class PlatformService extends EventEmitter implements IPlatformService { private $mobileHelper: Mobile.IMobileHelper, private $hostInfo: IHostInfo, private $devicePathProvider: IDevicePathProvider, - private $npm: INodePackageManager, private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $projectChangesService: IProjectChangesService, private $analyticsService: IAnalyticsService, - private $terminalSpinnerService: ITerminalSpinnerService) { + private $terminalSpinnerService: ITerminalSpinnerService, + private $pacoteService: IPacoteService + ) { super(); } @@ -92,10 +93,6 @@ export class PlatformService extends EventEmitter implements IPlatformService { const platformData = this.$platformsData.getPlatformData(platform, projectData); - if (version === undefined) { - version = this.getCurrentPlatformVersion(platform, projectData); - } - // Log the values for project this.$logger.trace("Creating NativeScript project for the %s platform", platform); this.$logger.trace("Path: %s", platformData.projectRoot); @@ -105,28 +102,28 @@ export class PlatformService extends EventEmitter implements IPlatformService { this.$logger.out("Copying template files..."); let packageToInstall = ""; - const npmOptions: IStringDictionary = { - pathToSave: path.join(projectData.platformsDir, platform), - dependencyType: "save" - }; + if (frameworkPath) { + packageToInstall = path.resolve(frameworkPath); + } else { + if (!version) { + version = this.getCurrentPlatformVersion(platform, projectData) || + await this.$npmInstallationManager.getLatestCompatibleVersion(platformData.frameworkPackageName); + } - if (!frameworkPath) { - packageToInstall = platformData.frameworkPackageName; - npmOptions["version"] = version; + packageToInstall = `${platformData.frameworkPackageName}@${version}`; } const spinner = this.$terminalSpinnerService.createSpinner(); - const projectDir = projectData.projectDir; const platformPath = path.join(projectData.platformsDir, platform); try { spinner.start(); - const downloadedPackagePath = await this.$npmInstallationManager.install(packageToInstall, projectDir, npmOptions); + const downloadedPackagePath = temp.mkdirSync("runtimeDir"); + temp.track(); + await this.$pacoteService.extractPackage(packageToInstall, downloadedPackagePath); let frameworkDir = path.join(downloadedPackagePath, constants.PROJECT_FRAMEWORK_FOLDER_NAME); frameworkDir = path.resolve(frameworkDir); - - const coreModuleName = await this.addPlatformCore(platformData, frameworkDir, platformTemplate, projectData, config, nativePrepare); - await this.$npm.uninstall(coreModuleName, { save: true }, projectData.projectDir); + await this.addPlatformCore(platformData, frameworkDir, platformTemplate, projectData, config, nativePrepare); } catch (err) { this.$fs.deleteDirectory(platformPath); throw err; @@ -842,15 +839,15 @@ export class PlatformService extends EventEmitter implements IPlatformService { const data = this.$projectDataService.getNSValue(projectData.projectDir, platformData.frameworkPackageName); const currentVersion = data && data.version ? data.version : "0.2.0"; + const installedModuleDir = temp.mkdirSync("runtime-to-update"); let newVersion = version === constants.PackageVersion.NEXT ? await this.$npmInstallationManager.getNextVersion(platformData.frameworkPackageName) : version || await this.$npmInstallationManager.getLatestCompatibleVersion(platformData.frameworkPackageName); - const installedModuleDir = await this.$npmInstallationManager.install(platformData.frameworkPackageName, projectData.projectDir, { version: newVersion, dependencyType: "save" }); + await this.$pacoteService.extractPackage(`${platformData.frameworkPackageName}@${newVersion}`, installedModuleDir); const cachedPackageData = this.$fs.readJson(path.join(installedModuleDir, "package.json")); newVersion = (cachedPackageData && cachedPackageData.version) || newVersion; const canUpdate = platformData.platformProjectService.canUpdatePlatform(installedModuleDir, projectData); - await this.$npm.uninstall(platformData.frameworkPackageName, { save: true }, projectData.projectDir); if (canUpdate) { if (!semver.valid(newVersion)) { this.$errors.fail("The version %s is not valid. The version should consists from 3 parts separated by dot.", newVersion); diff --git a/test/npm-support.ts b/test/npm-support.ts index 61ebc1c2df..5b9bc9e5d0 100644 --- a/test/npm-support.ts +++ b/test/npm-support.ts @@ -104,6 +104,9 @@ function createTestInjector(): IInjector { getChanges: () => Promise.resolve({}), generateHashes: () => Promise.resolve() }); + testInjector.register("pacoteService", { + extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => undefined + }); return testInjector; } diff --git a/test/platform-commands.ts b/test/platform-commands.ts index 9f2730fa32..f6d8253c34 100644 --- a/test/platform-commands.ts +++ b/test/platform-commands.ts @@ -168,6 +168,9 @@ function createTestInjector() { testInjector.register("platformEnvironmentRequirements", { checkEnvironmentRequirements: async (platform?: string, projectDir?: string, runtimeVersion?: string): Promise => true }); + testInjector.register("pacoteService", { + extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => undefined + }); return testInjector; } diff --git a/test/platform-service.ts b/test/platform-service.ts index 3c969262b9..918c451b01 100644 --- a/test/platform-service.ts +++ b/test/platform-service.ts @@ -2,7 +2,7 @@ import * as yok from "../lib/common/yok"; import * as stubs from "./stubs"; import * as PlatformServiceLib from "../lib/services/platform-service"; import * as StaticConfigLib from "../lib/config"; -import { VERSION_STRING } from "../lib/constants"; +import { VERSION_STRING, PACKAGE_JSON_FILE_NAME } from "../lib/constants"; import * as fsLib from "../lib/common/file-system"; import * as optionsLib from "../lib/options"; import * as hostInfoLib from "../lib/common/host-info"; @@ -23,6 +23,7 @@ import ProjectChangesLib = require("../lib/services/project-changes-service"); import { Messages } from "../lib/common/messages/messages"; import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import { INFO_PLIST_FILE_NAME, MANIFEST_FILE_NAME } from "../lib/constants"; +import { mkdir } from "shelljs"; require("should"); const temp = require("temp"); @@ -36,6 +37,7 @@ function createTestInjector() { testInjector.register('logger', stubs.LoggerStub); testInjector.register("nodeModulesDependenciesBuilder", {}); testInjector.register('npmInstallationManager', stubs.NpmInstallationManagerStub); + // TODO: Remove the projectData - it shouldn't be required in the service itself. testInjector.register('projectData', stubs.ProjectDataStub); testInjector.register('platformsData', stubs.PlatformsDataStub); testInjector.register('devicesService', {}); @@ -108,7 +110,16 @@ function createTestInjector() { testInjector.register("androidResourcesMigrationService", stubs.AndroidResourcesMigrationServiceStub); testInjector.register("filesHashService", { generateHashes: () => Promise.resolve(), - getChanges: () => Promise.resolve({test: "testHash"}) + getChanges: () => Promise.resolve({ test: "testHash" }) + }); + testInjector.register("pacoteService", { + extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => { + mkdir(path.join(destinationDirectory, "framework")); + (new fsLib.FileSystem(testInjector)).writeFile(path.join(destinationDirectory, PACKAGE_JSON_FILE_NAME), JSON.stringify({ + name: "package-name", + version: "1.0.0" + })); + } }); return testInjector; @@ -199,105 +210,85 @@ describe('Platform Service Tests', () => { await platformService.addPlatforms(["IoS"], "", projectData, config); await platformService.addPlatforms(["iOs"], "", projectData, config); }); + it("should fail if platform is already installed", async () => { const projectData: IProjectData = testInjector.resolve("projectData"); // By default fs.exists returns true, so the platforms directory should exists - await assert.isRejected(platformService.addPlatforms(["android"], "", projectData, config)); - await assert.isRejected(platformService.addPlatforms(["ios"], "", projectData, config)); + await assert.isRejected(platformService.addPlatforms(["android"], "", projectData, config), "Platform android already added"); + await assert.isRejected(platformService.addPlatforms(["ios"], "", projectData, config), "Platform ios already added"); }); - it("should fail if npm is unavalible", async () => { - const fs = testInjector.resolve("fs"); - fs.exists = () => false; - - const errorMessage = "Npm is unavalible"; - const npmInstallationManager = testInjector.resolve("npmInstallationManager"); - npmInstallationManager.install = () => { throw new Error(errorMessage); }; - const projectData: IProjectData = testInjector.resolve("projectData"); - try { - await platformService.addPlatforms(["android"], "", projectData, config); - } catch (err) { - assert.equal(errorMessage, err.message); - } - }); - it("should respect platform version in package.json's nativescript key", async () => { - const versionString = "2.5.0"; + it("should fail if unable to extract runtime package", async () => { const fs = testInjector.resolve("fs"); fs.exists = () => false; - const nsValueObject: any = {}; - nsValueObject[VERSION_STRING] = versionString; - const projectDataService = testInjector.resolve("projectDataService"); - projectDataService.getNSValue = () => nsValueObject; - - const npmInstallationManager = testInjector.resolve("npmInstallationManager"); - npmInstallationManager.install = (packageName: string, packageDir: string, options: INpmInstallOptions) => { - assert.deepEqual(options.version, versionString); - return ""; + const pacoteService = testInjector.resolve("pacoteService"); + const errorMessage = "Pacote service unable to extract package"; + pacoteService.extractPackage = async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => { + throw new Error(errorMessage); }; const projectData: IProjectData = testInjector.resolve("projectData"); - - await platformService.addPlatforms(["android"], "", projectData, config); - await platformService.addPlatforms(["ios"], "", projectData, config); + await assert.isRejected(platformService.addPlatforms(["android"], "", projectData, config), errorMessage); }); - it("should install latest platform if no information found in package.json's nativescript key", async () => { + + const assertCorrectDataIsPassedToPacoteService = async (versionString: string): Promise => { const fs = testInjector.resolve("fs"); fs.exists = () => false; - const projectDataService = testInjector.resolve("projectDataService"); - projectDataService.getNSValue = (): any => null; - - const npmInstallationManager = testInjector.resolve("npmInstallationManager"); - npmInstallationManager.install = (packageName: string, packageDir: string, options: INpmInstallOptions) => { - assert.deepEqual(options.version, undefined); - return ""; + const pacoteService = testInjector.resolve("pacoteService"); + let packageNamePassedToPacoteService = ""; + pacoteService.extractPackage = async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => { + packageNamePassedToPacoteService = packageName; }; + const platformsData = testInjector.resolve("platformsData"); + const packageName = "packageName"; + platformsData.getPlatformData = (platform: string, projectData: IProjectData): IPlatformData => { + return { + frameworkPackageName: packageName, + platformProjectService: new stubs.PlatformProjectServiceStub(), + emulatorServices: undefined, + projectRoot: "", + normalizedPlatformName: "", + appDestinationDirectoryPath: "", + deviceBuildOutputPath: "", + getValidBuildOutputData: (buildOptions: IBuildOutputOptions) => ({ packageNames: [] }), + frameworkFilesExtensions: [], + relativeToFrameworkConfigurationFilePath: "", + fastLivesyncFileExtensions: [] + }; + }; const projectData: IProjectData = testInjector.resolve("projectData"); await platformService.addPlatforms(["android"], "", projectData, config); + assert.equal(packageNamePassedToPacoteService, `${packageName}@${versionString}`); await platformService.addPlatforms(["ios"], "", projectData, config); - }); - }); - describe("#add platform(ios)", () => { - it("should call validate method", async () => { - const fs = testInjector.resolve("fs"); - fs.exists = () => false; - - const errorMessage = "Xcode is not installed or Xcode version is smaller that 5.0"; - const platformsData = testInjector.resolve("platformsData"); - const platformProjectService = platformsData.getPlatformData().platformProjectService; - const projectData: IProjectData = testInjector.resolve("projectData"); - platformProjectService.validate = () => { - throw new Error(errorMessage); + assert.equal(packageNamePassedToPacoteService, `${packageName}@${versionString}`); + }; + it("should respect platform version in package.json's nativescript key", async () => { + const versionString = "2.5.0"; + const nsValueObject: any = { + [VERSION_STRING]: versionString }; + const projectDataService = testInjector.resolve("projectDataService"); + projectDataService.getNSValue = () => nsValueObject; - try { - await platformService.addPlatforms(["ios"], "", projectData, config); - } catch (err) { - assert.equal(errorMessage, err.message); - } + await assertCorrectDataIsPassedToPacoteService(versionString); }); - }); - describe("#add platform(android)", () => { - it("should fail if java, ant or android are not installed", async () => { - const fs = testInjector.resolve("fs"); - fs.exists = () => false; - const errorMessage = "Java, ant or android are not installed"; - const platformsData = testInjector.resolve("platformsData"); - const platformProjectService = platformsData.getPlatformData().platformProjectService; - platformProjectService.validate = () => { - throw new Error(errorMessage); + it("should install latest platform if no information found in package.json's nativescript key", async () => { + + const projectDataService = testInjector.resolve("projectDataService"); + projectDataService.getNSValue = (): any => null; + + const latestCompatibleVersion = "1.0.0"; + const npmInstallationManager = testInjector.resolve("npmInstallationManager"); + npmInstallationManager.getLatestCompatibleVersion = async (packageName: string, referenceVersion?: string): Promise => { + return latestCompatibleVersion; }; - const projectData: IProjectData = testInjector.resolve("projectData"); - try { - await platformService.addPlatforms(["android"], "", projectData, config); - } catch (err) { - assert.equal(errorMessage, err.message); - } + await assertCorrectDataIsPassedToPacoteService(latestCompatibleVersion); }); }); }); @@ -962,18 +953,18 @@ describe('Platform Service Tests', () => { }, { name: "productFlavors are specified in .gradle file", buildOutput: [`/my/path/arm64Demo/${configuration}/${apkName}-arm64-demo-${configuration}.apk`, - `/my/path/arm64Full/${configuration}/${apkName}-arm64-full-${configuration}.apk`, - `/my/path/armDemo/${configuration}/${apkName}-arm-demo-${configuration}.apk`, - `/my/path/armFull/${configuration}/${apkName}-arm-full-${configuration}.apk`, - `/my/path/x86Demo/${configuration}/${apkName}-x86-demo-${configuration}.apk`, - `/my/path/x86Full/${configuration}/${apkName}-x86-full-${configuration}.apk`], + `/my/path/arm64Full/${configuration}/${apkName}-arm64-full-${configuration}.apk`, + `/my/path/armDemo/${configuration}/${apkName}-arm-demo-${configuration}.apk`, + `/my/path/armFull/${configuration}/${apkName}-arm-full-${configuration}.apk`, + `/my/path/x86Demo/${configuration}/${apkName}-x86-demo-${configuration}.apk`, + `/my/path/x86Full/${configuration}/${apkName}-x86-full-${configuration}.apk`], expectedResult: `/my/path/x86Full/${configuration}/${apkName}-x86-full-${configuration}.apk` }, { name: "split options are specified in .gradle file", buildOutput: [`/my/path/${configuration}/${apkName}-arm64-v8a-${configuration}.apk`, - `/my/path/${configuration}/${apkName}-armeabi-v7a-${configuration}.apk`, - `/my/path/${configuration}/${apkName}-universal-${configuration}.apk`, - `/my/path/${configuration}/${apkName}-x86-${configuration}.apk`], + `/my/path/${configuration}/${apkName}-armeabi-v7a-${configuration}.apk`, + `/my/path/${configuration}/${apkName}-universal-${configuration}.apk`, + `/my/path/${configuration}/${apkName}-x86-${configuration}.apk`], expectedResult: `/my/path/${configuration}/${apkName}-x86-${configuration}.apk` }, { name: "android-runtime has version < 4.0.0", @@ -983,7 +974,7 @@ describe('Platform Service Tests', () => { } const platform = "Android"; - const buildConfigs = [{buildForDevice: false}, {buildForDevice: true}]; + const buildConfigs = [{ buildForDevice: false }, { buildForDevice: true }]; const apkNames = ["app", "testProj"]; const configurations = ["debug", "release"]; @@ -993,7 +984,7 @@ describe('Platform Service Tests', () => { _.each(getTestCases(configuration, apkName), testCase => { it(`should find correct ${configuration} ${apkName}.apk when ${testCase.name} and buildConfig is ${JSON.stringify(buildConfig)}`, async () => { mockData(testCase.buildOutput, apkName); - const actualResult = await platformService.buildPlatform(platform, buildConfig, {projectName: ""}); + const actualResult = await platformService.buildPlatform(platform, buildConfig, { projectName: "" }); assert.deepEqual(actualResult, testCase.expectedResult); }); }); From 0a391531775528945d7fffdce3ce3785eb95c703 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 9 Jul 2018 10:50:39 +0300 Subject: [PATCH 2/2] chore: Update message when platform is added --- lib/services/platform-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/services/platform-service.ts b/lib/services/platform-service.ts index 55f863842b..c1abf65caa 100644 --- a/lib/services/platform-service.ts +++ b/lib/services/platform-service.ts @@ -132,7 +132,7 @@ export class PlatformService extends EventEmitter implements IPlatformService { } this.$fs.ensureDirectoryExists(platformPath); - this.$logger.out("Project successfully created."); + this.$logger.out(`Platform ${platform} successfully added.`); } private async addPlatformCore(platformData: IPlatformData, frameworkDir: string, platformTemplate: string, projectData: IProjectData, config: IPlatformOptions, nativePrepare?: INativePrepare): Promise {