From c7eb1e62b8fef2adaaf3845fe8af83b3d6c4ef32 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Wed, 18 Sep 2024 10:13:19 -0500 Subject: [PATCH] fix(macCatalyst): construct correct path for executable - targetBuildDir var now has `-maccatalyst` in it before it gets to this method, so no need to add it again - gracefully handle if -maccatalyst exists or not as area regresses frequently - fail fast with error message if targetBuildDir does not exist at all - add unit test for the with and without -maccatalyst case plus verify ios did not regress --- .../runCommand/__tests__/getBuildPath.test.ts | 97 +++++++++++++++++++ .../src/commands/runCommand/getBuildPath.ts | 32 ++++-- 2 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 packages/cli-platform-apple/src/commands/runCommand/__tests__/getBuildPath.test.ts diff --git a/packages/cli-platform-apple/src/commands/runCommand/__tests__/getBuildPath.test.ts b/packages/cli-platform-apple/src/commands/runCommand/__tests__/getBuildPath.test.ts new file mode 100644 index 000000000..b6b5e35ef --- /dev/null +++ b/packages/cli-platform-apple/src/commands/runCommand/__tests__/getBuildPath.test.ts @@ -0,0 +1,97 @@ +import path from 'path'; +import fs from 'fs'; +import {getTempDirectory} from '../../../../../../jest/helpers'; +import {BuildSettings} from '../getBuildSettings'; +import {getBuildPath} from '../getBuildPath'; + +const targetBuildDirName = 'foo'; +const targetBuildDirNameWithMaccatalyst = `${targetBuildDirName}-maccatalyst`; +const executableFolderPath = path.join('foo.app', 'Contents', 'MacOS', 'foo'); + +test('correctly determines macCatalyst build artifact path new style', async () => { + // setup: + const tmpBuildPath = getTempDirectory('maccatalyst-test-dir'); + fs.mkdirSync(path.join(tmpBuildPath, targetBuildDirNameWithMaccatalyst), { + recursive: true, + }); + + // - create buildSettings object that represents this to CLI + const buildSettings: BuildSettings = { + TARGET_BUILD_DIR: path.join( + tmpBuildPath, + targetBuildDirNameWithMaccatalyst, + ), + EXECUTABLE_FOLDER_PATH: executableFolderPath, + FULL_PRODUCT_NAME: 'unused-in-this-test', + INFOPLIST_PATH: 'unused-in-this-test', + }; + + // test: + // - send our buildSettings in and see what build path comes out + const buildPath = await getBuildPath(buildSettings, 'ios', true); + + // assert: + expect(buildPath).toBe( + path.join( + tmpBuildPath, + targetBuildDirNameWithMaccatalyst, + executableFolderPath, + ), + ); +}); + +test('correctly determines macCatalyst build artifact path old style', async () => { + // setup: + const tmpBuildPath = getTempDirectory('maccatalyst-test-dir'); + fs.mkdirSync(path.join(tmpBuildPath, targetBuildDirNameWithMaccatalyst), { + recursive: true, + }); + + // - create buildSettings object that represents this to CLI + // FIXME get the build settings as side effect from project definition, + // because it's the translation of project settings to path that fails + const buildSettings: BuildSettings = { + TARGET_BUILD_DIR: path.join(tmpBuildPath, targetBuildDirName), + EXECUTABLE_FOLDER_PATH: executableFolderPath, + FULL_PRODUCT_NAME: 'unused-in-this-test', + INFOPLIST_PATH: 'unused-in-this-test', + }; + + // test: + // - send our buildSettings in and see what build path comes out + const buildPath = await getBuildPath(buildSettings, 'ios', true); + + // assert: + expect(buildPath).toBe( + path.join( + tmpBuildPath, + targetBuildDirNameWithMaccatalyst, + executableFolderPath, + ), + ); +}); + +test('correctly determines iOS build artifact path', async () => { + // setup: + const tmpBuildPath = getTempDirectory('ios-test-dir'); + fs.mkdirSync(path.join(tmpBuildPath, targetBuildDirName), { + recursive: true, + }); + + // - create buildSettings object that represents this to CLI + const buildSettings: BuildSettings = { + TARGET_BUILD_DIR: path.join(tmpBuildPath, targetBuildDirName), + EXECUTABLE_FOLDER_PATH: executableFolderPath, + FULL_PRODUCT_NAME: 'unused-in-this-test', + INFOPLIST_PATH: 'unused-in-this-test', + }; + + // test: + // - send our buildSettings in and see what build path comes out + const buildPath = await getBuildPath(buildSettings); + + // assert: + expect(buildPath).toBe( + path.join(tmpBuildPath, targetBuildDirName, executableFolderPath), + ); +}); diff --git a/packages/cli-platform-apple/src/commands/runCommand/getBuildPath.ts b/packages/cli-platform-apple/src/commands/runCommand/getBuildPath.ts index 670fb788b..cad7e1d48 100644 --- a/packages/cli-platform-apple/src/commands/runCommand/getBuildPath.ts +++ b/packages/cli-platform-apple/src/commands/runCommand/getBuildPath.ts @@ -1,5 +1,6 @@ import {CLIError} from '@react-native-community/cli-tools'; import path from 'path'; +import fs from 'fs'; import {BuildSettings} from './getBuildSettings'; import {ApplePlatform} from '../../types'; @@ -8,7 +9,7 @@ export async function getBuildPath( platform: ApplePlatform = 'ios', isCatalyst: boolean = false, ) { - const targetBuildDir = buildSettings.TARGET_BUILD_DIR; + let targetBuildDir = buildSettings.TARGET_BUILD_DIR; const executableFolderPath = buildSettings.EXECUTABLE_FOLDER_PATH; const fullProductName = buildSettings.FULL_PRODUCT_NAME; @@ -24,11 +25,30 @@ export async function getBuildPath( throw new CLIError('Failed to get product name.'); } + // Default is iOS, the most common build + let buildPath = path.join(targetBuildDir, executableFolderPath); + + // catalyst builds need a suffix, but this regresses from time to time + // so check - there may be one already, or we may need to add suffix if (isCatalyst) { - return path.join(`${targetBuildDir}-maccatalyst`, executableFolderPath); - } else if (platform === 'macos') { - return path.join(targetBuildDir, fullProductName); - } else { - return path.join(targetBuildDir, executableFolderPath); + // make sure path has one and only one '-maccatalyst' on end + if (!targetBuildDir.match(/-maccatalyst$/)) { + targetBuildDir = `${targetBuildDir}-maccatalyst`; + } + buildPath = path.join(targetBuildDir, executableFolderPath); + } + + // macOS gets the product name, not the executable folder path + if (platform === 'macos') { + buildPath = path.join(targetBuildDir, fullProductName); } + + // Make sure the directory exists and fail fast vs silently failing + if (!fs.existsSync(targetBuildDir)) { + throw new CLIError( + `target build directory ${targetBuildDir} does not exist`, + ); + } + + return buildPath; }