Add ARM64 Desktop support #5864
Conversation
There was a problem hiding this comment.
Nice change. Happy it wasn't too invasive.
As a heads up, JSI unit tests normally also run against V8, but USE_V8 is temporarily disabled in master while Tudor has been doing some work to have it use new JSI headers and adding ARM64 support. It's not disabled in older branches, so it might cause test breaks there. I think Tudor may already have a version of V8 ready though.
Please wait for @asklar to sign off on the WinUI changes, and check with @acoates-ms before trying to bring this back further than 0.62, because we should be bringing that into devmain imminently.
There was a problem hiding this comment.
I'm curious about the longer term plan for this since ChakraCore is end of life'd:
chakra-core/ChakraCore#6384
Can you comment on that?
Also why is a ChakraCore nuget package being created out of the react native windows repo instead of the chakra core repo?
We're moving everything to V8 currently, but are still transitioning. The ARM64 ChakraCore variant is a bit special, and more details can be given offline/less publicly. |
JunielKatarn
added
the
AutoMerge
|
Hello @JunielKatarn! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 7 hours 17 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
| X86Release: | ||
| BuildConfiguration: Release | ||
| BuildPlatform: x86 | ||
| ARM64Debug: | ||
| BuildConfiguration: Debug |
There was a problem hiding this comment.
Do these both really need to be added to the PR pipeline? For that matter, why does the desktop PR pipeline have so many configs?
UWP gets x86 Debug and x64 Release - the PR CI is supposed to be a lean check.
There was a problem hiding this comment.
I was not aware UWP was missing build combinations. Why is this?
Maybe for UWP, heavily based on COM and obviously WinRT, compile/link issues are less likely to happen.
For raw Windows Desktop, we have found issues as specific as x86|Debug|v141.
In general, I think we should build every flavor.
Total time should be the same, thanks to parallel builds.
The checks should be lean, but coverage should be wide.
There was a problem hiding this comment.
Sorry, short answer: Yes. Both will be added.
There was a problem hiding this comment.
@JunielKatarn please don't add all combinations, see the comment above this:
matrix: # Why we only build some flavors: https://github.com/microsoft/react-native-windows/issues/4308
There was a problem hiding this comment.
I didn't see that comment. It's not in the Desktop job:
- job: RNWDesktopPR
displayName: Desktop PR
dependsOn: Setup
condition: ne( dependencies.Setup.outputs['checkPayload.shouldSkipPRBuild'], 'True' )
strategy:
matrix:
X64Debug:
BuildConfiguration: Debug
...
I can adjust this in a subsequent change, but we do build the whole current matrix for Desktop.
I'll look at that commented issue to see the what was the agreed exclusion criteria.
ghost
merged commit 
The ChakraCore team did not agree to produce a new NuGet version including the ARM64 build because this would require changing their pipeline. They decided to only provide the binaries. I believe such reluctance is related to ChakraCore being on "life-support", so to speak. |
* Add ARM64 ChakraCore NUSPEC * Add NuGet dependency ReactWindows.ChakraCore.ARM64 * Add ARM64 platform for ReactWindows-Desktop.sln * Remove ARM64 from a few projects * Add ARM64 artifacts to nuspec * Upgrade to ChakraCore.Debugger 0.0.0.44 * Upgrade to ReactWindows.ChakraCore.ARM64 1.11.20 * Change files * Update CI matrices * Re-introduced Hermes NuGet comment * Add missing platform condition in JSI.Desktop
* Add ARM64 Desktop support (#5864) * Add ARM64 ChakraCore NUSPEC * Add NuGet dependency ReactWindows.ChakraCore.ARM64 * Add ARM64 platform for ReactWindows-Desktop.sln * Remove ARM64 from a few projects * Add ARM64 artifacts to nuspec * Upgrade to ChakraCore.Debugger 0.0.0.44 * Upgrade to ReactWindows.ChakraCore.ARM64 1.11.20 * Change files * Update CI matrices * Re-introduced Hermes NuGet comment * Add missing platform condition in JSI.Desktop * Upgrade to ReactWindows.OpenSSL.StdCall.Static 1.0.2-p.5 * Upgrade to ReactNative.V8Jsi.Windows 0.3.4 * Upgrade to Microsoft.Windows.CppWinRT 2.0.200615.7 * Upgrade to Microsoft.ChakraCore.vc140 1.11.20 * Remove change file * Change files
|
Removing backport requests to 61 and 60 as these will not be needed |
Implements #5828
Microsoft Reviewers: Open in CodeFlow