Skip to content

Fix failed ios build after second info-plist merge #1083

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 20, 2015
Merged

Fix failed ios build after second info-plist merge #1083

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e010223
Fixes https://github.com/NativeScript/nativescript-cli/issues/1066
Oct 19, 2015
aae1c98
Cherrypick the fix that allows $npm to be used with different configu…
Oct 19, 2015
bca245a
Remove the unneeded code from plugin.remove function and from android…
Oct 19, 2015
2daee31
Fix plugin remove, mock interpolateConfigurationFile and address some…
Oct 20, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/definitions/platform.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ interface IPlatformData {
targetedOS?: string[];
configurationFileName?: string;
configurationFilePath?: string;
relativeToFrameworkConfigurationFilePath: string;
mergeXmlConfig?: any[];
}

Expand Down
3 changes: 2 additions & 1 deletion lib/definitions/project.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ interface IPlatformProjectService {
platformData: IPlatformData;
validate(): IFuture<void>;
createProject(frameworkDir: string, frameworkVersion: string): IFuture<void>;
interpolateData(projectRoot: string): IFuture<void>;
interpolateData(): IFuture<void>;
interpolateConfigurationFile(configurationFilePath?: string): IFuture<void>;
afterCreateProject(projectRoot: string): IFuture<void>;
buildProject(projectRoot: string, buildConfig?: IBuildConfig): IFuture<void>;
prepareProject(): IFuture<void>;
Expand Down
21 changes: 16 additions & 5 deletions lib/services/android-project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as semver from "semver";
import * as projectServiceBaseLib from "./platform-project-service-base";
import * as androidDebugBridgePath from "../common/mobile/android/android-debug-bridge";

class AndroidProjectService extends projectServiceBaseLib.PlatformProjectServiceBase implements IPlatformProjectService {
export class AndroidProjectService extends projectServiceBaseLib.PlatformProjectServiceBase implements IPlatformProjectService {
private static VALUES_DIRNAME = "values";
private static VALUES_VERSION_DIRNAME_PREFIX = AndroidProjectService.VALUES_DIRNAME + "-v";
private static ANDROID_PLATFORM_NAME = "android";
Expand Down Expand Up @@ -57,6 +57,7 @@ class AndroidProjectService extends projectServiceBaseLib.PlatformProjectService
frameworkFilesExtensions: [".jar", ".dat", ".so"],
configurationFileName: "AndroidManifest.xml",
configurationFilePath: path.join(projectRoot, "src", "main", "AndroidManifest.xml"),
relativeToFrameworkConfigurationFilePath: path.join("src", "main", "AndroidManifest.xml"),
mergeXmlConfig: [{ "nodename": "manifest", "attrname": "*" }, {"nodename": "application", "attrname": "*"}]
};
}
Expand Down Expand Up @@ -134,18 +135,28 @@ class AndroidProjectService extends projectServiceBaseLib.PlatformProjectService
}).future<void>()();
}

public interpolateData(projectRoot: string): IFuture<void> {
public interpolateData(): IFuture<void> {
return (() => {
// Interpolate the activity name and package
let manifestPath = this.platformData.configurationFilePath;
shell.sed('-i', /__PACKAGE__/, this.$projectData.projectId, manifestPath);
// Interpolate the apilevel and package
this.interpolateConfigurationFile().wait();

let stringsFilePath = path.join(this.getAppResourcesDestinationDirectoryPath().wait(), 'values', 'strings.xml');
shell.sed('-i', /__NAME__/, this.$projectData.projectName, stringsFilePath);
shell.sed('-i', /__TITLE_ACTIVITY__/, this.$projectData.projectName, stringsFilePath);

let gradleSettingsFilePath = path.join(this.platformData.projectRoot, "settings.gradle");
shell.sed('-i', /__PROJECT_NAME__/, this.getProjectNameFromId(), gradleSettingsFilePath);
}).future<void>()();
}

public interpolateConfigurationFile(): IFuture<void> {
return (() => {
let manifestPath = this.platformData.configurationFilePath;

console.log("ANDROID MANIFEST FILE PATH!!!!");
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need these console.logs

console.log(manifestPath);

shell.sed('-i', /__PACKAGE__/, this.$projectData.projectId, manifestPath);
shell.sed('-i', /__APILEVEL__/, this.$options.sdk || this.$androidToolsInfo.getToolsInfo().wait().compileSdkVersion.toString(), manifestPath);
}).future<void>()();
}
Expand Down
21 changes: 14 additions & 7 deletions lib/services/ios-project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ
targetedOS: ['darwin'],
configurationFileName: "Info.plist",
configurationFilePath: path.join(projectRoot, this.$projectData.projectName, this.$projectData.projectName+"-Info.plist"),
relativeToFrameworkConfigurationFilePath: path.join("__PROJECT_NAME__", "__PROJECT_NAME__-Info.plist"),
mergeXmlConfig: [{ "nodename": "plist", "attrname": "*" }, {"nodename": "dict", "attrname": "*"}]
};
}
Expand Down Expand Up @@ -118,20 +119,26 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ
}).future<void>()();
}

