-
-
Notifications
You must be signed in to change notification settings - Fork 206
feat(android): build only the ABIs of the devices being deployed to #6130
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -47,6 +47,8 @@ import { | |||||||||||||||
| import { IInjector } from "../common/definitions/yok"; | ||||||||||||||||
| import { injector } from "../common/yok"; | ||||||||||||||||
| import { INotConfiguredEnvOptions } from "../common/definitions/commands"; | ||||||||||||||||
| import { AndroidPrepareData } from "../data/prepare-data"; | ||||||||||||||||
| import { IProjectChangesInfo } from "../definitions/project-changes"; | ||||||||||||||||
|
|
||||||||||||||||
| interface NativeDependency { | ||||||||||||||||
| name: string; | ||||||||||||||||
|
|
@@ -148,7 +150,9 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject | |||||||||||||||
| private $androidPluginBuildService: IAndroidPluginBuildService, | ||||||||||||||||
| private $platformEnvironmentRequirements: IPlatformEnvironmentRequirements, | ||||||||||||||||
| private $androidResourcesMigrationService: IAndroidResourcesMigrationService, | ||||||||||||||||
| private $devicesService: Mobile.IDevicesService, | ||||||||||||||||
| private $filesHashService: IFilesHashService, | ||||||||||||||||
| private $liveSyncProcessDataService: ILiveSyncProcessDataService, | ||||||||||||||||
| private $gradleCommandService: IGradleCommandService, | ||||||||||||||||
| private $gradleBuildService: IGradleBuildService, | ||||||||||||||||
| private $analyticsService: IAnalyticsService | ||||||||||||||||
|
|
@@ -835,8 +839,61 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject | |||||||||||||||
| await adb.executeShellCommand(["rm", "-rf", deviceRootPath]); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public async checkForChanges(): Promise<void> { | ||||||||||||||||
| // Nothing android specific to check yet. | ||||||||||||||||
| /** | ||||||||||||||||
| * When the native build is narrowed down to the ABIs of the connected | ||||||||||||||||
| * devices, a device that joins later has no package of its own in the build | ||||||||||||||||
| * output. Nothing else would trigger a native rebuild for it - the sources | ||||||||||||||||
| * did not change - so flag it here. | ||||||||||||||||
| */ | ||||||||||||||||
| public async checkForChanges( | ||||||||||||||||
| changesInfo: IProjectChangesInfo, | ||||||||||||||||
| prepareData: AndroidPrepareData, | ||||||||||||||||
| projectData: IProjectData | ||||||||||||||||
| ): Promise<void> { | ||||||||||||||||
| if (changesInfo.nativeChanged) { | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const platformData = this.getPlatformData(projectData); | ||||||||||||||||
| const deviceDescriptors = this.$liveSyncProcessDataService.getDeviceDescriptors( | ||||||||||||||||
| projectData.projectDir | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| for (const deviceDescriptor of deviceDescriptors) { | ||||||||||||||||
| const buildData = <IAndroidBuildData>deviceDescriptor.buildData; | ||||||||||||||||
| if (!buildData || !buildData.buildFilterDevicesArch) { | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const packagesOutputPath = platformData.getBuildOutputPath(buildData); | ||||||||||||||||
| if (!this.$fs.exists(packagesOutputPath)) { | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const builtPackages = this.$fs.readDirectory(packagesOutputPath); | ||||||||||||||||
| // a universal package runs on every device, nothing to rebuild | ||||||||||||||||
| if (_.some(builtPackages, (f) => f.indexOf("universal") !== -1)) { | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const device = _.find( | ||||||||||||||||
| this.$devicesService.getDevicesForPlatform(buildData.platform), | ||||||||||||||||
| (d) => d.deviceInfo.identifier === deviceDescriptor.identifier | ||||||||||||||||
| ); | ||||||||||||||||
| const abi = device && (device.deviceInfo.abis || [])[0]; | ||||||||||||||||
| if (!abi) { | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const abiRegex = new RegExp(`${abi}.*\\.apk$`); | ||||||||||||||||
| if (!_.some(builtPackages, (entry) => abiRegex.test(entry))) { | ||||||||||||||||
|
Comment on lines
+888
to
+889
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Match the ABI as an exact package token. The current expression treats Escape the ABI and require output-name separators around it. Add a regression case with Proposed fix- const abiRegex = new RegExp(`${abi}.*\\.apk$`);
+ const escapedAbi = _.escapeRegExp(abi);
+ const abiRegex = new RegExp(
+ `(?:^|-)${escapedAbi}(?:-|(?=\\.apk$)).*\\.apk$`
+ );📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| this.$logger.trace( | ||||||||||||||||
| `No package was built for '${abi}', marking the native project as changed.` | ||||||||||||||||
| ); | ||||||||||||||||
| changesInfo.nativeChanged = true; | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public getDeploymentTarget(projectData: IProjectData): semver.SemVer { | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,12 +9,14 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IAndroidBuildData } from "../../definitions/build"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IChildProcess } from "../../common/declarations"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { injector } from "../../common/yok"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as _ from "lodash"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class GradleBuildService | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extends EventEmitter | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| implements IGradleBuildService { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private $childProcess: IChildProcess, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private $devicesService: Mobile.IDevicesService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private $gradleBuildArgsService: IGradleBuildArgsService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private $gradleCommandService: IGradleCommandService | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,6 +30,9 @@ export class GradleBuildService | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const buildTaskArgs = await this.$gradleBuildArgsService.getBuildTaskArgs( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildData | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.applyDevicesAbiFilter(buildTaskArgs, buildData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const spawnOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emitOptions: { eventName: constants.BUILD_OUTPUT_EVENT_NAME }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throwError: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -51,6 +56,47 @@ export class GradleBuildService | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Narrows the native build down to the ABIs of the devices this build is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * about to be deployed to. The app's gradle configuration decides what to do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * with `abiFilters` - typically an `ndk.abiFilters`/`splits` block in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `App_Resources/Android/app.gradle`. An explicitly passed `-PabiFilters` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * always wins. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private applyDevicesAbiFilter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildTaskArgs: string[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildData: IAndroidBuildData | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!buildData.buildFilterDevicesArch) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_.some(buildTaskArgs, (arg) => arg.startsWith("-PabiFilters"))) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let devices = this.$devicesService.getDevicesForPlatform( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildData.platform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (buildData.device) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| devices = devices.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (d) => d.deviceInfo.identifier === buildData.device | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (buildData.emulator) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| devices = devices.filter((d) => d.isEmulator); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const abis = _.uniq( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| devices | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((d) => (d.deviceInfo.abis || [])[0]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((abi) => !!abi) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (abis.length) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildTaskArgs.push(`-PabiFilters=${abis.join(",")}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Skip device ABI filtering for AAB builds.
Proposed fix- if (!buildData.buildFilterDevicesArch) {
+ if (!buildData.buildFilterDevicesArch || buildData.aab) {
return;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public async cleanProject( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectRoot: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildData: IAndroidBuildData | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -75,7 +75,12 @@ export class BuildArtifactsService implements IBuildArtifactsService { | |||||||||||||||||||||||||||||||||||||||||||
| return []; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| public copyLatestAppPackage( | ||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * Copies what the build produced to `targetPath`. A build can produce more | ||||||||||||||||||||||||||||||||||||||||||||
| * than one package - an app split per ABI - so a directory target receives | ||||||||||||||||||||||||||||||||||||||||||||
| * all of them, while a single file target receives the universal one. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
| public copyAppPackages( | ||||||||||||||||||||||||||||||||||||||||||||
| targetPath: string, | ||||||||||||||||||||||||||||||||||||||||||||
| platformData: IPlatformData, | ||||||||||||||||||||||||||||||||||||||||||||
| buildOutputOptions: IBuildOutputOptions | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -85,26 +90,36 @@ export class BuildArtifactsService implements IBuildArtifactsService { | |||||||||||||||||||||||||||||||||||||||||||
| const outputPath = | ||||||||||||||||||||||||||||||||||||||||||||
| buildOutputOptions.outputPath || | ||||||||||||||||||||||||||||||||||||||||||||
| platformData.getBuildOutputPath(buildOutputOptions); | ||||||||||||||||||||||||||||||||||||||||||||
| const applicationPackage = this.getLatestApplicationPackage( | ||||||||||||||||||||||||||||||||||||||||||||
| const applicationPackages = this.getAllAppPackages( | ||||||||||||||||||||||||||||||||||||||||||||
| outputPath, | ||||||||||||||||||||||||||||||||||||||||||||
| platformData.getValidBuildOutputData(buildOutputOptions) | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
| const packageFile = applicationPackage.packageName; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.ensureDirectoryExists(path.dirname(targetPath)); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.exists(targetPath) && | ||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.getFsStats(targetPath).isDirectory() | ||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||
| const sourceFileName = path.basename(packageFile); | ||||||||||||||||||||||||||||||||||||||||||||
| const targetIsDirectory = | ||||||||||||||||||||||||||||||||||||||||||||
| (this.$fs.exists(targetPath) && | ||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.getFsStats(targetPath).isDirectory()) || | ||||||||||||||||||||||||||||||||||||||||||||
| !path.extname(targetPath); | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
98
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Create a new directory target before copying packages. When Proposed fix const targetIsDirectory =
(this.$fs.exists(targetPath) &&
this.$fs.getFsStats(targetPath).isDirectory()) ||
!path.extname(targetPath);
+
+ if (targetIsDirectory) {
+ this.$fs.ensureDirectoryExists(targetPath);
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| let packagesToCopy = applicationPackages; | ||||||||||||||||||||||||||||||||||||||||||||
| if (!targetIsDirectory && applicationPackages.length > 1) { | ||||||||||||||||||||||||||||||||||||||||||||
| this.$logger.trace( | ||||||||||||||||||||||||||||||||||||||||||||
| `Specified target path: '${targetPath}' is directory. Same filename will be used: '${sourceFileName}'.` | ||||||||||||||||||||||||||||||||||||||||||||
| `Specified target path: '${targetPath}' is a single file, but the build produced ${applicationPackages.length} packages. Only the universal one will be copied.` | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
| packagesToCopy = applicationPackages.filter((pack) => | ||||||||||||||||||||||||||||||||||||||||||||
| path.basename(pack.packageName).includes("universal") | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+105
to
112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Do not silently skip a single-file copy without a universal APK. When ABI splits exist without a universal APK, this filter returns no packages. The command then succeeds without creating 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| targetPath = path.join(targetPath, sourceFileName); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.copyFile(packageFile, targetPath); | ||||||||||||||||||||||||||||||||||||||||||||
| this.$logger.info(`Copied file '${packageFile}' to '${targetPath}'.`); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| _.each(packagesToCopy, (pack) => { | ||||||||||||||||||||||||||||||||||||||||||||
| const packageFile = pack.packageName; | ||||||||||||||||||||||||||||||||||||||||||||
| const targetFilePath = targetIsDirectory | ||||||||||||||||||||||||||||||||||||||||||||
| ? path.join(targetPath, path.basename(packageFile)) | ||||||||||||||||||||||||||||||||||||||||||||
| : targetPath; | ||||||||||||||||||||||||||||||||||||||||||||
| this.$fs.copyFile(packageFile, targetFilePath); | ||||||||||||||||||||||||||||||||||||||||||||
| this.$logger.info(`Copied file '${packageFile}' to '${targetFilePath}'.`); | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private getLatestApplicationPackage( | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe selected devices, not all connected devices.
When the user passes
--deviceor--emulator, the build uses only the matching target devices. Replace “connected devices” with “selected target devices” to avoid an incorrect ABI-filter expectation.🤖 Prompt for AI Agents