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

DotNetCompatibilityCheck: don't abort install on incompatible platform #459

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

apacker1
Copy link
Contributor

@apacker1 apacker1 commented Sep 25, 2023

If running NetCoreCheck.exe fails with error code ERROR_EXE_MACHINE_TYPE_MISMATCH or ERROR_BAD_EXE_FORMAT then don't abort the installation, just set the property to -1. Fixes wixtoolset/issues#7737

@github-actions
Copy link

github-actions bot commented Sep 25, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@apacker1
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

wixbot added a commit to wixtoolset/.github that referenced this pull request Sep 25, 2023
@apacker1 apacker1 marked this pull request as ready for review September 25, 2023 21:06
@barnson
Copy link
Member

barnson commented Sep 25, 2023

Thanks. This looks good. We'll triage the issue at our meeting next week. My gut says -1 is probably not the best value, just because of signs. I'm thinking 1 (+1) or 65535. Thoughts?

@apacker1
Copy link
Contributor Author

Yeah, I wasn't sure about that. I guess the main thing is that the value shouldn't be something that the NetCoreCheck.exe authors might use in the future, which I don't really know how to predict. However, https://github.com/dotnet/deployment-tools/blob/main/src/clickonce/native/projects/NetCoreCheck/NetCoreCheck.h does make it look like any additional error codes will probably just increment from the current ones that are defined, around 0x3000. So 65535 should be safe. Maybe they could conceivably use +1 if they come up with some "successful, but ..." result code? So I guess I'd vote for 65535, but I'm happy to do whatever you think is best.

@barnson
Copy link
Member

barnson commented Sep 25, 2023

Yeah, that was my thinking too. I left a comment on the issue so triage makes a recommendation.

@barnson
Copy link
Member

barnson commented Oct 3, 2023

Triage team says 13 is a good compromise of return value.

…r code ERROR_EXE_MACHINE_TYPE_MISMATCH or ERROR_BAD_EXE_FORMAT then don't abort the installation, just set the property to 13. Fixes issue #7737
@apacker1
Copy link
Contributor Author

apacker1 commented Oct 4, 2023

@barnson OK, I've updated my code to use 13. Also updated wixtoolset/web#229 to reflect this change

@barnson barnson merged commit 0014af6 into wixtoolset:develop Oct 4, 2023
3 checks passed
@barnson
Copy link
Member

barnson commented Oct 4, 2023

Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2023
@apacker1 apacker1 deleted the issue-7737 branch October 4, 2023 16:21
@apacker1 apacker1 restored the issue-7737 branch October 4, 2023 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DotNetCompatibilityCheck with Platform attribute "x64" aborts installation on x86 OS
2 participants