public interpolateData(projectRoot: string): IFuture<void> {
public interpolateData(): IFuture<void> {
return (() => {
let infoPlistFilePath = path.join(projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER, util.format("%s-%s", IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER, "Info.plist"));
shell.sed('-i', "__CFBUNDLEIDENTIFIER__", this.$projectData.projectId, infoPlistFilePath);
let infoPlistFilePath = path.join(this.platformData.projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER, util.format("%s-%s", IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER, "Info.plist"));
this.interpolateConfigurationFile(infoPlistFilePath).wait();

this.replaceFileName("-Info.plist", path.join(projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER)).wait();
this.replaceFileName("-Prefix.pch", path.join(projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER)).wait();
this.replaceFileName(IOSProjectService.XCODE_PROJECT_EXT_NAME, projectRoot).wait();
this.replaceFileName("-Info.plist", path.join(this.platformData.projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER)).wait();
Copy link
Contributor

Choose a reason for hiding this comment

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

You can extract this long path into a dedicated variable - it is used on more than one place

this.replaceFileName("-Prefix.pch", path.join(this.platformData.projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER)).wait();
this.replaceFileName(IOSProjectService.XCODE_PROJECT_EXT_NAME, this.platformData.projectRoot).wait();

let pbxprojFilePath = path.join(projectRoot, this.$projectData.projectName + IOSProjectService.XCODE_PROJECT_EXT_NAME, "project.pbxproj");
let pbxprojFilePath = path.join(this.platformData.projectRoot, this.$projectData.projectName + IOSProjectService.XCODE_PROJECT_EXT_NAME, "project.pbxproj");
this.replaceFileContent(pbxprojFilePath).wait();
}).future<void>()();
}

public interpolateConfigurationFile(configurationFilePath?: string): IFuture<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for this function to return a future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - https://github.com/NativeScript/nativescript-cli/pull/1083/files#diff-b64860922c31aeed973aef6663b2a536R152

return (() => {
shell.sed('-i', "__CFBUNDLEIDENTIFIER__", this.$projectData.projectId, configurationFilePath || this.platformData.configurationFilePath);
}).future<void>()();
}

public afterCreateProject(projectRoot: string): IFuture<void> {
return (() => {
this.$fs.rename(path.join(projectRoot, IOSProjectService.IOS_PROJECT_NAME_PLACEHOLDER),
Expand Down
2 changes: 1 addition & 1 deletion lib/services/platform-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class PlatformService implements IPlatformService {
this.$fs.deleteDirectory(path.join(frameworkDir, "../../")).wait();
}

platformData.platformProjectService.interpolateData(platformData.projectRoot).wait();
platformData.platformProjectService.interpolateData().wait();
platformData.platformProjectService.afterCreateProject(platformData.projectRoot).wait();

this.$projectDataService.initialize(this.$projectData.projectDir);
Expand Down
50 changes: 27 additions & 23 deletions lib/services/plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,7 @@ export class PluginsService implements IPluginsService {
// Remove the plugin and call merge for another plugins that have configuration file
let pluginConfigurationFilePath = this.getPluginConfigurationFilePath(pluginData, platformData);
if(this.$fs.exists(pluginConfigurationFilePath).wait()) {
let tnsModulesDestinationPath = path.join(platformData.appDestinationDirectoryPath, constants.APP_FOLDER_NAME, constants.TNS_MODULES_FOLDER_NAME);
let nodeModules = this.$broccoliBuilder.getChangedNodeModules(tnsModulesDestinationPath, platform).wait();

_(nodeModules)
.map(nodeModule => this.getNodeModuleData(pluginName).wait())
.map(nodeModuleData => this.convertToPluginData(nodeModuleData))
.filter(data => data.isPlugin && this.$fs.exists(this.getPluginConfigurationFilePath(data, platformData)).wait())
.forEach((data, index) => {
executeUninstallCommand().wait();

if(index === 0) {
this.initializeConfigurationFileFromCache(platformData).wait();
}

if(data.name !== pluginName) {
this.merge(data, platformData).wait();
}
})
.value();
this.merge(pluginData, platformData).wait();
}

if(pluginData.pluginVariables) {
Expand Down Expand Up @@ -139,13 +121,14 @@ export class PluginsService implements IPluginsService {
let frameworkVersion = this.$projectDataService.getValue(platformData.frameworkPackageName).wait().version;
this.$npm.cache(platformData.frameworkPackageName, frameworkVersion).wait();

let relativeConfigurationFilePath = path.relative(platformData.projectRoot, platformData.configurationFilePath);
// We need to resolve this manager here due to some restrictions from npm api and in order to load PluginsService.NPM_CONFIG config
let npmInstallationManager: INpmInstallationManager = this.$injector.resolve("npmInstallationManager");
let cachedPackagePath = npmInstallationManager.getCachedPackagePath(platformData.frameworkPackageName, frameworkVersion);
let cachedConfigurationFilePath = path.join(cachedPackagePath, constants.PROJECT_FRAMEWORK_FOLDER_NAME, relativeConfigurationFilePath);
let cachedConfigurationFilePath = path.join(cachedPackagePath, constants.PROJECT_FRAMEWORK_FOLDER_NAME, platformData.relativeToFrameworkConfigurationFilePath);
let cachedConfigurationFileContent = this.$fs.readText(cachedConfigurationFilePath).wait();
this.$fs.writeFile(platformData.configurationFilePath, cachedConfigurationFileContent).wait();

shelljs.cp("-f", cachedConfigurationFilePath, path.dirname(platformData.configurationFilePath));
platformData.platformProjectService.interpolateConfigurationFile().wait();
}).future<void>()();
}

Expand Down Expand Up @@ -347,7 +330,7 @@ export class PluginsService implements IPluginsService {
doc.parseFromString(xml, 'text/xml');
}

private merge(pluginData: IPluginData, platformData: IPlatformData): IFuture<void> {
private mergeCore(pluginData: IPluginData, platformData: IPlatformData): IFuture<void> {
return (() => {
let pluginConfigurationFilePath = this.getPluginConfigurationFilePath(pluginData, platformData);
let configurationFilePath = platformData.configurationFilePath;
Expand All @@ -368,6 +351,27 @@ export class PluginsService implements IPluginsService {
}).future<void>()();
}

private merge(pluginData: IPluginData, platformData: IPlatformData): IFuture<void> {
return (() => {
let tnsModulesDestinationPath = path.join(platformData.appDestinationDirectoryPath, constants.APP_FOLDER_NAME, constants.TNS_MODULES_FOLDER_NAME);
let nodeModules = this.$broccoliBuilder.getChangedNodeModules(tnsModulesDestinationPath, platformData.normalizedPlatformName.toLowerCase()).wait();

_(_.keys(nodeModules))
Copy link
Contributor

Choose a reason for hiding this comment

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

why not

_(nodeModules)
     .keys()
     ....

.map(nodeModule => this.getNodeModuleData(path.join(nodeModule, "package.json")).wait())
.map(nodeModuleData => this.convertToPluginData(nodeModuleData))
.filter(data => data.isPlugin && this.$fs.exists(this.getPluginConfigurationFilePath(data, platformData)).wait())
.forEach((data, index) => {
if(index === 0) {
this.initializeConfigurationFileFromCache(platformData).wait();
}

this.mergeCore(data, platformData).wait();
})
.value();

}).future<void>()();
}

private getPluginConfigurationFilePath(pluginData: IPluginData, platformData: IPlatformData): string {
let pluginPlatformsFolderPath = pluginData.pluginPlatformsFolderPath(platformData.normalizedPlatformName.toLowerCase());
let pluginConfigurationFilePath = path.join(pluginPlatformsFolderPath, platformData.configurationFileName);
Expand Down
1 change: 0 additions & 1 deletion lib/tools/broccoli/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ let through = require("through2");

export class Builder implements IBroccoliBuilder {
constructor(private $fs: IFileSystem,
private $nodeModulesTree: INodeModulesTree,
private $projectData: IProjectData,
private $projectDataService: IProjectDataService,
private $injector: IInjector,
Expand Down
1 change: 1 addition & 0 deletions test/platform-commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class PlatformData implements IPlatformData {
frameworkVersion = "";
appDestinationDirectoryPath = "";
appResourcesDestinationDirectoryPath = "";
relativeToFrameworkConfigurationFilePath = "";
}

class ErrorsNoFailStub implements IErrors {
Expand Down
39 changes: 29 additions & 10 deletions test/plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {Yok} from '../lib/common/yok';
import future = require("fibers/future");
import * as stubs from './stubs';
import {NodePackageManager} from "../lib/node-package-manager";
import {NpmInstallationManager} from "../lib/npm-installation-manager";
import {FileSystem} from "../lib/common/file-system";
import {ProjectData} from "../lib/project-data";
import {ChildProcess} from "../lib/common/child-process";
Expand All @@ -23,6 +24,9 @@ import {ResourceLoader} from "../lib/common/resource-loader";
import {EOL} from "os";
import {PluginsService} from "../lib/services/plugins-service";
import {AddPluginCommand} from "../lib/commands/plugin/add-plugin";
import {Builder} from "../lib/tools/broccoli/builder";
import {AndroidProjectService} from "../lib/services/android-project-service";
import {AndroidToolsInfo} from "../lib/android-tools-info";
import {assert} from "chai";
import * as path from "path";
import * as temp from "temp";
Expand All @@ -40,13 +44,17 @@ function createTestInjector() {
testInjector.register("childProcess", ChildProcess);
testInjector.register("platformService", PlatformService);
testInjector.register("platformsData", PlatformsData);
testInjector.register("androidProjectService", {});
testInjector.register("androidEmulatorServices", {});
testInjector.register("androidToolsInfo", AndroidToolsInfo);
testInjector.register("sysInfo", {});
testInjector.register("mobileHelper", {});
testInjector.register("androidProjectService", AndroidProjectService);
testInjector.register("iOSProjectService", {});
testInjector.register("devicesServices", {});
testInjector.register("projectDataService", ProjectDataService);
testInjector.register("prompter", {});
testInjector.register("resources", ResourceLoader);
testInjector.register("broccoliBuilder", {});
testInjector.register("broccoliBuilder", Builder);
testInjector.register("options", Options);
testInjector.register("errors", Errors);
testInjector.register("logger", stubs.LoggerStub);
Expand All @@ -71,7 +79,7 @@ function createTestInjector() {
savePluginVariablesInProjectFile: (pluginData: IPluginData) => future.fromResult(),
interpolatePluginVariables: (pluginData: IPluginData, pluginConfigurationFileContent: string) => future.fromResult(pluginConfigurationFileContent)
});
testInjector.register("npmInstallationManager", {});
testInjector.register("npmInstallationManager", NpmInstallationManager);

return testInjector;
}
Expand All @@ -87,7 +95,7 @@ function createProjectFile(testInjector: IInjector): string {
"nativescript": {
"id": "org.nativescript.Test",
"tns-android": {
"version": "1.0.0"
"version": "1.4.0"
}
}
};
Expand Down Expand Up @@ -203,7 +211,7 @@ describe("Plugins service", () => {
});
it("fails when the plugin does not support the installed framework", () => {
let isWarningMessageShown = false;
let expectedWarningMessage = "mySamplePlugin 1.0.1 for android is not compatible with the currently installed framework version 1.0.0.";
let expectedWarningMessage = "mySamplePlugin 1.5.0 for android is not compatible with the currently installed framework version 1.4.0.";

// Creates plugin in temp folder
let pluginName = "mySamplePlugin";
Expand All @@ -214,7 +222,7 @@ describe("Plugins service", () => {
"version": "0.0.1",
"nativescript": {
"platforms": {
"android": "1.0.1"
"android": "1.5.0"
}
},
};
Expand Down Expand Up @@ -557,23 +565,34 @@ describe("Plugins service", () => {
// Mock platformsData
let platformsData = testInjector.resolve("platformsData");
platformsData.getPlatformData = (platform: string) => {
let androidProjectService = testInjector.resolve("androidProjectService");

return {
appDestinationDirectoryPath: appDestinationDirectoryPath,
frameworkPackageName: "tns-android",
configurationFileName: "AndroidManifest.xml",
configurationFilePath: path.join(appDestinationDirectoryPath, "AndroidManifest.xml"),
configurationFilePath: path.join(appDestinationDirectoryPath, "src", "main", "AndroidManifest.xml"),
relativeToFrameworkConfigurationFilePath: path.join("src", "main", "AndroidManifest.xml"),
mergeXmlConfig: [{ "nodename": "manifest", "attrname": "*" }],
platformProjectService: {
preparePluginNativeCode: (pluginData: IPluginData) => future.fromResult()
preparePluginNativeCode: (pluginData: IPluginData) => future.fromResult(),
interpolateConfigurationFile: () => androidProjectService.interpolateConfigurationFile()
},
normalizedPlatformName: "Android"
};
};

// Mock projectData
let projectData = testInjector.resolve("projectData");
projectData.projectId = "com.example.android.basiccontactables";

// Ensure the pluginDestinationPath folder exists
let pluginPlatformsDirPath = path.join(projectFolder, "node_modules", pluginName, "platforms", "android");
fs.ensureDirectoryExists(pluginPlatformsDirPath).wait();

let shelljs = require("shelljs");
shelljs.cp("-R", path.join(pluginFolderPath, "*"), path.join(projectFolder, "node_modules", pluginName));

// Creates valid plugin's AndroidManifest.xml file
let xml = '<?xml version="1.0" encoding="UTF-8"?>' +
'<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.example.android.basiccontactables" android:versionCode="1" android:versionName="1.0" >' +
Expand All @@ -584,11 +603,11 @@ describe("Plugins service", () => {

pluginsService.prepare(pluginJsonData).wait();

let expectedXml = '<?xmlversion="1.0"encoding="UTF-8"?><manifestxmlns:android="http://schemas.android.com/apk/res/android"package="com.example.android.basiccontactables"android:versionCode="1"android:versionName="1.0"><uses-permissionandroid:name="android.permission.READ_EXTERNAL_STORAGE"/><uses-permissionandroid:name="android.permission.WRITE_EXTERNAL_STORAGE"/><uses-permissionandroid:name="android.permission.INTERNET"/><applicationandroid:allowBackup="true"android:icon="@drawable/ic_launcher"android:label="@string/app_name"android:theme="@style/Theme.Sample"><activityandroid:name="com.example.android.basiccontactables.MainActivity"android:label="@string/app_name"android:launchMode="singleTop"><meta-dataandroid:name="android.app.searchable"android:resource="@xml/searchable"/><intent-filter><actionandroid:name="android.intent.action.SEARCH"/></intent-filter><intent-filter><actionandroid:name="android.intent.action.MAIN"/></intent-filter></activity></application><uses-permissionandroid:name="android.permission.READ_CONTACTS"/></manifest>';
let expectedXml = '<?xmlversion="1.0"encoding="utf-8"?><manifestxmlns:android="http://schemas.android.com/apk/res/android"package="com.example.android.basiccontactables"android:versionCode="1"android:versionName="1.0"><supports-screensandroid:smallScreens="true"android:normalScreens="true"android:largeScreens="true"android:xlargeScreens="true"/><uses-sdkandroid:minSdkVersion="17"android:targetSdkVersion="23"/><uses-permissionandroid:name="android.permission.READ_EXTERNAL_STORAGE"/><uses-permissionandroid:name="android.permission.WRITE_EXTERNAL_STORAGE"/><uses-permissionandroid:name="android.permission.INTERNET"/><applicationandroid:name="com.tns.NativeScriptApplication"android:allowBackup="true"android:icon="@drawable/icon"android:label="@string/app_name"android:theme="@style/AppTheme"><activityandroid:name="com.tns.NativeScriptActivity"android:label="@string/title_activity_kimera"android:configChanges="keyboardHidden|orientation|screenSize"><intent-filter><actionandroid:name="android.intent.action.MAIN"/><categoryandroid:name="android.intent.category.LAUNCHER"/></intent-filter></activity><activityandroid:name="com.tns.ErrorReportActivity"/></application><uses-permissionandroid:name="android.permission.READ_CONTACTS"/></manifest>';
expectedXml = helpers.stringReplaceAll(expectedXml, EOL, "");
expectedXml = helpers.stringReplaceAll(expectedXml, " ", "");

let actualXml = fs.readText(path.join(appDestinationDirectoryPath, "AndroidManifest.xml")).wait();
let actualXml = fs.readText(path.join(appDestinationDirectoryPath, "src", "main", "AndroidManifest.xml")).wait();
actualXml = helpers.stringReplaceAll(actualXml, "\n", "");
actualXml = helpers.stringReplaceAll(actualXml, " ", "");

Expand Down
Loading