From cb1c9b2fd0177a9d9a3cc25ebbdf1c0714784fb6 Mon Sep 17 00:00:00 2001 From: szymonrybczak Date: Wed, 3 Jul 2024 15:32:00 +0200 Subject: [PATCH 1/6] fix(apple): compare also `Podfile` and `Podfile.lock` when deciding to install Cocoapods --- .../src/commands/buildCommand/createBuild.ts | 14 +++++-- .../src/commands/runCommand/createRun.ts | 14 +++++-- packages/cli-platform-apple/src/tools/pods.ts | 38 +++++++++++++++---- packages/cli-tools/src/cacheManager.ts | 2 + 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/packages/cli-platform-apple/src/commands/buildCommand/createBuild.ts b/packages/cli-platform-apple/src/commands/buildCommand/createBuild.ts index 3e06b6c10..795c6bb07 100644 --- a/packages/cli-platform-apple/src/commands/buildCommand/createBuild.ts +++ b/packages/cli-platform-apple/src/commands/buildCommand/createBuild.ts @@ -27,10 +27,16 @@ const createBuild = ? await getArchitecture(platformConfig.sourceDir) : undefined; - await resolvePods(ctx.root, ctx.dependencies, platformName, { - forceInstall: args.forcePods, - newArchEnabled: isAppRunningNewArchitecture, - }); + await resolvePods( + ctx.root, + platformConfig.sourceDir, + ctx.dependencies, + platformName, + { + forceInstall: args.forcePods, + newArchEnabled: isAppRunningNewArchitecture, + }, + ); installedPods = true; } diff --git a/packages/cli-platform-apple/src/commands/runCommand/createRun.ts b/packages/cli-platform-apple/src/commands/runCommand/createRun.ts index 486b9ebf5..62525725b 100644 --- a/packages/cli-platform-apple/src/commands/runCommand/createRun.ts +++ b/packages/cli-platform-apple/src/commands/runCommand/createRun.ts @@ -72,10 +72,16 @@ const createRun = ? await getArchitecture(platformConfig.sourceDir) : undefined; - await resolvePods(ctx.root, ctx.dependencies, platformName, { - forceInstall: args.forcePods, - newArchEnabled: isAppRunningNewArchitecture, - }); + await resolvePods( + ctx.root, + platformConfig.sourceDir, + ctx.dependencies, + platformName, + { + forceInstall: args.forcePods, + newArchEnabled: isAppRunningNewArchitecture, + }, + ); installedPods = true; } diff --git a/packages/cli-platform-apple/src/tools/pods.ts b/packages/cli-platform-apple/src/tools/pods.ts index 2895c3d9f..c15f4c176 100644 --- a/packages/cli-platform-apple/src/tools/pods.ts +++ b/packages/cli-platform-apple/src/tools/pods.ts @@ -8,7 +8,6 @@ import { getLoader, } from '@react-native-community/cli-tools'; import installPods from './installPods'; -import findPodfilePath from '../config/findPodfilePath'; import { DependencyConfig, IOSDependencyConfig, @@ -61,7 +60,7 @@ export function generateMd5Hash(text: string) { return createHash('md5').update(text).digest('hex'); } -export function compareMd5Hashes(hash1: string, hash2: string) { +export function compareMd5Hashes(hash1?: string, hash2?: string) { return hash1 === hash2; } @@ -91,12 +90,15 @@ async function install( export default async function resolvePods( root: string, + sourceDir: string, nativeDependencies: NativeDependencies, platformName: ApplePlatform, options?: ResolvePodsOptions, ) { const packageJson = getPackageJson(root); - const podfilePath = findPodfilePath(root, platformName); + const podfilePath = path.join(sourceDir, 'Podfile'); // sourceDir is calculated based on Podfile location, see getProjectConfig() + + const podfileLockPath = path.join(sourceDir, 'Podfile.lock'); const platformFolderPath = podfilePath ? podfilePath.slice(0, podfilePath.lastIndexOf('/')) : path.join(root, platformName); @@ -108,6 +110,20 @@ export default async function resolvePods( ); const dependenciesString = dependenciesToString(platformDependencies); const currentDependenciesHash = generateMd5Hash(dependenciesString); + // Users can manually add dependencies to Podfile, so we can't entirely rely on `dependencies` from `config`'s output. + const currentPodfileHash = generateMd5Hash( + fs.readFileSync(podfilePath, 'utf8'), + ); + const currentPodfileLockHash = generateMd5Hash( + fs.readFileSync(podfileLockPath, 'utf8'), + ); + + const cachedPodfileHash = cacheManager.get(packageJson.name, 'podfile'); + const cachedPodfileLockHash = cacheManager.get( + packageJson.name, + 'podfileLock', + ); + const cachedDependenciesHash = cacheManager.get( packageJson.name, 'dependencies', @@ -120,13 +136,17 @@ export default async function resolvePods( currentDependenciesHash, platformFolderPath, ); - } else if (arePodsInstalled && cachedDependenciesHash === undefined) { - cacheManager.set(packageJson.name, 'dependencies', currentDependenciesHash); } else if ( - !cachedDependenciesHash || - !compareMd5Hashes(currentDependenciesHash, cachedDependenciesHash) || - !arePodsInstalled + arePodsInstalled && + compareMd5Hashes(currentDependenciesHash, cachedDependenciesHash) && + compareMd5Hashes(currentPodfileHash, cachedPodfileHash) && + compareMd5Hashes(currentPodfileLockHash, cachedPodfileLockHash) ) { + cacheManager.set(packageJson.name, 'dependencies', currentDependenciesHash); + cacheManager.set(packageJson.name, 'podfile', currentPodfileHash); + cacheManager.set(packageJson.name, 'podfileLock', currentPodfileLockHash); + } else { + console.log('eh'); const loader = getLoader('Installing CocoaPods...'); try { await installPods(loader, { @@ -139,6 +159,8 @@ export default async function resolvePods( 'dependencies', currentDependenciesHash, ); + cacheManager.set(packageJson.name, 'podfile', currentPodfileHash); + cacheManager.set(packageJson.name, 'podfileLock', currentPodfileLockHash); loader.succeed(); } catch { loader.fail(); diff --git a/packages/cli-tools/src/cacheManager.ts b/packages/cli-tools/src/cacheManager.ts index 227eb674a..dbcf48afe 100644 --- a/packages/cli-tools/src/cacheManager.ts +++ b/packages/cli-tools/src/cacheManager.ts @@ -10,6 +10,8 @@ type CacheKey = | 'lastChecked' | 'latestVersion' | 'dependencies' + | 'podfile' + | 'podfileLock' | 'lastUsedIOSDeviceId'; type Cache = {[key in CacheKey]?: string}; From 3be0257b017355a04de0f169afd0ea2f2bee7994 Mon Sep 17 00:00:00 2001 From: szymonrybczak Date: Thu, 4 Jul 2024 14:25:18 +0200 Subject: [PATCH 2/6] fix: save lock checksum instead of hash --- packages/cli-platform-apple/src/tools/pods.ts | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/packages/cli-platform-apple/src/tools/pods.ts b/packages/cli-platform-apple/src/tools/pods.ts index c15f4c176..059e8ac26 100644 --- a/packages/cli-platform-apple/src/tools/pods.ts +++ b/packages/cli-platform-apple/src/tools/pods.ts @@ -13,6 +13,7 @@ import { IOSDependencyConfig, } from '@react-native-community/cli-types'; import {ApplePlatform} from '../types'; +import readline from 'readline'; interface ResolvePodsOptions { forceInstall?: boolean; @@ -64,6 +65,30 @@ export function compareMd5Hashes(hash1?: string, hash2?: string) { return hash1 === hash2; } +async function getChecksum(podfileLockPath: string) { + const fileStream = fs.createReadStream(podfileLockPath); + + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }); + + let lines = []; + for await (const line of rl) { + lines.push(line); + } + + lines = lines.reverse(); + + for (const line of lines) { + if (line.includes('PODFILE CHECKSUM')) { + return line.split(': ')[1]; + } + } + + return undefined; +} + async function install( packageJson: Record, cachedDependenciesHash: string | undefined, @@ -78,12 +103,13 @@ async function install( }); cacheManager.set(packageJson.name, 'dependencies', currentDependenciesHash); loader.succeed(); - } catch { + } catch (error) { loader.fail(); throw new CLIError( `Something when wrong while installing CocoaPods. Please run ${chalk.bold( 'pod install', )} manually`, + error as Error, ); } } @@ -114,12 +140,10 @@ export default async function resolvePods( const currentPodfileHash = generateMd5Hash( fs.readFileSync(podfilePath, 'utf8'), ); - const currentPodfileLockHash = generateMd5Hash( - fs.readFileSync(podfileLockPath, 'utf8'), - ); + let currentPodfileLockChecksum = await getChecksum(podfileLockPath); const cachedPodfileHash = cacheManager.get(packageJson.name, 'podfile'); - const cachedPodfileLockHash = cacheManager.get( + const cachedPodfileLockChecksum = cacheManager.get( packageJson.name, 'podfileLock', ); @@ -140,13 +164,16 @@ export default async function resolvePods( arePodsInstalled && compareMd5Hashes(currentDependenciesHash, cachedDependenciesHash) && compareMd5Hashes(currentPodfileHash, cachedPodfileHash) && - compareMd5Hashes(currentPodfileLockHash, cachedPodfileLockHash) + compareMd5Hashes(currentPodfileLockChecksum, cachedPodfileLockChecksum) ) { cacheManager.set(packageJson.name, 'dependencies', currentDependenciesHash); cacheManager.set(packageJson.name, 'podfile', currentPodfileHash); - cacheManager.set(packageJson.name, 'podfileLock', currentPodfileLockHash); + cacheManager.set( + packageJson.name, + 'podfileLock', + currentPodfileLockChecksum ?? '', + ); } else { - console.log('eh'); const loader = getLoader('Installing CocoaPods...'); try { await installPods(loader, { @@ -160,14 +187,21 @@ export default async function resolvePods( currentDependenciesHash, ); cacheManager.set(packageJson.name, 'podfile', currentPodfileHash); - cacheManager.set(packageJson.name, 'podfileLock', currentPodfileLockHash); + // We need to read again the checksum because value changed after running `pod install` + currentPodfileLockChecksum = await getChecksum(podfileLockPath); + cacheManager.set( + packageJson.name, + 'podfileLock', + currentPodfileLockChecksum ?? '', + ); loader.succeed(); - } catch { + } catch (error) { loader.fail(); throw new CLIError( `Something when wrong while installing CocoaPods. Please run ${chalk.bold( 'pod install', )} manually`, + error as Error, ); } } From bfdfac922ef00c7db77f69ee974441c4c1dcf3f7 Mon Sep 17 00:00:00 2001 From: Szymon Rybczak Date: Tue, 9 Jul 2024 14:23:48 +0200 Subject: [PATCH 3/6] Update packages/cli-platform-apple/src/tools/pods.ts Co-authored-by: Riccardo Cipolleschi --- packages/cli-platform-apple/src/tools/pods.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/cli-platform-apple/src/tools/pods.ts b/packages/cli-platform-apple/src/tools/pods.ts index 059e8ac26..c5deed972 100644 --- a/packages/cli-platform-apple/src/tools/pods.ts +++ b/packages/cli-platform-apple/src/tools/pods.ts @@ -80,11 +80,9 @@ async function getChecksum(podfileLockPath: string) { lines = lines.reverse(); - for (const line of lines) { - if (line.includes('PODFILE CHECKSUM')) { - return line.split(': ')[1]; - } - } + return lines + .filter((line) => line.includes('PODFILE CHECKSUM'))[0] + .split(': ')[1] return undefined; } From 5abea6e43d9b706e1bad19803baed31505c4ec5d Mon Sep 17 00:00:00 2001 From: szymonrybczak Date: Tue, 9 Jul 2024 14:40:28 +0200 Subject: [PATCH 4/6] fix: simplify receiving checksum from `Podfile` --- packages/cli-platform-apple/src/tools/pods.ts | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/packages/cli-platform-apple/src/tools/pods.ts b/packages/cli-platform-apple/src/tools/pods.ts index c5deed972..495ad52fd 100644 --- a/packages/cli-platform-apple/src/tools/pods.ts +++ b/packages/cli-platform-apple/src/tools/pods.ts @@ -13,7 +13,6 @@ import { IOSDependencyConfig, } from '@react-native-community/cli-types'; import {ApplePlatform} from '../types'; -import readline from 'readline'; interface ResolvePodsOptions { forceInstall?: boolean; @@ -65,26 +64,20 @@ export function compareMd5Hashes(hash1?: string, hash2?: string) { return hash1 === hash2; } -async function getChecksum(podfileLockPath: string) { - const fileStream = fs.createReadStream(podfileLockPath); +async function getChecksum( + podfileLockPath: string, +): Promise { + const file = fs.readFileSync(podfileLockPath, 'utf8'); - const rl = readline.createInterface({ - input: fileStream, - crlfDelay: Infinity, - }); + const checksumLine = file + .split('\n') + .find((line) => line.includes('PODFILE CHECKSUM')); - let lines = []; - for await (const line of rl) { - lines.push(line); + if (checksumLine) { + return checksumLine.split(': ')[1]; + } else { + return undefined; } - - lines = lines.reverse(); - - return lines - .filter((line) => line.includes('PODFILE CHECKSUM'))[0] - .split(': ')[1] - - return undefined; } async function install( From b99f2bc88f330197bd6880fa6764abb347b6bf4f Mon Sep 17 00:00:00 2001 From: szymonrybczak Date: Sat, 31 Aug 2024 18:41:45 +0200 Subject: [PATCH 5/6] fix: return undefined if `Podfile.lock` not found --- packages/cli-platform-apple/src/tools/pods.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/cli-platform-apple/src/tools/pods.ts b/packages/cli-platform-apple/src/tools/pods.ts index 495ad52fd..117bf10b8 100644 --- a/packages/cli-platform-apple/src/tools/pods.ts +++ b/packages/cli-platform-apple/src/tools/pods.ts @@ -67,15 +67,19 @@ export function compareMd5Hashes(hash1?: string, hash2?: string) { async function getChecksum( podfileLockPath: string, ): Promise { - const file = fs.readFileSync(podfileLockPath, 'utf8'); + try { + const file = fs.readFileSync(podfileLockPath, 'utf8'); - const checksumLine = file - .split('\n') - .find((line) => line.includes('PODFILE CHECKSUM')); + const checksumLine = file + .split('\n') + .find((line) => line.includes('PODFILE CHECKSUM')); - if (checksumLine) { - return checksumLine.split(': ')[1]; - } else { + if (checksumLine) { + return checksumLine.split(': ')[1]; + } + + return undefined; + } catch { return undefined; } } From bc02e8e495e0895be204f85be155140ca483c72e Mon Sep 17 00:00:00 2001 From: szymonrybczak Date: Sat, 31 Aug 2024 18:42:17 +0200 Subject: [PATCH 6/6] fix: sync unit tests with current impl --- .../src/__tests__/pods.test.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/cli-platform-apple/src/__tests__/pods.test.ts b/packages/cli-platform-apple/src/__tests__/pods.test.ts index 00b9d5094..9ba0a8112 100644 --- a/packages/cli-platform-apple/src/__tests__/pods.test.ts +++ b/packages/cli-platform-apple/src/__tests__/pods.test.ts @@ -1,3 +1,4 @@ +import path from 'path'; import {writeFiles, getTempDirectory, cleanup} from '../../../../jest/helpers'; import installPods from '../tools/installPods'; import resolvePods, { @@ -51,6 +52,7 @@ const DIR = getTempDirectory('root_test'); const createTempFiles = (rest?: Record) => { writeFiles(DIR, { 'package.json': JSON.stringify(packageJson), + 'ios/Podfile': '', ...rest, }); }; @@ -83,9 +85,9 @@ describe('getPlatformDependencies', () => { describe('resolvePods', () => { it('should install pods if they are not installed', async () => { - createTempFiles({'ios/Podfile/Manifest.lock': ''}); + createTempFiles(); - await resolvePods(DIR, {}, 'ios'); + await resolvePods(DIR, path.join(DIR, 'ios'), {}, 'ios'); expect(installPods).toHaveBeenCalled(); }); @@ -93,7 +95,9 @@ describe('resolvePods', () => { it('should install pods when force option is set to true', async () => { createTempFiles(); - await resolvePods(DIR, {}, 'ios', {forceInstall: true}); + await resolvePods(DIR, path.join(DIR, 'ios'), {}, 'ios', { + forceInstall: true, + }); expect(installPods).toHaveBeenCalled(); }); @@ -101,7 +105,7 @@ describe('resolvePods', () => { it('should install pods when there is no cached hash of dependencies', async () => { createTempFiles(); - await resolvePods(DIR, {}, 'ios'); + await resolvePods(DIR, path.join(DIR, 'ios'), {}, 'ios'); expect(mockSet).toHaveBeenCalledWith( packageJson.name, @@ -111,22 +115,26 @@ describe('resolvePods', () => { }); it('should skip pods installation if the cached hash and current hash are the same', async () => { - createTempFiles({'ios/Pods/Manifest.lock': ''}); + createTempFiles({ + 'ios/Pods/Manifest.lock': '', + 'ios/Podfile.lock': `PODFILE CHECKSUM: ${dependencyHash}`, + }); mockGet.mockImplementation(() => dependencyHash); - await resolvePods(DIR, {}, 'ios'); + await resolvePods(DIR, path.join(DIR, 'ios'), {}, 'ios'); expect(installPods).not.toHaveBeenCalled(); }); it('should install pods if the cached hash and current hash are different', async () => { - createTempFiles({'ios/Pods/Manifest.lock': ''}); + createTempFiles(); mockGet.mockImplementation(() => dependencyHash); await resolvePods( DIR, + path.join(DIR, 'ios'), { dep1: { name: 'dep1',