Skip to content

Commit

Permalink
fix(cordova): handle new format of output.json files for Android buil…
Browse files Browse the repository at this point in the history
…ds (#4470)

fixes #4467
  • Loading branch information
imhoffd committed Jun 15, 2020
1 parent 174985f commit 689b886
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 13 deletions.
11 changes: 10 additions & 1 deletion packages/@ionic/cli/src/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,22 @@ export interface CordovaPackageJson extends PackageJson {
};
}

export interface CordovaAndroidBuildOutputEntry {
export interface LegacyAndroidBuildOutputEntry {
outputType: {
type: string;
};
path: string;
}

export interface AndroidBuildOutput {
artifactType: {
type: string;
};
elements: {
outputFile: string;
}[];
}

export interface Runner<T extends object, U> {
run(options: T): Promise<U>;
}
Expand Down
12 changes: 10 additions & 2 deletions packages/@ionic/cli/src/guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import {
APIResponse,
APIResponseError,
APIResponseSuccess,
AndroidBuildOutput,
App,
AppAssociation,
BitbucketCloudRepoAssociation,
BitbucketServerRepoAssociation,
CommandPreRun,
CordovaAndroidBuildOutputEntry,
CordovaPackageJson,
ExitCodeException,
GithubBranch,
Expand All @@ -17,6 +17,7 @@ import {
IMultiProjectConfig,
IProjectConfig,
IntegrationName,
LegacyAndroidBuildOutputEntry,
Login,
OpenIdToken,
Org,
Expand Down Expand Up @@ -55,7 +56,7 @@ export function isCordovaPackageJson(obj: any): obj is CordovaPackageJson {
typeof obj.cordova.plugins === 'object';
}

export function isCordovaAndroidBuildOutputFile(obj: any): obj is CordovaAndroidBuildOutputEntry[] {
export function isLegacyAndroidBuildOutputFile(obj: any): obj is LegacyAndroidBuildOutputEntry[] {
if (!Array.isArray(obj)) {
return false;
}
Expand All @@ -70,6 +71,13 @@ export function isCordovaAndroidBuildOutputFile(obj: any): obj is CordovaAndroid
&& typeof obj[0].outputType.type === 'string';
}

export function isAndroidBuildOutputFile(obj: any): obj is AndroidBuildOutput {
return obj &&
typeof obj.artifactType === 'object' &&
typeof obj.artifactType.type === 'string' &&
Array.isArray(obj.elements);
}

export function isExitCodeException(err: any): err is ExitCodeException {
return err && typeof err.exitCode === 'number' && err.exitCode >= 0 && err.exitCode <= 255;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,96 @@ describe('@ionic/cli', () => {

});

describe('getAndroidBuildOutputJson', () => {

it('should throw for fs error', () => {
spyOn(fsSpy, 'readJson').and.callFake(async () => { throw new Error('error') });

const p = project.getAndroidBuildOutputJson('/path/to/output.json');
expect(p).rejects.toThrowError('Could not parse build output file');
});

it('should throw for unrecognized format', () => {
spyOn(fsSpy, 'readJson').and.callFake(async () => ({ foo: 'bar' }));

const p = project.getAndroidBuildOutputJson('/path/to/output.json');
expect(p).rejects.toThrowError('Could not parse build output file');
});

it('should parse legacy output.json', () => {
const file = [
{
outputType: {
type: 'APK',
},
path: 'app-debug.apk',
},
];

spyOn(fsSpy, 'readJson').and.callFake(async () => file);

const p = project.getAndroidBuildOutputJson('/path/to/output.json');
expect(p).resolves.toEqual(file);
});

it('should parse output.json', () => {
const file = {
artifactType: {
type: 'APK',
},
elements: [
{
outputFile: 'app-debug.apk',
}
],
};

spyOn(fsSpy, 'readJson').and.callFake(async () => file);

const p = project.getAndroidBuildOutputJson('/path/to/output.json');
expect(p).resolves.toEqual(file);
});

});

describe('getAndroidPackageFilePath', () => {

it('should get file path from legacy output.json', () => {
const file = [
{
outputType: {
type: 'APK',
},
path: 'foo-debug.apk',
},
];

spyOn(fsSpy, 'readJson').and.callFake(async () => file);

const p = project.getAndroidPackageFilePath('/path/to/proj', { release: false });
expect(p).resolves.toEqual('platforms/android/app/build/outputs/apk/debug/foo-debug.apk');
});

it('should get file path from output.json', () => {
const file = {
artifactType: {
type: 'APK',
},
elements: [
{
outputFile: 'bar-debug.apk',
}
],
};

spyOn(fsSpy, 'readJson').and.callFake(async () => file);

const p = project.getAndroidPackageFilePath('/path/to/proj', { release: false });
expect(p).resolves.toEqual('platforms/android/app/build/outputs/apk/debug/bar-debug.apk');
});

});

});

});
30 changes: 20 additions & 10 deletions packages/@ionic/cli/src/lib/integrations/cordova/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { readJson, readdirSafe, statSafe } from '@ionic/utils-fs';
import * as Debug from 'debug';
import * as path from 'path';

import { CordovaAndroidBuildOutputEntry } from '../../../definitions';
import { isCordovaAndroidBuildOutputFile } from '../../../guards';
import { AndroidBuildOutput, LegacyAndroidBuildOutputEntry } from '../../../definitions';
import { isAndroidBuildOutputFile, isLegacyAndroidBuildOutputFile } from '../../../guards';
import { input } from '../../color';
import { FatalException } from '../../errors';

Expand All @@ -25,11 +25,13 @@ export async function getPlatforms(projectDir: string): Promise<string[]> {
return platforms;
}

export async function getAndroidBuildOutputJson(p: string): Promise<CordovaAndroidBuildOutputEntry[]> {
export async function getAndroidBuildOutputJson(p: string): Promise<LegacyAndroidBuildOutputEntry[] | AndroidBuildOutput> {
try {
const json = await readJson(p);

if (isCordovaAndroidBuildOutputFile(json)) {
if (isAndroidBuildOutputFile(json)) {
return json;
} else if (isLegacyAndroidBuildOutputFile(json)) {
return json;
} else {
debug('Output file does not match expected format: %O', json);
Expand All @@ -41,6 +43,19 @@ export async function getAndroidBuildOutputJson(p: string): Promise<CordovaAndro
throw new FatalException(`Could not parse build output file: ${p}`);
}

export async function getAndroidPackageFilePath(root: string, { release = false }: GetPackagePathOptions): Promise<string> {
const outputPath = path.resolve(root, CORDOVA_ANDROID_PACKAGE_PATH, release ? 'release' : 'debug');
const outputJsonPath = path.resolve(outputPath, 'output.json');
const outputJson = await getAndroidBuildOutputJson(outputJsonPath);

const p = 'elements' in outputJson
? outputJson.elements[0].outputFile
: outputJson[0].path;

// TODO: handle multiple files from output.json, prompt to select?
return path.relative(root, path.resolve(outputPath, p));
}

export interface GetPackagePathOptions {
emulator?: boolean;
release?: boolean;
Expand All @@ -51,12 +66,7 @@ export interface GetPackagePathOptions {
*/
export async function getPackagePath(root: string, appName: string, platform: string, { emulator = false, release = false }: GetPackagePathOptions = {}): Promise<string> {
if (platform === 'android') {
const outputPath = path.resolve(root, CORDOVA_ANDROID_PACKAGE_PATH, release ? 'release' : 'debug');
const outputJsonPath = path.resolve(outputPath, 'output.json');
const outputJson = await getAndroidBuildOutputJson(outputJsonPath);

// TODO: handle multiple files from output.json, prompt to select?
return path.relative(root, path.resolve(outputPath, outputJson[0].path));
return getAndroidPackageFilePath(root, { emulator, release });
} else if (platform === 'ios') {
if (emulator) {
return path.join(CORDOVA_IOS_SIMULATOR_PACKAGE_PATH, `${appName}.app`);
Expand Down

0 comments on commit 689b886

Please sign in to comment.