Remove GetCallingAssembly usage #35063
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Today the WebApplicationBuilder tries to guess what the application name should be based on the immediate caller of the API using Assembly.GetCallingAssembly. This is because several components use the application name as a way to find the application assembly in order to do other things; - User secrets uses it to find the location of the directory that holds configuration secrets. - MVC uses it as the assembly to start the search for controllers, pages, etc. - Identifty uses it to find application parts. This API is a bit fragile for internal refactoring (if the immediate caller is anything except for user code, then we will treat our own assembly as the calling assembly) and more importantly, it breaks in NativeAOT scenarios. Instead of the calling assembly, we now default to the entry assembly. - This will "break" in scenarios where the WebApplication is started in a library and the entry point is the host application (controllers will be scanned in potentially the wrong place by default or it will find the wrong controllers). - Now that we have the option that allows specifying the name of the application name via WebApplicationOptions, In the future, we're investigating ways that the compiler can inject the calling assembly via an attribute similar to [CallerMemberName] that would capture the calling assembly in a more reliable and AOT friendly way. - Added a test that asserts that the default application name is the entry assembly. - Updated the slnf file
ghost
added
the
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
captainsafia
reviewed
Aug 5, 2021
| // We need to override the application name since the call to Configure will set it to | ||
| // be the calling assembly's name. | ||
| webHostBuilder.UseSetting(WebHostDefaults.ApplicationKey, (callingAssembly ?? Assembly.GetEntryAssembly())?.GetName()?.Name ?? string.Empty); | ||
| webHostBuilder.UseSetting(WebHostDefaults.ApplicationKey, (Assembly.GetEntryAssembly())?.GetName()?.Name ?? string.Empty); |
Member
There was a problem hiding this comment.
Do we want people to be able ot set the assembly name directly from the options for the controllers in libraries scenarios?
Member
Author
There was a problem hiding this comment.
Yes, this is just the default. The API I added (WebApplicationOptions) allows setting the name. Or you can set it via command line args
PS: You just made me realize I should update this test to be a different assembly name, now that the entry assembly is the default!
- Change the name of the overriding assembly since the entry assembly is the default.
halter73
reviewed
Aug 5, 2021
| public void WebApplicationBuilderApplicationNameCanBeOverridden() | ||
| { | ||
| var assemblyName = Assembly.GetEntryAssembly().GetName().Name; | ||
| var assemblyName = typeof(WebApplicationTests).Assembly.GetName().Name; |
Member
There was a problem hiding this comment.
Can we just pick a random name so we don't accidently lose coverage in case we revert behavior or something?
Member
Author
There was a problem hiding this comment.
The only reason it could be random is because we're not loading the assembly in this situation. It should be a real assembly name...
halter73
approved these changes
Aug 5, 2021
amcasey
added
area-minimal
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This API is a bit fragile for internal refactoring (if the immediate caller is anything except for user code, then we will treat our own assembly as the calling assembly) and more importantly, it breaks in NativeAOT scenarios. Instead of the calling assembly, we now default to the entry assembly.
Fixes #34838