Skip to content

Use WIX_NATIVE_MACHINE to detect native architecture of target machine #37318

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 2 commits into from
Oct 6, 2021

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Oct 6, 2021

Requires #37290, build will fail until that fix is made.

This leverages the feature I added to Wix which reads the native architecture from the recent feature:
wixtoolset/wix3@75699d8

@ericstj ericstj requested a review from Pilchie as a code owner October 6, 2021 05:19
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 6, 2021
@ericstj ericstj requested review from joeloff, dougbu and wtgodbe October 6, 2021 05:35
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.

How long after this is in can the change be tested❔ I know we don't have many automated tests of our installers but this seems like an important change to get right

@joeloff
Copy link
Member

joeloff commented Oct 6, 2021

How long after this is in can the change be tested❔ I know we don't have many automated tests of our installers but this seems like an important change to get right

Once you have a build with the change, you should be able to grab an arm64 machine and then install both the x64 and arm64 builds. That should be enough to verify the installer change.

@ericstj
Copy link
Member Author

ericstj commented Oct 6, 2021

You can also test the x64 installer on an x64 machine and observe correct behavior. I tested the host MSI as part of validating the arcade fix which is the same authoring as this.

Prior to this change the existing logic has been validated in RC2 builds on both x64 and arm64 hardware.

@ericstj
Copy link
Member Author

ericstj commented Oct 6, 2021

Please merge when you feel comfortable with PR results. I reran "quarantined" leg because it timed out, not sure if that's required here. I'd like to merge so I can create backport PR.

@dougbu dougbu merged commit cec67a8 into dotnet:main Oct 6, 2021
@ghost ghost added this to the 7.0-preview1 milestone Oct 6, 2021
@ericstj
Copy link
Member Author

ericstj commented Oct 6, 2021

/backport to release/6.0

@ghost
Copy link

ghost commented Oct 6, 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.

@dougbu
Copy link
Contributor

dougbu commented Oct 6, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2021

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2021

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2021

@dougbu an error occurred while backporting to release/6.0, please check the run log for details!

Error: @dougbu is not a repo collaborator, backporting is not allowed.

@dougbu
Copy link
Contributor

dougbu commented Oct 6, 2021

Oops, I missed your request for some reason @ericstj.

Good thing GitHub's collaborators list is still woefully messed up. /fyi @TanayParikh

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