Skip to content

Plamen5kov/as compatible cli #3193

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.

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 6 commits into from
Nov 8, 2017
Merged
Changes from 5 commits
Commits
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
107 changes: 84 additions & 23 deletions lib/services/android-project-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
];

private _androidProjectPropertiesManagers: IDictionary<IAndroidProjectPropertiesManager>;
private isASTemplate: boolean;

constructor(private $androidEmulatorServices: Mobile.IEmulatorPlatformServices,
private $androidToolsInfo: IAndroidToolsInfo,
Expand All @@ -38,6 +39,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
private $npm: INodePackageManager) {
super($fs, $projectDataService);
this._androidProjectPropertiesManagers = Object.create(null);
this.isASTemplate = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing we're talking about AndroidStudio template here - can we name the variable isAndroidStudioTemplate for example so that it's a bit clearer

}

private _platformsDirCache: string = null;
Expand All @@ -46,19 +48,41 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
if (!projectData && !this._platformData) {
throw new Error("First call of getPlatformData without providing projectData.");
}
if (projectData && projectData.platformsDir) {
const projectRoot = path.join(projectData.platformsDir, AndroidProjectService.ANDROID_PLATFORM_NAME);
if (this.isAndroidStudioCompattibleTemplate(projectData)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor typo
isAndroidStudioCompattibleTemplate -> isAndroidStudioCompatibleTemplate

this.isASTemplate = true;
}

const appDestinationDirectoryArr = ["src", "main", "assets"];
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a suggestion for this code. Right now we start with 3 items in the array and later on we unshift one or two items to it. This seems a bit backwards to me.
Instead, we could start from the first item and work our way towards the end like so:

const appDestinationDirectoryArr = [projectRoot];
if (this.isAndroidStudioTemplate) {
    appDestinationDirectoryArr.push(APP_FOLDER_NAME);
}

appDestinationDirectoryArr.push("src", "main", "assets");

This way the logic will be start-to-end in contrast of end-to-start. In addition, push is very likely faster than unshift.
This can be done for all 3 occurrences of this type of code down below.

if (this.isASTemplate) {
appDestinationDirectoryArr.unshift("app");
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 use APP_FOLDER_NAME from constants here

}
appDestinationDirectoryArr.unshift(projectRoot);

const configurationsDirectoryArr = ["src", "main", "AndroidManifest.xml"];
Copy link
Contributor

Choose a reason for hiding this comment

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

We can introduce constants for these hard-coded strings. I'm thinking something like:

export class AndroidSpecificFileNames {
    static SRC_DIR = "src";
    static MAIN_DIR = "main";
    static MANIFEST_FILE_NAME = "AndroidManifest.xml";
    static BUILD_DIR = "build";
    static OUTPUTS_DIR = "outputs";
    static APK_DIR = "apk";
    static ASSETS_DIR = "assets";
    static RES_DIR = "res";
}

for example. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add them to constants file since we already have an established commonplace for constants.

if (this.isASTemplate) {
configurationsDirectoryArr.unshift("app");
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 use APP_FOLDER_NAME from constants here too

}
configurationsDirectoryArr.unshift(projectRoot);

const deviceBuildOutputArr = ["build", "outputs", "apk"];
if (this.isASTemplate) {
deviceBuildOutputArr.unshift("app");
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 use APP_FOLDER_NAME here too

}
deviceBuildOutputArr.unshift(projectRoot);

if (projectData && projectData.platformsDir && this._platformsDirCache !== projectData.platformsDir) {
this._platformsDirCache = projectData.platformsDir;
const projectRoot = path.join(projectData.platformsDir, AndroidProjectService.ANDROID_PLATFORM_NAME);
const packageName = this.getProjectNameFromId(projectData);

this._platformData = {
frameworkPackageName: "tns-android",
normalizedPlatformName: "Android",
appDestinationDirectoryPath: path.join(projectRoot, "src", "main", "assets"),
appDestinationDirectoryPath: path.join(...appDestinationDirectoryArr),
platformProjectService: this,
emulatorServices: this.$androidEmulatorServices,
projectRoot: projectRoot,
deviceBuildOutputPath: path.join(projectRoot, "build", "outputs", "apk"),
deviceBuildOutputPath: path.join(...deviceBuildOutputArr),
getValidPackageNames: (buildOptions: { isReleaseBuild?: boolean, isForDevice?: boolean }): string[] => {
const buildMode = buildOptions.isReleaseBuild ? Configurations.Release.toLowerCase() : Configurations.Debug.toLowerCase();

Expand All @@ -70,10 +94,11 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
},
frameworkFilesExtensions: [".jar", ".dat", ".so"],
configurationFileName: "AndroidManifest.xml",
configurationFilePath: path.join(projectRoot, "src", "main", "AndroidManifest.xml"),
configurationFilePath: path.join(...configurationsDirectoryArr),
relativeToFrameworkConfigurationFilePath: path.join("src", "main", "AndroidManifest.xml"),
fastLivesyncFileExtensions: [".jpg", ".gif", ".png", ".bmp", ".webp"] // http://developer.android.com/guide/appendix/media-formats.html
};

}

return this._platformData;
Expand All @@ -93,7 +118,12 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject

public getAppResourcesDestinationDirectoryPath(projectData: IProjectData, frameworkVersion?: string): string {
if (this.canUseGradle(projectData, frameworkVersion)) {
return path.join(this.getPlatformData(projectData).projectRoot, "src", "main", "res");
const resourcePath: string[] = ["src", "main", "res"];
if (this.isASTemplate) {
resourcePath.unshift("app");
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 use the APP_FOLDER_NAME constant here too

}

return path.join(this.getPlatformData(projectData).projectRoot, path.join(...resourcePath));
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need for the inner path.join statement - it could just be

return path.join(this.getPlatformData(projectData).projectRoot, ...resourcePath);

}

return path.join(this.getPlatformData(projectData).projectRoot, "res");
Expand Down Expand Up @@ -125,25 +155,31 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
const androidToolsInfo = this.$androidToolsInfo.getToolsInfo();
const targetSdkVersion = androidToolsInfo && androidToolsInfo.targetSdkVersion;
this.$logger.trace(`Using Android SDK '${targetSdkVersion}'.`);
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "libs", "-R");

if (config.pathToTemplate) {
const mainPath = path.join(this.getPlatformData(projectData).projectRoot, "src", "main");
this.$fs.createDirectory(mainPath);
shell.cp("-R", path.join(path.resolve(config.pathToTemplate), "*"), mainPath);
this.isASTemplate = this.isAndroidStudioCompattibleTemplate(projectData);
if (this.isASTemplate) {
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "*", "-R");
} else {
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "src", "-R");
}
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "build.gradle settings.gradle build-tools", "-Rf");
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "libs", "-R");

try {
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradle.properties", "-Rf");
} catch (e) {
this.$logger.warn(`\n${e}\nIt's possible, the final .apk file will contain all architectures instead of the ones described in the abiFilters!\nYou can fix this by using the latest android platform.`);
}
if (config.pathToTemplate) {
const mainPath = path.join(this.getPlatformData(projectData).projectRoot, "src", "main");
this.$fs.createDirectory(mainPath);
shell.cp("-R", path.join(path.resolve(config.pathToTemplate), "*"), mainPath);
} else {
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "src", "-R");
}
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "build.gradle settings.gradle build-tools", "-Rf");

this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradle", "-R");
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradlew gradlew.bat", "-f");
try {
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradle.properties", "-Rf");
} catch (e) {
this.$logger.warn(`\n${e}\nIt's possible, the final .apk file will contain all architectures instead of the ones described in the abiFilters!\nYou can fix this by using the latest android platform.`);
}

this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradle", "-R");
this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "gradlew gradlew.bat", "-f");
}

this.cleanResValues(targetSdkVersion, projectData, frameworkVersion);

Expand Down Expand Up @@ -269,8 +305,11 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
buildOptions.unshift("--stacktrace");
buildOptions.unshift("--debug");
}

buildOptions.unshift("buildapk");
if (buildConfig.release) {
buildOptions.unshift("assembleRelease");
} else {
buildOptions.unshift("assembleDebug");
}

const handler = (data: any) => {
this.emit(constants.BUILD_OUTPUT_EVENT_NAME, data);
Expand Down Expand Up @@ -593,6 +632,28 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
const normalizedPlatformVersion = `${semver.major(platformVersion)}.${semver.minor(platformVersion)}.0`;
return semver.gte(normalizedPlatformVersion, newRuntimeGradleRoutineVersion);
}

private isAndroidStudioCompattibleTemplate(projectData: IProjectData): boolean {
const currentPlatformData: IDictionary<any> = this.$projectDataService.getNSValue(projectData.projectDir, constants.TNS_ANDROID_RUNTIME_NAME);
let platformVersion = currentPlatformData && currentPlatformData[constants.VERSION_STRING];

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 false;
}
}

const asCompatibleTemplate = "3.4.0";
Copy link
Contributor

Choose a reason for hiding this comment

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

We could name this androidStudioCompatibleTemplate

const normalizedPlatformVersion = `${semver.major(platformVersion)}.${semver.minor(platformVersion)}.0`;

return semver.gte(normalizedPlatformVersion, asCompatibleTemplate);
}
}

$injector.register("androidProjectService", AndroidProjectService);