Skip to content

fix: install pods on ios-commands with New Arch #2122

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
Show file tree
Hide file tree
Changes from all 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
12 changes: 4 additions & 8 deletions packages/cli-doctor/src/commands/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import getEnvironmentInfo from '../tools/envinfo';
import {logger, version} from '@react-native-community/cli-tools';
import {Config} from '@react-native-community/cli-types';
import {getArchitecture} from '@react-native-community/cli-platform-ios';
import {readFile} from 'fs-extra';
import path from 'path';
import {stringify} from 'yaml';
Expand Down Expand Up @@ -52,16 +53,11 @@ const info = async function getInfo(_argv: Array<string>, ctx: Config) {
}

try {
const project = await readFile(
path.join(
ctx.project.ios.sourceDir,
'/Pods/Pods.xcodeproj/project.pbxproj',
),
const isNewArchitecture = await getArchitecture(
ctx.project.ios.sourceDir,
);

platforms.iOS.newArchEnabled = project.includes(
'-DRCT_NEW_ARCH_ENABLED=1',
);
platforms.iOS.newArchEnabled = isNewArchitecture;
} catch {
platforms.iOS.newArchEnabled = notFound;
}
Expand Down
10 changes: 9 additions & 1 deletion packages/cli-platform-ios/src/commands/buildIOS/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,20 @@ import {BuildFlags, buildOptions} from './buildOptions';
import {getConfiguration} from './getConfiguration';
import {getXcodeProjectAndDir} from './getXcodeProjectAndDir';
import resolvePods from '../../tools/pods';
import getArchitecture from '../../tools/getArchitecture';

async function buildIOS(_: Array<string>, ctx: Config, args: BuildFlags) {
const {xcodeProject, sourceDir} = getXcodeProjectAndDir(ctx.project.ios);

const isAppRunningNewArchitecture = ctx.project.ios?.sourceDir
? await getArchitecture(ctx.project.ios?.sourceDir)
: undefined;

// check if pods need to be installed
await resolvePods(ctx.root, ctx.dependencies, {forceInstall: args.forcePods});
await resolvePods(ctx.root, ctx.dependencies, {
forceInstall: args.forcePods,
newArchEnabled: isAppRunningNewArchitecture,
});

process.chdir(sourceDir);

Expand Down
10 changes: 9 additions & 1 deletion packages/cli-platform-ios/src/commands/runIOS/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {promptForDeviceSelection} from '../../tools/prompts';
import getSimulators from '../../tools/getSimulators';
import {getXcodeProjectAndDir} from '../buildIOS/getXcodeProjectAndDir';
import resolvePods from '../../tools/pods';
import getArchitecture from '../../tools/getArchitecture';

export interface FlagsT extends BuildFlags {
simulator?: string;
Expand All @@ -48,8 +49,15 @@ async function runIOS(_: Array<string>, ctx: Config, args: FlagsT) {

let {packager, port} = args;

const isAppRunningNewArchitecture = ctx.project.ios?.sourceDir
? await getArchitecture(ctx.project.ios?.sourceDir)
: undefined;

// check if pods need to be installed
await resolvePods(ctx.root, ctx.dependencies, {forceInstall: args.forcePods});
await resolvePods(ctx.root, ctx.dependencies, {
forceInstall: args.forcePods,
newArchEnabled: isAppRunningNewArchitecture,
});

const packagerStatus = await isPackagerRunning(port);

Expand Down
1 change: 1 addition & 0 deletions packages/cli-platform-ios/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

export {default as commands} from './commands';
export {projectConfig, dependencyConfig, findPodfilePaths} from './config';
export {default as getArchitecture} from './tools/getArchitecture';
export {default as installPods} from './tools/installPods';
14 changes: 14 additions & 0 deletions packages/cli-platform-ios/src/tools/getArchitecture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {readFile} from 'fs-extra';
import path from 'path';

export default async function getArchitecture(iosSourceDir: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have the same logic in doctor command:

const project = await readFile(
path.join(
ctx.project.ios.sourceDir,
'/Pods/Pods.xcodeproj/project.pbxproj',
),
);
platforms.iOS.newArchEnabled = project.includes(
'-DRCT_NEW_ARCH_ENABLED=1',
);

mind using this function also there - so we won't have in two places same code? 🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's already here:

const isNewArchitecture = await getArchitecture(

try {
const project = await readFile(
path.join(iosSourceDir, '/Pods/Pods.xcodeproj/project.pbxproj'),
);

return project.includes('-DRCT_NEW_ARCH_ENABLED=1');
Copy link
Member

Choose a reason for hiding this comment

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

@cipolleschi is this fine if we use that to detect new arch?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, this should be enough.

} catch {
return false;
}
}
42 changes: 25 additions & 17 deletions packages/cli-platform-ios/src/tools/installPods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,29 @@ import runBundleInstall from './runBundleInstall';

interface PodInstallOptions {
skipBundleInstall?: boolean;
newArchEnabled?: boolean;
iosFolderPath?: string;
}

async function runPodInstall(
loader: Ora,
shouldHandleRepoUpdate: boolean = true,
) {
interface RunPodInstallOptions {
shouldHandleRepoUpdate?: boolean;
newArchEnabled?: boolean;
}

async function runPodInstall(loader: Ora, options?: RunPodInstallOptions) {
const shouldHandleRepoUpdate = options?.shouldHandleRepoUpdate || true;
try {
loader.start(
`Installing CocoaPods dependencies ${chalk.dim(
'(this may take a few minutes)',
)}`,
`Installing CocoaPods dependencies ${chalk.bold(
options?.newArchEnabled ? 'with New Architecture' : '',
)} ${chalk.dim('(this may take a few minutes)')}`,
);

await execa('bundle', ['exec', 'pod', 'install']);
await execa('bundle', ['exec', 'pod', 'install'], {
env: {
RCT_NEW_ARCH_ENABLED: options?.newArchEnabled ? '1' : '0',
},
});
} catch (error) {
// "pod" command outputs errors to stdout (at least some of them)
const stderr = (error as any).stderr || (error as any).stdout;
Expand All @@ -40,7 +49,10 @@ async function runPodInstall(
*/
if (stderr.includes('pod repo update') && shouldHandleRepoUpdate) {
await runPodUpdate(loader);
await runPodInstall(loader, false);
await runPodInstall(loader, {
shouldHandleRepoUpdate: false,
newArchEnabled: options?.newArchEnabled,
});
} else {
loader.fail();
logger.error(stderr);
Expand Down Expand Up @@ -110,18 +122,14 @@ async function installCocoaPods(loader: Ora) {
}
}

async function installPods(
loader?: Ora,
iosFolderPath?: string,
options?: PodInstallOptions,
) {
async function installPods(loader?: Ora, options?: PodInstallOptions) {
loader = loader || new NoopLoader();
try {
if (!iosFolderPath && !fs.existsSync('ios')) {
if (!options?.iosFolderPath && !fs.existsSync('ios')) {
return;
}

process.chdir(iosFolderPath ?? 'ios');
process.chdir(options?.iosFolderPath ?? 'ios');

const hasPods = fs.existsSync('Podfile');

Expand All @@ -143,7 +151,7 @@ async function installPods(
await installCocoaPods(loader);
}

await runPodInstall(loader);
await runPodInstall(loader, {newArchEnabled: options?.newArchEnabled});
} finally {
process.chdir('..');
}
Expand Down
31 changes: 24 additions & 7 deletions packages/cli-platform-ios/src/tools/pods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {

interface ResolvePodsOptions {
forceInstall?: boolean;
newArchEnabled?: boolean;
}

interface NativeDependencies {
Expand Down Expand Up @@ -65,8 +66,9 @@ async function install(
) {
const loader = getLoader('Installing CocoaPods...');
try {
await installPods(loader, iosFolderPath, {
await installPods(loader, {
skipBundleInstall: !!cachedDependenciesHash,
iosFolderPath,
});
cacheManager.set(packageJson.name, 'dependencies', currentDependenciesHash);
loader.succeed();
Expand Down Expand Up @@ -114,11 +116,26 @@ export default async function resolvePods(
!compareMd5Hashes(currentDependenciesHash, cachedDependenciesHash) ||
!arePodsInstalled
) {
await install(
packageJson,
cachedDependenciesHash,
currentDependenciesHash,
iosFolderPath,
);
const loader = getLoader('Installing CocoaPods...');
try {
await installPods(loader, {
skipBundleInstall: !!cachedDependenciesHash,
newArchEnabled: options?.newArchEnabled,
iosFolderPath,
});
cacheManager.set(
packageJson.name,
'dependencies',
currentDependenciesHash,
);
loader.succeed();
} catch {
loader.fail();
throw new CLIError(
`Something when wrong while installing CocoaPods. Please run ${chalk.bold(
'pod install',
)} manually`,
);
Comment on lines +134 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, I'm wondering should we add here info about flags that we added 🤔

}
}
}