Skip to content

Retarget DOTNETHOME when installing x64 on ARM64 #36100

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

Merged
merged 4 commits into from
Sep 16, 2021

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Sep 2, 2021

Draft that enables installing x64 product on ARM64.

Related PRs:
dotnet/arcade#7785
dotnet/runtime#58669
dotnet/installer#11843

@ericstj ericstj requested review from dougbu, wli3 and joeloff September 2, 2021 17:09
@ericstj ericstj requested a review from Pilchie as a code owner September 2, 2021 17:09
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 2, 2021
@ericstj ericstj marked this pull request as draft September 2, 2021 17:09
@Pilchie Pilchie requested review from wtgodbe and pranavkm September 2, 2021 17:40
@dougbu
Copy link
Contributor

dougbu commented Sep 2, 2021

I don't think GitHub will notify us when this PR is ready for review. Could you please let us know @ericstj

@ericstj
Copy link
Member Author

ericstj commented Sep 2, 2021

Sure, I'll let you know, mostly I just need to wait until the arcade change is reviewed and then replicate feedback here. One thing that's unique to ASP.NET is this: https://github.com/dotnet/aspnetcore/pull/36100/files#diff-b146853a2811958d2332489ea0501ea790ed1df7061172676d9c65a143d499b9R70

Can you have a look and let me know what you think?

@dougbu
Copy link
Contributor

dougbu commented Sep 3, 2021

Can you have a look and let me know what you think?

@wtgodbe or @joeloff, I think you have more context to answer @ericstj's question about https://github.com/dotnet/aspnetcore/pull/36100/files#diff-841d5a29fed63d4cad2012857ad278859c9fbfdb19a78c6899af34c6155d34daR69-R72

@ericstj on the other hand, we avoid TODO comments as much as possible. If we need a fix there, please either do it in this PR or file an issue and mention the issue in those comments.

@ericstj
Copy link
Member Author

ericstj commented Sep 3, 2021

@ericstj Eric St. John FTE on the other hand, we avoid TODO comments as much as possible. If we need a fix there, please either do it in this PR or file an issue and mention the issue in those comments.

I'm not going to commit it. It's in this PR while in draft because I need feedback from ASP.NET about it. I intend to get your feedback to address that comment and remove before committing.

@ericstj ericstj marked this pull request as ready for review September 13, 2021 18:28
@ericstj
Copy link
Member Author

ericstj commented Sep 13, 2021

This should now be ready for review. Note that I resolved the clashing registry keys between x64 and arm64 by making x64 install to a different path on ARM64 machines.

Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Couple of questions but I defer to @joeloff and @wtgodbe because they have more WiX-iness

@ericstj
Copy link
Member Author

ericstj commented Sep 15, 2021

I believe I responded to all feedback, reviewers, can you please have a look at the responses and let me know if this looks good?

@ericstj ericstj requested review from joeloff and dougbu September 15, 2021 21:23
@ericstj ericstj closed this Sep 16, 2021
@ericstj ericstj reopened this Sep 16, 2021
@ericstj ericstj merged commit 068797e into dotnet:main Sep 16, 2021
@ghost ghost added this to the 7.0-preview1 milestone Sep 16, 2021
@dougbu
Copy link
Contributor

dougbu commented Sep 16, 2021

@ericstj do we need this fix in release/6.0❔ If yes, release/6.0-rc2 as well❔

@ericstj
Copy link
Member Author

ericstj commented Sep 16, 2021

Yes, I will be bringing a long list of commits to port to 6.0-rc2

@ghost
Copy link

ghost commented Sep 16, 2021

Hi @ericstj. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@ericstj
Copy link
Member Author

ericstj commented Sep 18, 2021

/backport to release/6.0-rc2

@ghost
Copy link

ghost commented Sep 18, 2021

Hi @ericstj. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc2: https://github.com/dotnet/aspnetcore/actions/runs/1247487572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants