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

Make UseEnvironment/ContentRoot override config #41003

Closed
wants to merge 1 commit into from

Conversation

Tratcher
Copy link
Member

dotnet/aspnetcore#18499

UseEnvironment and UseContentRoot have a composition problem. Because they were implemented using config they're order sensitive and can be overridden by config. That has caused confusion especially for UseEnvironment.

Proposal: Have UseEnvironment and UseContentRoot store their data in IHostBuilder.Properties rather than config and treat this as higher priority than config. This works well for UseEnvironment, but there are a few places where UseContentRoot is used to provide a default value that is intended to compose with config. Those can be easily expressed using ConfigureHostConfig like UseContentRoot used to.

There's at least one more UseContentRoot call in UseWindowsServices we might want to update.
https://github.com/dotnet/extensions/blob/f4066026ca06984b07e90e61a6390ac38152ba93/src/Hosting/WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs#L50

@Tratcher Tratcher self-assigned this Aug 18, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Aug 18, 2020

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

@Tratcher
Copy link
Member Author

Interesting question if this breaks these scenarios:
https://github.com/dotnet/aspnetcore/blob/bbb851e3ebf40f79531bc13dd5c1b56b332237fc/src/Servers/IIS/IIS/src/WebHostBuilderIISExtensions.cs#L36
https://github.com/dotnet/aspnetcore/blob/d0ab959c89a0adf6b86d35039b6a91349a5bedd1/src/Hosting/TestHost/src/WebHostBuilderExtensions.cs#L126
They're each using the IWebHostBuilder variant of the API, but they wouldn't be able to override the content root if someone and called IHostBuilder.UseContentRoot before them.

All of the composition concerns are with UseContentRoot so we could leave that one alone, but it seems strange for the two to be different.

@danmoseley
Copy link
Member

Hello, changes into this branch must meet bug bar and require a template. Fwded email.

Is this a port from master?

@Tratcher Tratcher marked this pull request as draft August 19, 2020 18:32
@Tratcher Tratcher closed this Aug 21, 2020
@Tratcher Tratcher deleted the tratcher/useenv branch September 17, 2020 23:21
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants