fix(apple): include check if Podfile and Podfile.lock changed when deciding to install Cocoapods

[szymonrybczak]

Summary:

Comparing only dependencies field from config's command output takes into account only autolinked dependencies, however users can add manually Pods inside Podfile, in this Pull Request I added validating hash for Podfile and Podfile.lock.

Test Plan:

1. Clone the repository and do all the required steps from the Contributing guide
2. Create react-native.config.js and enable automaticPodsInstallation
3. Whenever Podfile or Podfile.lock content changes installation of Cocoapods should be triggered

node /path/to/react-native-cli/packages/cli/build/bin.js init

Checklist

• Documentation is up to date to reflect these changes.
• Follows commit message convention described in CONTRIBUTING.md

[szymonrybczak]

cc @tido64, I know that you had some concerns related to this topic. I'd really appreciate your input here what else should we improve to solve your case.

[thymikee]

Can you include some tests please?

[TMisiukiewicz]

Could you check if it'd be faster to use checksum that is auto-generated inside Podfile.lock? I am wondering how it compares to your current implementation 🤔

[szymonrybczak]

Could you check if it'd be faster to use checksum that is auto-generated inside Podfile.lock? I am wondering how it compares to your current implementation 🤔

It turns out that hashing file is faster 👀

I've created two simple scripts that checks that:

Hashing file script

const fs = require('fs');
const { createHash } = require('crypto');

function generateMd5Hash(text) {
  return createHash('md5').update(text).digest('hex');
}

async function main() {
  const hash = generateMd5Hash(
    fs.readFileSync('_/ios/Podfile.lock', 'utf8'),
  );

  console.log(hash);
}

main();

Reading file and checking checksum

const fs = require('fs');
const readline = require('readline')

async function main() {
  const fileStream = fs.createReadStream('_/ios/Podfile.lock');

  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')) {
      console.log(line.split(': ')[1]);
    }
  }
}

main();

On fresh project when Podfile.lock contains ~1k lines, the results are pretty much the same

❯ wc -l ios/Podfile.lock
    1402 ios/Podfile.lock

❯ hyperfine "node hash.js"
Benchmark 1: node hash.js
  Time (mean ± σ):      42.1 ms ±   4.1 ms    [User: 34.2 ms, System: 5.9 ms]
  Range (min … max):    39.9 ms …  72.0 ms    63 runs

~/development/tmp/elo                                                          
❯ hyperfine "node checksum.js"
Benchmark 1: node checksum.js
  Time (mean ± σ):      46.7 ms ±  17.3 ms    [User: 37.6 ms, System: 6.5 ms]
  Range (min … max):    42.8 ms … 178.3 ms    60 runs

but I've created an example scenario when Podfile.lock contains ~60k lines, and creating hash is much faster:

❯ wc -l ios/Podfile.lock
   60286 ios/Podfile.lock
                                                        
❯ hyperfine "node hash.js"
Benchmark 1: node hash.js
  Time (mean ± σ):      45.9 ms ±   1.0 ms    [User: 37.7 ms, System: 6.9 ms]
  Range (min … max):    43.5 ms …  49.8 ms    57 runs
                                                        
❯ hyperfine "node checksum.js"
Benchmark 1: node checksum.js
  Time (mean ± σ):      67.1 ms ±   1.8 ms    [User: 72.1 ms, System: 13.4 ms]
  Range (min … max):    64.5 ms …  70.8 ms    39 runs

[tido64]

Could you check if it'd be faster to use checksum that is auto-generated inside Podfile.lock? I am wondering how it compares to your current implementation 🤔

It turns out that hashing file is faster 👀

Rather than speed, I think in this case I'd would much prefer that we are consistent with CocoaPods. If it thinks that the manifest needs to be updated, so should we, and vice versa. And I think the only way to ensure that is to reuse the checksum or CocoaPods directly.

[szymonrybczak]

Yeah, so I've added reading checksum after installing Cocoapods, so that new checksum is saved. Together with @TMisiukiewicz we did some investigations and it turns out that there are 3 scenarios with current implementation where we'll run pod install where it's not necessary:

1. someone install dependency and it's presented in config#dependencies output and then install Cocoapods with pod install manually, then CLI runs pod install because outdated checksum value is in cache and it's different than the one saved in Podfile.lock,
2. the same scenario as 1) but with adding pod directly inside Podfile,
3. someone has 3rd party scripts that is referenced in Podfile and it's adding dynamically Pods.

For 1), 2) I think we could add phase of updating cache values e.g. in post-install hook in pod install command (not sure if this is possible without changing Podfile in template right now).

For 3) I think it's really edge case-y and supporting this would solve very small amount of cases, or maybe even 0.

[cipolleschi]

Small nits, the logic sounds good to me!