Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect arm64 hostfxr.dll #24512

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

benvillalobos
Copy link
Member

@benvillalobos benvillalobos commented Mar 23, 2022

Help msbuild understand when it should load an arm64 hostfxr. I verified this works locally on an arm64 machine.

The check arm64 folder check should only run when finding hostfxr, otherwise we crash later in the build.

@benvillalobos
Copy link
Member Author

cc @marcpopMSFT

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we special casing this to only hostfxr? Are there other DLLs we load that we don't have an Arm64 version of, and do the Amd64 versions of those work correctly on Arm64?

@benvillalobos
Copy link
Member Author

Why are we special casing this to only hostfxr? Are there other DLLs we load that we don't have an Arm64 version of, and do the Amd64 versions of those work correctly on Arm64?

I retried my local build with this check fully expecting this to fail so I could share the log, but I retried without the hostfxr-specificity and it built properly. Pushed up that change.

@benvillalobos benvillalobos removed the request for review from donJoseLuis March 23, 2022 21:18
Comment on lines 42 to +43
string architecture = IntPtr.Size == 8 ? "x64" : "x86";
architecture = RuntimeInformation.ProcessArchitecture == Architecture.Arm64 ? "arm64" : architecture;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not:

Suggested change
string architecture = IntPtr.Size == 8 ? "x64" : "x86";
architecture = RuntimeInformation.ProcessArchitecture == Architecture.Arm64 ? "arm64" : architecture;
string architecture = Enum.GetName(RuntimeInformation.ProcessArchitecture).ToLowerInvariant();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of time, I decided not to consider the other code paths it could potentially go down if it were on a different arch. Though I suspect this to be the better way to go about it

@benvillalobos benvillalobos merged commit fec52a4 into dotnet:release/6.0.3xx Mar 24, 2022
@benvillalobos benvillalobos deleted the arm64-hostfxr branch March 24, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report an error when using --skip-manifest-update and --from-rollback-file
3 participants