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

Skip getting Windows GUI bit on non-Windows in AppHostShellShimMaker #35600

Merged

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Sep 20, 2023

AppHostShellShimMaker had an implicit assumption that ResourceUpdater.IsSupportedOS() was tied to Windows. That is no longer the case as of RC1, resulting in PEUtils.GetWindowsGraphicalUserInterfaceBit being unnecessarily called on non-Windows when creating a shim.

See dotnet/runtime#92344.
This doesn't address the underlying issue around the function not properly handling endianness, but it brings back the previous behaviour where the problematic function isn't called when not needed.

cc @agocke @dotnet/domestic-cat

Customer Impact

When installing a tool on a non-Windows platform, we are unnecessarily trying to get the Windows subsystem bit for a PE image. This exposed a pre-existing issue where the function called did not properly handle endianness and resulted in a failure to install the tool on non-little-endian systems. This was a regression in RC1. See dotnet/runtime#92344.

Testing

Manual.

Risk

Low. This switches the order of a check such that we only try to get the Windows subsystem bit on Windows (instead of always getting it and only using it on Windows).

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Sep 20, 2023
@elinor-fung elinor-fung added this to the 8.0.1xx milestone Sep 20, 2023
@agocke
Copy link
Member

agocke commented Sep 20, 2023

Don't we need to know if we're publishing a Windows GUI app from Linux? Aren't there important modifications that happen in that situation?

@elinor-fung
Copy link
Member Author

elinor-fung commented Sep 21, 2023

This is for creating the apphost shim when installing dotnet tools using the SDK, not normal build/publish of a user's app. The apphost used for that scenario is the one matching the current OS, so only Windows uses the Windows apphost and there is no creation of a Windows GUI tool shim from non-Windows. The existing logic already checks for OperatingSystem.IsWindows() - this just flips it so that we check that first before trying to get the Windows GUI bit.

@elinor-fung elinor-fung added Servicing-consider and removed untriaged Request triage from a team member labels Sep 21, 2023
@elinor-fung
Copy link
Member Author

Approved by Tactics via e-mail.

@elinor-fung elinor-fung merged commit faef6d6 into dotnet:release/8.0.1xx-rc2 Sep 22, 2023
16 checks passed
@elinor-fung elinor-fung deleted the shimMaker-guiBit branch September 22, 2023 00:02
@elinor-fung
Copy link
Member Author

/backport to release/8.0.1xx

@github-actions
Copy link
Contributor

Started backporting to release/8.0.1xx: https://github.com/dotnet/sdk/actions/runs/6340385921

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.

4 participants