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

[macOS] Don't force the RID to be x64-only #16678

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented Aug 10, 2023

Description of Change

Use maccatalyst-arm64 on arm64 and maccatalyst-x64 on x64.

@mattleibow mattleibow requested a review from a team as a code owner August 10, 2023 17:00
Eilon
Eilon previously approved these changes Aug 10, 2023
@mattleibow
Copy link
Member Author

Blazor is still causing issues with a parallel/double build of a RID for an app.

@mattleibow
Copy link
Member Author

The real fix is here: #16765

@Eilon Eilon added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Aug 15, 2023
@MartyIX
Copy link
Contributor

MartyIX commented Aug 16, 2023

Maybe better

Don't force the RID to be x64
->
[macOS] Don't force the RID to be x64

?

@rmarinho
Copy link
Member

/rebase

We still need to keep the single RID as the blazor bits have issues
@mattleibow mattleibow changed the title Don't force the RID to be x64 [macOS] Don't force the RID to be x64-only Aug 17, 2023
@jfversluis jfversluis removed the request for review from rachelkang August 17, 2023 09:27
@mattleibow mattleibow requested review from Eilon and Redth August 17, 2023 22:08
@mattleibow mattleibow enabled auto-merge (squash) August 17, 2023 22:08
@samhouts samhouts requested review from Eilon and removed request for Eilon August 17, 2023 22:14
@@ -9,6 +9,7 @@
<NoWarn>$(NoWarn),CA1416</NoWarn>
<!-- Disable multi-RID builds to workaround a parallel build issue -->
<RuntimeIdentifier Condition="$(TargetFramework.Contains('-maccatalyst'))">maccatalyst-x64</RuntimeIdentifier>
<RuntimeIdentifier Condition="$(TargetFramework.Contains('-maccatalyst')) and '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'arm64'">maccatalyst-arm64</RuntimeIdentifier>
Copy link
Member

Choose a reason for hiding this comment

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

Does it matter that this is checking whether the current machine is ARM64? Why would that matter for what RID to target? I get that it matters if you're going to run it, but it shouldn't affect compilation, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we typically build and run on the same machine on CI. The real issue is that Blazor and/or maccatalyst can do multiple rids. So without this people on arm64 can or deploy as it always builds x64.

@mattleibow mattleibow merged commit 2287124 into main Aug 18, 2023
39 checks passed
@mattleibow mattleibow deleted the dev/mac-is-arm-too branch August 18, 2023 15:05
rmarinho pushed a commit that referenced this pull request Aug 19, 2023
We still need to keep the single RID as the blazor bits have issues
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants