cli icon indicating copy to clipboard operation
cli copied to clipboard

fix(macCatalyst): construct correct path for executable

Open mikehardy opened this issue 1 year ago • 5 comments

Summary:

it appears targetBuildDir var now has -maccatalyst in it before it gets to this method, so no need to add it again

This was tweaked in #2312 but appears to have regressed again - are macCatalyst builds checked in CI 🤔 ?

Without this change, the path is incorrect (note the duplicate -maccatalyst in path):


Error: spawn /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-gsuyjvwnbefbzvbfqtovnhthepew/Build/Products/Debug-maccatalyst-maccatalyst/rnfbdemo.app/Contents/MacOS/rnfbdemo ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:284:19)
    at onErrorNT (node:internal/child_process:477:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
Emitted 'error' event on ChildProcess instance at:
    at ChildProcess._handle.onexit (node:internal/child_process:290:12)
    at onErrorNT (node:internal/child_process:477:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-gsuyjvwnbefbzvbfqtovnhthepew/Build/Products/Debug-maccatalyst-maccatalyst/rnfbdemo.app/Contents/MacOS/rnfbdemo',
  path: '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-gsuyjvwnbefbzvbfqtovnhthepew/Build/Products/Debug-maccatalyst-maccatalyst/rnfbdemo.app/Contents/MacOS/rnfbdemo',
  spawnargs: []
}

Highlight:

/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-gsuyjvwnbefbzvbfqtovnhthepew/Build/Products/Debug-maccatalyst-maccatalyst/rnfbdemo.app/Contents/MacOS/rnfbdemo

Test Plan:

I build and test with macCatalyst all the time: https://github.com/mikehardy/rnfbdemo/blob/main/make-demo.sh

I added a unit test that exercises the method, sending it old (without -maccatalyst) and new (with -maccatalyst) inputs to verify it handles both, and verify I didn't regress the more common ios build code path

Checklist

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

mikehardy avatar Sep 18 '24 15:09 mikehardy

Note: this was against CLI v14.1.0 - I'm currently making sure rn75 works This may be changed in rn76 RCs - I'll be checking those later

mikehardy avatar Sep 18 '24 15:09 mikehardy

are macCatalyst builds checked in CI 🤔 ?

Unfortunately no :(

Note: this was against CLI v14.1.0 - I'm currently making sure rn75 works This may be changed in rn76 RCs - I'll be checking those later

Maybe to make this solution more future-proof we'll fallback to other value if initial one fails? 🤔

szymonrybczak avatar Sep 18 '24 17:09 szymonrybczak

I guess it is hard in CI, since you need an Xcode development team to do a macCatalyst build - it was pretty annoying to set up in my build test rig, at least 😅

Would you like me to alter the patch to actually check existence of the path so it can try both and return the one that works ?

mikehardy avatar Sep 18 '24 17:09 mikehardy

I guess it is hard in CI, since you need an Xcode development team to do a macCatalyst build - it was pretty annoying to set up in my build test rig, at least 😅

Ah, right 😞

Would you like me to alter the patch to actually check existence of the path so it can try both and return the one that works ?

Yes, IMO that's the safest solution.

szymonrybczak avatar Sep 18 '24 19:09 szymonrybczak

I added a unit test and handle both cases now, force-pushed, and while the idea is roughly the same code is now different so re-requesting review, thanks you all

mikehardy avatar Sep 19 '24 21:09 mikehardy

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.

github-actions[bot] avatar Feb 04 '25 03:02 github-actions[bot]

Thanks @mikehardy!

thymikee avatar Feb 10 '25 15:02 thymikee