From 51597bcdacda525a0209f10571edf9c74c67fee0 Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Sun, 22 Jul 2018 21:34:19 +0300 Subject: [PATCH 1/2] chore: Add logging in projectChangesService Add logging in projectChangesService's checkForChanges method in order to know which files are changed. This will help us to understand why prepare is triggered. --- lib/services/project-changes-service.ts | 27 +++++++++++++++++++++---- test/project-changes-service.ts | 6 ++---- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/services/project-changes-service.ts b/lib/services/project-changes-service.ts index 7c92087431..bd9c7f6cd7 100644 --- a/lib/services/project-changes-service.ts +++ b/lib/services/project-changes-service.ts @@ -54,6 +54,7 @@ export class ProjectChangesService implements IProjectChangesService { private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $fs: IFileSystem, private $filesHashService: IFilesHashService, + private $logger: ILogger, private $injector: IInjector) { } @@ -83,9 +84,13 @@ export class ProjectChangesService implements IProjectChangesService { projectData, this.fileChangeRequiresBuild); + this.$logger.trace(`Set nativeChanged to ${this._changesInfo.nativeChanged}. skipModulesNativeCheck is: ${projectChangesOptions.skipModulesNativeCheck}`); + if (this._newFiles > 0 || this._changesInfo.nativeChanged) { + this.$logger.trace(`Setting modulesChanged to true, newFiles: ${this._newFiles}, nativeChanged: ${this._changesInfo.nativeChanged}`); this._changesInfo.modulesChanged = true; } + if (platform === this.$devicePlatformsConstants.iOS.toLowerCase()) { this._changesInfo.configChanged = this.filesChanged([path.join(platformResourcesDir, platformData.configurationFileName), path.join(platformResourcesDir, "LaunchScreen.storyboard"), @@ -97,12 +102,15 @@ export class ProjectChangesService implements IProjectChangesService { path.join(platformResourcesDir, APP_GRADLE_FILE_NAME) ]); } + + this.$logger.trace(`Set value of configChanged to ${this._changesInfo.configChanged}`); } const projectService = platformData.platformProjectService; await projectService.checkForChanges(this._changesInfo, projectChangesOptions, projectData); if (projectChangesOptions.bundle !== this._prepareInfo.bundle || projectChangesOptions.release !== this._prepareInfo.release) { + this.$logger.trace(`Setting all setting to true. Current options are: `, projectChangesOptions, " old prepare info is: ", this._prepareInfo); this._changesInfo.appFilesChanged = true; this._changesInfo.appResourcesChanged = true; this._changesInfo.modulesChanged = true; @@ -112,9 +120,11 @@ export class ProjectChangesService implements IProjectChangesService { this._prepareInfo.bundle = projectChangesOptions.bundle; } if (this._changesInfo.packageChanged) { + this.$logger.trace("Set modulesChanged to true as packageChanged is true"); this._changesInfo.modulesChanged = true; } if (this._changesInfo.modulesChanged || this._changesInfo.appResourcesChanged) { + this.$logger.trace(`Set configChanged to true, current value of moduleChanged is: ${this._changesInfo.modulesChanged}, appResourcesChanged is: ${this._changesInfo.appResourcesChanged}`); this._changesInfo.configChanged = true; } if (this._changesInfo.hasChanges) { @@ -129,6 +139,7 @@ export class ProjectChangesService implements IProjectChangesService { this._changesInfo.nativePlatformStatus = this._prepareInfo.nativePlatformStatus; + this.$logger.trace("checkForChanges returns", this._changesInfo); return this._changesInfo; } @@ -234,14 +245,16 @@ export class ProjectChangesService implements IProjectChangesService { } private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean { - const dirName = path.basename(dir); + this.$logger.trace(`containsNewerFiles will check ${dir}`); if (_.startsWith(dirName, '.')) { + this.$logger.trace(`containsNewerFiles returns false for ${dir} as its name starts with dot (.) .`); return false; } const dirFileStat = this.$fs.getFsStats(dir); if (this.isFileModified(dirFileStat, dir)) { + this.$logger.trace(`containsNewerFiles returns true for ${dir} as the dir itself has been modified.`); return true; } @@ -256,24 +269,30 @@ export class ProjectChangesService implements IProjectChangesService { const changed = this.isFileModified(fileStats, filePath); if (changed) { + this.$logger.trace(`File ${filePath} has been changed.`); if (processFunc) { this._newFiles++; + this.$logger.trace(`Incremented the newFiles counter. Current value is ${this._newFiles}`); const filePathRelative = path.relative(projectData.projectDir, filePath); if (processFunc.call(this, filePathRelative, projectData)) { + this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`); return true; } } else { + this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`); return true; } } if (fileStats.isDirectory()) { if (this.containsNewerFiles(filePath, skipDir, projectData, processFunc)) { + this.$logger.trace(`containsNewerFiles returns true for ${dir}.`); return true; } } - } + + this.$logger.trace(`containsNewerFiles returns false for ${dir}.`); return false; } @@ -291,7 +310,7 @@ export class ProjectChangesService implements IProjectChangesService { } private fileChangeRequiresBuild(file: string, projectData: IProjectData) { - if (path.basename(file) === "package.json") { + if (path.basename(file) === PACKAGE_JSON_FILE_NAME) { return true; } const projectDir = projectData.projectDir; @@ -302,7 +321,7 @@ export class ProjectChangesService implements IProjectChangesService { let filePath = file; while (filePath !== NODE_MODULES_FOLDER_NAME) { filePath = path.dirname(filePath); - const fullFilePath = path.join(projectDir, path.join(filePath, "package.json")); + const fullFilePath = path.join(projectDir, path.join(filePath, PACKAGE_JSON_FILE_NAME)); if (this.$fs.exists(fullFilePath)) { const json = this.$fs.readJson(fullFilePath); if (json["nativescript"] && _.startsWith(file, path.join(filePath, "platforms"))) { diff --git a/test/project-changes-service.ts b/test/project-changes-service.ts index 9876fac6bf..ba80af7566 100644 --- a/test/project-changes-service.ts +++ b/test/project-changes-service.ts @@ -6,7 +6,7 @@ import { PlatformsData } from "../lib/platforms-data"; import { ProjectChangesService } from "../lib/services/project-changes-service"; import * as Constants from "../lib/constants"; import { FileSystem } from "../lib/common/file-system"; -import { HooksServiceStub } from "./stubs"; +import { HooksServiceStub, LoggerStub } from "./stubs"; // start tracking temporary folders/files temp.track(); @@ -34,9 +34,7 @@ class ProjectChangesServiceTest extends BaseServiceTest { this.injector.register("filesHashService", { generateHashes: () => Promise.resolve({}) }); - this.injector.register("logger", { - warn: () => ({}) - }); + this.injector.register("logger", LoggerStub); this.injector.register("hooksService", HooksServiceStub); const fs = this.injector.resolve("fs"); From d159572220621beefc6a871a4b6930d89849ac04 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 23 Jul 2018 13:10:50 +0300 Subject: [PATCH 2/2] fix: Skip preparation of plugins native files in case they are not changed Skip the preparation of plugins' native files in case there's no change of the files since last operation when the files have been prepared. Do this by using hash sums of files in each plugin's `platforms/` dir in case such exists. This way, in case you apply a change outside of the `/platforms/` dir, the plugin's native files will not be prepared again. This way application will not be rebuilt as there's no need to do this. --- lib/constants.ts | 1 + lib/services/plugins-service.ts | 48 +++++++++++++++-- test/ios-project-service.ts | 4 ++ test/plugins-service.ts | 91 +++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/lib/constants.ts b/lib/constants.ts index 42e724c28c..5865093dad 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -219,3 +219,4 @@ export class AddPlaformErrors { } export const PLUGIN_BUILD_DATA_FILENAME = "plugin-data.json"; +export const PLUGINS_BUILD_DATA_FILENAME = ".ns-plugins-build-data.json"; diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 8277f74e5e..b08b9f0bc3 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -36,6 +36,7 @@ export class PluginsService implements IPluginsService { private $options: IOptions, private $logger: ILogger, private $errors: IErrors, + private $filesHashService: IFilesHashService, private $injector: IInjector) { } public async add(plugin: string, projectData: IProjectData): Promise { @@ -107,7 +108,7 @@ export class PluginsService implements IPluginsService { return await platformData.platformProjectService.validatePlugins(projectData); } - public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData, projectFilesConfig: IProjectFilesConfig): Promise { + public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData, projectFilesConfig: IProjectFilesConfig): Promise { platform = platform.toLowerCase(); const platformData = this.$platformsData.getPlatformData(platform, projectData); const pluginData = this.convertToPluginData(dependencyData, projectData.projectDir); @@ -141,9 +142,26 @@ export class PluginsService implements IPluginsService { public async preparePluginNativeCode(pluginData: IPluginData, platform: string, projectData: IProjectData): Promise { const platformData = this.$platformsData.getPlatformData(platform, projectData); - pluginData.pluginPlatformsFolderPath = (_platform: string) => path.join(pluginData.fullPath, "platforms", _platform); - await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData); + + const pluginPlatformsFolderPath = pluginData.pluginPlatformsFolderPath(platform); + if (this.$fs.exists(pluginPlatformsFolderPath)) { + const pathToPluginsBuildFile = path.join(platformData.projectRoot, constants.PLUGINS_BUILD_DATA_FILENAME); + + const allPluginsNativeHashes = this.getAllPluginsNativeHashes(pathToPluginsBuildFile); + const oldPluginNativeHashes = allPluginsNativeHashes[pluginData.name]; + const currentPluginNativeHashes = await this.getPluginNativeHashes(pluginPlatformsFolderPath); + + if (!oldPluginNativeHashes || this.$filesHashService.hasChangesInShasums(oldPluginNativeHashes, currentPluginNativeHashes)) { + await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData); + this.setPluginNativeHashes({ + pathToPluginsBuildFile, + pluginData, + currentPluginNativeHashes, + allPluginsNativeHashes + }); + } + } } public async ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise { @@ -307,6 +325,30 @@ export class PluginsService implements IPluginsService { return isValid; } + + private async getPluginNativeHashes(pluginPlatformsDir: string): Promise { + let data: IStringDictionary = {}; + if (this.$fs.exists(pluginPlatformsDir)) { + const pluginNativeDataFiles = this.$fs.enumerateFilesInDirectorySync(pluginPlatformsDir); + data = await this.$filesHashService.generateHashes(pluginNativeDataFiles); + } + + return data; + } + + private getAllPluginsNativeHashes(pathToPluginsBuildFile: string): IDictionary { + let data: IDictionary = {}; + if (this.$fs.exists(pathToPluginsBuildFile)) { + data = this.$fs.readJson(pathToPluginsBuildFile); + } + + return data; + } + + private setPluginNativeHashes(opts: { pathToPluginsBuildFile: string, pluginData: IPluginData, currentPluginNativeHashes: IStringDictionary, allPluginsNativeHashes: IDictionary }): void { + opts.allPluginsNativeHashes[opts.pluginData.name] = opts.currentPluginNativeHashes; + this.$fs.writeJson(opts.pathToPluginsBuildFile, opts.allPluginsNativeHashes); + } } $injector.register("pluginsService", PluginsService); diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index a29e8cff37..0828d9361b 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -126,6 +126,10 @@ function createTestInjector(projectPath: string, projectName: string, xcode?: IX on: () => ({}) }); testInjector.register("emulatorHelper", {}); + testInjector.register("filesHashService", { + hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true, + generateHashes: async (files: string[]): Promise => ({}) + }); return testInjector; } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index d49dbebaef..41dcfeae72 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -34,6 +34,7 @@ import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import StaticConfigLib = require("../lib/config"); import * as path from "path"; import * as temp from "temp"; +import { PLUGINS_BUILD_DATA_FILENAME } from '../lib/constants'; temp.track(); let isErrorThrown = false; @@ -119,6 +120,10 @@ function createTestInjector() { testInjector.register("androidResourcesMigrationService", stubs.AndroidResourcesMigrationServiceStub); testInjector.register("platformEnvironmentRequirements", {}); + testInjector.register("filesHashService", { + hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true, + generateHashes: async (files: string[]): Promise => ({}) + }); return testInjector; } @@ -541,4 +546,90 @@ describe("Plugins service", () => { await pluginsService.prepare(pluginJsonData, "android", projectData, {}); }); }); + + describe("preparePluginNativeCode", () => { + const setupTest = (opts: { hasChangesInShasums?: boolean, newPluginHashes?: IStringDictionary, buildDataFileExists?: boolean, hasPluginPlatformsDir?: boolean }): any => { + const testData: any = { + pluginsService: null, + isPreparePluginNativeCodeCalled: false, + dataPassedToWriteJson: null + }; + + const unitTestsInjector = new Yok(); + unitTestsInjector.register("platformsData", { + getPlatformData: (platform: string, projectData: IProjectData) => ({ + projectRoot: "projectRoot", + platformProjectService: { + preparePluginNativeCode: async (pluginData: IPluginData, projData: IProjectData) => { + testData.isPreparePluginNativeCodeCalled = true; + } + } + }) + }); + + const pluginHashes = opts.newPluginHashes || { "file1": "hash1" }; + const pluginData: IPluginData = { + fullPath: "plugin_full_path", + name: "plugin_name" + }; + + unitTestsInjector.register("filesHashService", { + hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => !!opts.hasChangesInShasums, + generateHashes: async (files: string[]): Promise => pluginHashes + }); + + unitTestsInjector.register("fs", { + exists: (file: string) => { + if (file.indexOf(PLUGINS_BUILD_DATA_FILENAME) !== -1) { + return !!opts.buildDataFileExists; + } + + if (file.indexOf("platforms") !== -1) { + return !!opts.hasPluginPlatformsDir; + } + + return true; + }, + readJson: (file: string) => ({ + [pluginData.name]: pluginHashes + }), + writeJson: (file: string, json: any) => { testData.dataPassedToWriteJson = json; }, + enumerateFilesInDirectorySync: (): string[] => ["some_file"] + }); + + unitTestsInjector.register("npm", {}); + unitTestsInjector.register("options", {}); + unitTestsInjector.register("logger", {}); + unitTestsInjector.register("errors", {}); + unitTestsInjector.register("injector", unitTestsInjector); + + const pluginsService: PluginsService = unitTestsInjector.resolve(PluginsService); + testData.pluginsService = pluginsService; + testData.pluginData = pluginData; + return testData; + }; + + const platform = "platform"; + const projectData: IProjectData = {}; + + it("does not prepare the files when plugin does not have platforms dir", async () => { + const testData = setupTest({ hasPluginPlatformsDir: false }); + await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData); + assert.isFalse(testData.isPreparePluginNativeCodeCalled); + }); + + it("prepares the files when plugin has platforms dir and has not been built before", async () => { + const newPluginHashes = { "file": "hash" }; + const testData = setupTest({ newPluginHashes, hasPluginPlatformsDir: true }); + await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData); + assert.isTrue(testData.isPreparePluginNativeCodeCalled); + assert.deepEqual(testData.dataPassedToWriteJson, { [testData.pluginData.name]: newPluginHashes }); + }); + + it("does not prepare the files when plugin has platforms dir and files have not changed since then", async () => { + const testData = setupTest({ hasChangesInShasums: false, buildDataFileExists: true, hasPluginPlatformsDir: true }); + await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData); + assert.isFalse(testData.isPreparePluginNativeCodeCalled); + }); + }); });