Skip to content

[release/6.0] Fix diff checks for ContentRoot and WebRoot #41182

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 1 commit into from
May 4, 2022

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Apr 14, 2022

Prevent unnecessary startup errors in WebApplicationBuilder

Without this fix, when a wwwroot directory exists, calling the builder.WebHost.ConfigureAppConfiguration() method throws a NotSupportedException error thinking the web root directory (wwwroot by default) was modified when it wasn't.

Description

Backport of #40095. #38212 (comment) brought this back to my attention.

Customer Impact

Normally we'd tell customers to use builder.Configuration instead of builder.WebHost.ConfigureAppConfiguration(), but builder.WebHost is an IWebHostBuilder adapter for many extension methods like .UseStaticWebAssets().

This bug causes .UseStaticWebAssets() and just about any other method calling ConfigureAppConfiguration() to throw a NotSupportedException when it shouldn't. There is a workaround, but it's hard to discover and shouldn't be necessary.

var builder = WebApplication.CreateBuilder(args);
builder.WebHost.UseStaticWebAssets(); // Throws NotSupportedException today
builder.WebHost.UseWebRoot("wwwroot").UseStaticWebAssets(); // Works

Fixes #39546 and #38212

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This change has automated tests and was fixed in 7.0-preview2 and verified by customers.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@halter73 halter73 requested a review from captainsafia April 14, 2022 00:51
@halter73 halter73 requested a review from Tratcher as a code owner April 14, 2022 00:51
@ghost ghost added the area-runtime label Apr 14, 2022
@ghost ghost added this to the 6.0.x milestone Apr 14, 2022
@ghost
Copy link

ghost commented Apr 14, 2022

Hi @halter73. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@halter73 halter73 added the Servicing-consider Shiproom approval is required for the issue label Apr 26, 2022
@ghost
Copy link

ghost commented Apr 26, 2022

Hi @halter73. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@adityamandaleeka
Copy link
Member

@halter73 Has this been sent to tactics?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants