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

Fixing problem on Linux where dotnet run fails because we try to set an IIS config #17903

Open
wants to merge 5 commits into
base: contrib
Choose a base branch
from

Conversation

nul800sebastiaan
Copy link
Member

This is a follow up on #17886 which, after some discussion, seemed like a bad idea as there was a small possibility that we would break behavior that package developers might rely on.

Instead of not setting the global config, this catches errors while adding global rule to allow synchronous IO for IIS only (Kestrel one should run anywhere regardless, so I've kept it as it was before).

This has been tested on a Ubuntu machine with the apt package manager using these 3 commands:

sudo apt-get update && \
  sudo apt-get install -y dotnet-sdk-9.0
sudo apt-get update && \
  sudo apt-get install -y aspnetcore-runtime-9.0
sudo apt-get install -y dotnet-runtime-9.0

Installing Umbraco as described in the documentation would fail with a TypeLoaderException before this change:

image

After this fix it runs like it should:

image

Notes

  • I only put the configuration options for IIS in a try/catch - it never failed on Kestrel and I don't see why it would, Kestrel is fully supported on Windows as well
  • Had to use a little workaround to get the try/catch to actually work, by deferring the setting of the option to a new method (as found by @AndyButland in this SO post: https://stackoverflow.com/a/3346975)
  • I've left the obsoletion message in there, I think it's a good idea to remove this global override as soon as possible. I've set it to v17 for now, but should it be for v16?

@AndyButland AndyButland self-requested a review January 7, 2025 09:37
Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

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

Looks good, but agree the obsolete can reference 16 and I left a couple of comments inline.

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.

3 participants