build(deps): align android ndk version w/current template

[mikehardy]

Summary:

Currently template requires 21.4.xx https://github.com/facebook/react-native/blob/1b3581e245d1ae5f965f80c59dc57aece0a854e7/template/android/build.gradle#L9

This NDK check may need to be moved out of --contributor if NDK is required for TurboModules?

Test Plan:

CI should test this? It does not pass for me locally when I try it via npx react-native doctor --contributor because I have NDK installed side-by-side, and upstream envinfo needs to detect this:

https://github.com/tabrindle/envinfo/issues/218

[thymikee]

Thanks! Mind taking a look at the failing tests?

[mikehardy]

@thymikee sure! and a quick CC to @grabbou and @lucasbento just to mention to all at the same time that my work is kind of "burst-y", meaning I work on a lot of repos and kind of take them one at time giving each a time slice as I can. It may be a while before I come back to consider this but I wlil check it out (as well as @grabbou's thoughts from Discord on how to best align these checks with react-native versions) next chance I get

[mikehardy]

Had a moment - this needs deeper inspection, the test had a false positive? Or a broken assumption somewhere in the 2x2 matrix of "expect this boolean" / "got that boolean"

FAIL unit packages/cli-doctor/src/tools/healthchecks/__tests__/androidNDK.test.ts
  ● androidNDK › returns false if the NDK version is in range

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      47 |     };
      48 |     const diagnostics = await androidNDK.getDiagnostics(environmentInfo);
    > 49 |     expect(diagnostics.needsToBeFixed).toBe(false);
         |                                        ^
      50 |   });
      51 |
      52 |   it('logs manual installation steps to the screen', () => {

      at Object.<anonymous> (packages/cli-doctor/src/tools/healthchecks/__tests__/androidNDK.test.ts:49:[40](https://github.com/react-native-community/cli/runs/5160960139?check_suite_focus=true#step:5:40))
          at runMicrotasks (<anonymous>)

[vnovick]

@thymikee, verified that it's working

@mikehardy to fix the test, you need to change expected Android NDK version in androidNdk.test.ts:

line 45

    environmentInfo.SDKs['Android SDK'] = {
      'Android NDK': '21.4',
    };

[mikehardy]

This one should get some special attention for M1 machines, they apparently need a very new NDK - https://github.com/reactwg/react-native-releases/discussions/13#discussioncomment-2370415 (version 24.0.7956693)

[thymikee]

I'm good with setting this to >24 on the main branch :)

[mikehardy]

For my purposes doctor needs to know react native version and offer version specific recs / warnings since I don't only support unreleased nightlies..., but agreed on main / 0.69!

[cortinico]

Couple of thoughts here:

The fact that the template uses NDK 21 doesn't mean that the minimum supported NDK version is 21. We're building RN with NDK 17 internally. I haven't tested this on the OSS pipeline, but I believe that you could use older versions of the NDK to build RN.

This one should get some special attention for M1 machines, they apparently need a very new NDK - reactwg/react-native-releases#13 (reply in thread) (version 24.0.7956693)

Yup that's a problem. As we have several M1-specific patches, maybe having some sort of special handling for M1 machines inside doctor would help 👍

[mikehardy]

The fact that the template uses NDK 21 doesn't mean that the minimum supported NDK version is 21. We're building RN with NDK 17 internally. I haven't tested this on the OSS pipeline, but I believe that you could use older versions of the NDK to build RN.

I did not know that! Thanks, so alignment might be a warning (with a soft message, or maybe just an info) but definitely not a red X or anything apparently

This one should get some special attention for M1 machines, they apparently need a very new NDK - reactwg/react-native-releases#13 (reply in thread) (version 24.0.7956693)

Yup that's a problem. As we have several M1-specific patches, maybe having some sort of special handling for M1 machines inside doctor would help +1

Yeah - simply collecting the valid / invalid states is most of the work I think which is why I started to trying to write it all down here

Now to actually have some code I hope...

[mikehardy]

setting this to draft as it's clear it takes me a while to come back around and I used my periodic CLI time just now with some clean and jdk/sdk fixes. Will get the NDK hopefully at some point, including with M1 and whatever is needed for hermes now

[github-actions[bot]]

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.