Core - ARM64 support for .Net 4.8.1 (rebased)#5267
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughARM64 build targets are added across the solution, native and managed projects, runtime lookup code, and NuGet packaging files. ChangesARM64 build and packaging support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant CefRuntime
participant RuntimeInformation
participant FileSystem
Application->>CefRuntime: Request AnyCPU runtime loading
CefRuntime->>RuntimeInformation: Read ProcessArchitecture
CefRuntime->>FileSystem: Resolve and load architecture-specific runtime
FileSystem-->>CefRuntime: Return assembly or file-not-found result
CefRuntime-->>Application: Complete loading or raise error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| </ItemGroup> | ||
| <Target Name="PostBuild" AfterTargets="PostBuildEvent"> | ||
| <Exec Command="if $(PlatformName) == x86 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars32.bat"
 editbin /largeaddressaware /TSAWARE "$(TargetPath)" 
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
)" /> | ||
| <Exec Command="if $(PlatformName) == x86 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars32.bat"
 editbin /largeaddressaware /TSAWARE "$(TargetPath)" 
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else if $(PlatformName) == x64 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else if $(PlatformName) == arm64 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvarsamd64_arm64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
)" /> |
There was a problem hiding this comment.
Need to verify the output with dumpbin
| string assemblyName = args.Name.Split(new[] { ',' }, 2)[0] + ".dll"; | ||
| string archSpecificPath = Path.Combine(basePath, | ||
| Environment.Is64BitProcess ? "x64" : "x86", | ||
| RuntimeInformation.ProcessArchitecture.ToString().ToLowerInvariant(), |
There was a problem hiding this comment.
Can extract this into a static field for reuse
| $ARCHES.Remove("arm64") | ||
| $CefSln = Join-Path $WorkingDir 'CefSharp3.sln' | ||
| $NugetPackagePath = "nuget"; | ||
| $NupkgFiles = @('CefSharp.Common.nuspec', 'CefSharp.WinForms.nuspec', 'CefSharp.Wpf.nuspec', 'CefSharp.OffScreen.nuspec') | ||
| $VCXProjPackageConfigFiles = @('CefSharp.Core.Runtime\packages.CefSharp.Core.Runtime.config', 'CefSharp.BrowserSubprocess.Core\packages.CefSharp.BrowserSubprocess.Core.config'); | ||
| $SupportedArches.AddRange(@("x86", "x64")); | ||
| $SupportedArches.AddRange(@("x86", "x64", "arm64")); |
There was a problem hiding this comment.
I stripped this down to the bare minimum, additional changes to support native arm64 compilation can be added in a followup PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.ps1`:
- Around line 495-499: The default multi-arch path now includes arm64, but VSX
still bootstraps the VC environment from only the first entry in $ARCHES, so the
arm64 build can inherit the wrong toolset. Update the VC setup logic in VSX to
select an arm64-capable vcvarsall mode whenever the arches list contains arm64,
or otherwise ensure the initial bootstrap matches the architectures being built
instead of always using $ARCHES[0].
In `@CefSharp.BrowserSubprocess/CefSharp.BrowserSubprocess.csproj`:
- Line 45: The build step in CefSharp.BrowserSubprocess.csproj uses the
x64-to-ARM64 cross toolchain for the arm64 branch, which is incorrect for native
ARM64 Visual Studio environments. Update the Exec command in the browser
subprocess project so the arm64 path uses the native ARM64 vcvars bootstrap (or
branches on host architecture to choose the right vcvars entrypoint) before
running editbin and sn, keeping the existing x86 and x64 behavior unchanged.
In `@NuGet/CefSharp.Common.targets`:
- Around line 472-475: The AnyCPU restore-copy path in CefSharp.Common.targets
updates the x86/x64 destination properties but leaves the arm64 destination
unmodified, so the arm64 payload is copied to the root instead of an arm64
subfolder. Update the property group that defines CefRedistArm64TargetDir so the
AnyCPU case appends arm64\ in the same way CefRedist32TargetDir and
CefRedist64TargetDir append x86\ and x64\, and keep the change aligned with the
AnyCPU restore-copy target that now includes arm64.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d80cf05-8ab1-46c2-9ce7-3da49111f671
📒 Files selected for processing (22)
CefSharp.BrowserSubprocess.Core/CefSharp.BrowserSubprocess.Core.vcxprojCefSharp.BrowserSubprocess/CefSharp.BrowserSubprocess.csprojCefSharp.Core.Runtime/CefSharp.Core.Runtime.vcxprojCefSharp.Native.propsCefSharp.OffScreen.Example/CefSharp.OffScreen.Example.csprojCefSharp.OffScreen.Example/CefSharp.OffScreen.Example.netcore.csprojCefSharp.Test/CefSharp.Test.csprojCefSharp.Test/CefSharp.Test.netcore.csprojCefSharp.WinForms.Example/CefSharp.WinForms.Example.csprojCefSharp.WinForms.Example/CefSharp.WinForms.Example.netcore.csprojCefSharp.Wpf.Example/CefSharp.Wpf.Example.csprojCefSharp.Wpf.Example/CefSharp.Wpf.Example.netcore.csprojCefSharp.Wpf.HwndHost.Example/CefSharp.Wpf.HwndHost.Example.csprojCefSharp.Wpf.HwndHost.Example/CefSharp.Wpf.HwndHost.Example.netcore.csprojCefSharp/CefRuntime.csCefSharp/CefSharp.csprojCefSharp/DependencyChecker.csCefSharp3.slnNuGet/CefSharp.Common.nuspecNuGet/CefSharp.Common.propsNuGet/CefSharp.Common.targetsbuild.ps1
| </ItemGroup> | ||
| <Target Name="PostBuild" AfterTargets="PostBuildEvent"> | ||
| <Exec Command="if $(PlatformName) == x86 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars32.bat"
 editbin /largeaddressaware /TSAWARE "$(TargetPath)" 
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
)" /> | ||
| <Exec Command="if $(PlatformName) == x86 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars32.bat"
 editbin /largeaddressaware /TSAWARE "$(TargetPath)" 
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else if $(PlatformName) == x64 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
) else if $(PlatformName) == arm64 (
 call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvarsamd64_arm64.bat"
 editbin /TSAWARE /STACK:8388608 "$(TargetPath)"
 sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"
)" /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Visual Studio 2022 C++ command-line environments, what vcvars batch file should be used when the host machine is Windows ARM64 and the target architecture is ARM64?
💡 Result:
When the host machine is Windows ARM64 and you are targeting ARM64, the recommended approach is to use the vcvarsall.bat batch file with the arm64 argument [1]. To initialize the environment, execute the following command in your command-line shell: "C:\Program Files\Microsoft Visual Studio\2022[Edition]\VC\Auxiliary\Build\vcvarsall.bat" arm64 Replace [Edition] with your specific Visual Studio 2022 edition (e.g., Community, Professional, or Enterprise). Using the arm64 argument invokes the native ARM64-hosted toolchain, which is the correct environment for building ARM64 binaries on an ARM64 machine [2][1]. While some cross-compilation batch files exist for other scenarios (such as vcvarsamd64_arm64.bat for x64-to-ARM64 builds), they are not appropriate when the host is already ARM64, as using an emulated or cross-hosted environment can lead to issues, such as the inability to correctly utilize certain compiler switches like /arm64EC [3][2]. Alternatively, Visual Studio 2022 provides an "ARM64 Developer Command Prompt" shortcut in the Start menu that automatically runs this configuration for you [3].
Citations:
- 1: MSVC first-class support for producing ARM64EC binaries SCons/scons#4745
- 2: https://github.com/MicrosoftDocs/visualstudio-docs/blob/main/docs/install/visual-studio-on-arm-devices.md
- 3: https://neuroanswers.net/c/programming/q/build-arm64ec-visual-studio-2022-command-line
Use the native ARM64 vcvars bootstrap here. vcvarsamd64_arm64.bat is the x64→ARM64 cross-environment; on ARM64-hosted VS this should use vcvarsall.bat arm64 (or branch on host architecture) so editbin and sn run under the native ARM64 toolchain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CefSharp.BrowserSubprocess/CefSharp.BrowserSubprocess.csproj` at line 45, The
build step in CefSharp.BrowserSubprocess.csproj uses the x64-to-ARM64 cross
toolchain for the arm64 branch, which is incorrect for native ARM64 Visual
Studio environments. Update the Exec command in the browser subprocess project
so the arm64 path uses the native ARM64 vcvars bootstrap (or branches on host
architecture to choose the right vcvars entrypoint) before running editbin and
sn, keeping the existing x86 and x64 behavior unchanged.
There was a problem hiding this comment.
CI is only going to cross compile so that's the priority, native compilation support can be added in a follow up PR.
show proposed change for future reference.
There was a problem hiding this comment.
@amaitland, makes sense — if CI is only doing x64-hosted cross-compilation for this PR, I’m happy to treat native ARM64-hosted VS support as follow-up work.
For future reference, here’s the shape of the change I had in mind for Line 45, keeping the existing x86/x64 behavior unchanged and only making the arm64 path host-aware:
<Target Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="if $(PlatformName) == x86 (&`#xD`;&`#xA`; call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars32.bat"&`#xD`;&`#xA`; editbin /largeaddressaware /TSAWARE "$(TargetPath)"&`#xD`;&`#xA`; sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"&`#xD`;&`#xA`;) else if $(PlatformName) == x64 (&`#xD`;&`#xA`; call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvars64.bat"&`#xD`;&`#xA`; editbin /TSAWARE /STACK:8388608 "$(TargetPath)"&`#xD`;&`#xA`; sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"&`#xD`;&`#xA`;) else if $(PlatformName) == arm64 (&`#xD`;&`#xA`; if /I "%PROCESSOR_ARCHITECTURE%" == "ARM64" (&`#xD`;&`#xA`; call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvarsall.bat" arm64&`#xD`;&`#xA`; ) else (&`#xD`;&`#xA`; call "$(DevEnvDir)..\..\VC\Auxiliary\Build\vcvarsamd64_arm64.bat"&`#xD`;&`#xA`; )&`#xD`;&`#xA`; editbin /TSAWARE /STACK:8388608 "$(TargetPath)"&`#xD`;&`#xA`; sn -R "$(TargetPath)" "$(ProjectDir)..\CefSharp.snk"&`#xD`;&`#xA`;)" />
</Target>That would preserve the current CI path while also handling native ARM64-hosted developer environments later.
If you want, I can also open a follow-up issue for that so it doesn’t get lost.
|
✅ Build CefSharp 149.0.40-CI5546 completed (commit b8a1244891 by @amaitland) |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. A stacked PR containing fixes has been created.
Time taken: |
|
✅ Build CefSharp 149.0.40-CI5548 completed (commit c98eee8cb3 by @amaitland) |
amaitland
force-pushed
the
ter-805-arm64-target
branch
from
6bf720e to
4c02c35
Compare
|
✅ Build CefSharp 150.0.110-CI5556 completed (commit a66d263e82 by @amaitland) |
2 participants
Fixes #4155
Summary by CodeRabbit